mirror of
https://github.com/multica-ai/multica.git
synced 2026-08-13 19:43:27 +02:00
* fix(issues): return to the source list after deleting an issue Deleting an issue from its detail page always pushed the workspace Issues list, so opening an issue from My Issues (or a project list, search, a pin, an agent panel) and deleting it dropped the user's navigation context — GH #5995. Go back instead. `useBackOrReplace` steps back when the platform reports in-app history and replaces with a fallback path when there is none, so a shared link opened cold or a new tab never steps off the app. Web answers via the Navigation API, falling back to counting its own pushes; desktop reads the active tab's virtual history. `replace`, not `push`: the deleted issue's URL must not stay in history for the back button to land on a 404. The not-found "Back to Issues" button loses the same context, so it moves to the same helper and its label becomes a plain "Back". Co-authored-by: multica-agent <github@multica.ai> * fix(web): track history position, not push count, for canGoBack The Navigation API fallback only ever counted pushes, so `pushes > 0` did not mean the current entry still had an in-app page behind it. Cold-open an issue, click Issues, press the browser's Back button, then delete: the tracker still claimed history and `back()` would step off Multica — the exact case the fallback exists to prevent. Count depth instead: a push adds one, any traversal takes one away (clamped at zero). Reaching the document's first entry requires traversing back at least as many times as we pushed, so a positive depth can never be claimed while sitting on it. A browser Forward is now conservative — it reports no history where a step back would have been fine — which costs a fallback navigation rather than an exit. Also corrects the useBackOrReplace contract comment: stepping back leaves the dead URL in forward history, so the guarantee is that it never lands on the back stack, not that it leaves history entirely. Co-authored-by: multica-agent <github@multica.ai> * fix(web): answer canGoBack from the browser alone, never from push counts Review found the counter still lied, one level deeper: it counted `router.push` calls, and a call is not a committed history entry. Next drops a push to `replaceState` when the canonical URL is unchanged (app-router.js, the `pendingPush && href !== canonicalUrl` branch), so clicking a self-link — the breadcrumb on an issue detail page, a pin to the issue you are on — incremented depth with no entry behind it, and a delete from there could still step off the app. Fixing the count needs per-entry markers, which means depending on both React effect ordering (our provider's effect runs before app-router's history commit) and Next's own preserveCustomHistoryState behaviour. Two rounds of review have now found holes in hand-rolled history tracking; a third layer of it is not the way to buy this guarantee. So stop deriving. `canGoBack` is the Navigation API's answer or `false`. Browsers without it take the fallback path, which is exactly what they did before any of this existed — nobody regresses, and the "wrong true walks the user out of the app" failure is now unreachable by construction. Drops the tracker, the popstate wiring and the push wrapper: the `multica:navigate` bridge returns to its original shape. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Bohan-J <bohan@devv.ai> Co-authored-by: multica-agent <github@multica.ai>
126 lines
3.7 KiB
TypeScript
126 lines
3.7 KiB
TypeScript
/**
|
|
* MUL-5208 — the web half of the `multica:navigate` bridge.
|
|
*
|
|
* Shared content (comments, chat, issue descriptions) fires this event whenever
|
|
* a link resolves to an in-app destination, including an absolute URL on this
|
|
* deployment's own origin. Desktop answers it by opening a tab; the web must
|
|
* answer it with a router push, or those links silently do nothing.
|
|
*/
|
|
import { describe, expect, it, vi, afterEach, beforeEach } from "vitest";
|
|
import { render } from "@testing-library/react";
|
|
|
|
const router = vi.hoisted(() => ({
|
|
push: vi.fn(),
|
|
replace: vi.fn(),
|
|
back: vi.fn(),
|
|
prefetch: vi.fn(),
|
|
}));
|
|
|
|
vi.mock("next/navigation", () => ({
|
|
useRouter: () => router,
|
|
usePathname: () => "/acme/issues",
|
|
useSearchParams: () => new URLSearchParams(),
|
|
}));
|
|
|
|
import { WebNavigationProvider } from "./navigation";
|
|
import { useNavigation, type NavigationAdapter } from "@multica/views/navigation";
|
|
|
|
function navigate(path: string) {
|
|
window.dispatchEvent(
|
|
new CustomEvent("multica:navigate", { detail: { path } }),
|
|
);
|
|
}
|
|
|
|
function renderAdapter(): () => NavigationAdapter {
|
|
let adapter: NavigationAdapter | null = null;
|
|
function Probe() {
|
|
adapter = useNavigation();
|
|
return null;
|
|
}
|
|
render(
|
|
<WebNavigationProvider>
|
|
<Probe />
|
|
</WebNavigationProvider>,
|
|
);
|
|
return () => adapter!;
|
|
}
|
|
|
|
beforeEach(() => {
|
|
router.push.mockReset();
|
|
});
|
|
|
|
describe("WebNavigationProvider internal link bridge", () => {
|
|
it("pushes the path a content link resolved to", () => {
|
|
render(<WebNavigationProvider>{null}</WebNavigationProvider>);
|
|
|
|
navigate("/acme/issues/MUL-1");
|
|
|
|
expect(router.push).toHaveBeenCalledWith("/acme/issues/MUL-1");
|
|
});
|
|
|
|
it("ignores an event without a path", () => {
|
|
render(<WebNavigationProvider>{null}</WebNavigationProvider>);
|
|
|
|
window.dispatchEvent(new CustomEvent("multica:navigate", { detail: {} }));
|
|
|
|
expect(router.push).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("stops listening once unmounted", () => {
|
|
const { unmount } = render(
|
|
<WebNavigationProvider>{null}</WebNavigationProvider>,
|
|
);
|
|
|
|
unmount();
|
|
navigate("/acme/issues/MUL-1");
|
|
|
|
expect(router.push).not.toHaveBeenCalled();
|
|
});
|
|
});
|
|
|
|
/**
|
|
* `canGoBack` decides whether a page whose subject was just deleted steps back
|
|
* or replaces with a fallback. A wrong `true` walks the user out of Multica,
|
|
* so the adapter must expose the browser's own answer and nothing derived.
|
|
*/
|
|
describe("WebNavigationProvider canGoBack", () => {
|
|
const win = window as unknown as { navigation?: unknown };
|
|
|
|
afterEach(() => {
|
|
delete win.navigation;
|
|
});
|
|
|
|
it("passes through the Navigation API's answer", () => {
|
|
win.navigation = { canGoBack: true };
|
|
|
|
expect(renderAdapter()().canGoBack!()).toBe(true);
|
|
});
|
|
|
|
it("reads the answer live rather than freezing it at render", () => {
|
|
win.navigation = { canGoBack: true };
|
|
const adapter = renderAdapter();
|
|
|
|
win.navigation = { canGoBack: false };
|
|
|
|
expect(adapter().canGoBack!()).toBe(false);
|
|
});
|
|
|
|
// Regression (PR review, twice): a count of `push` calls stood in for real
|
|
// history here. It could not — Next drops a push to `replaceState` when the
|
|
// canonical URL is unchanged, so a push that committed no entry still made
|
|
// this claim `true`. Calling push must not move this answer at all.
|
|
it("is unmoved by a push that committed no history entry", () => {
|
|
win.navigation = { canGoBack: false };
|
|
const adapter = renderAdapter();
|
|
|
|
adapter().push("/acme/issues");
|
|
|
|
expect(router.push).toHaveBeenCalledWith("/acme/issues");
|
|
expect(adapter().canGoBack!()).toBe(false);
|
|
});
|
|
|
|
it("reports false where the browser cannot answer, so callers use the fallback", () => {
|
|
expect(renderAdapter()().canGoBack!()).toBe(false);
|
|
});
|
|
});
|