From b3116de3549828bf53ea5d6a86d68ffd230567a3 Mon Sep 17 00:00:00 2001 From: Ajay Ghanwat Date: Fri, 7 Aug 2026 17:54:13 +0530 Subject: [PATCH] fix(ui): derive tab selection from the url instead of syncing to it My diagnosis in the previous commit was wrong. The component root was not being replaced by a reactive re-render: the client router owns popstate and swaps the whole page shell on back and forward, which discards component state entirely. Every mechanism that tried to push state into the component from outside was therefore doomed -- clicking a tab, announcing an event, tracking the last applied value. In url mode the query parameter is now simply the source of truth, read where the selection is computed. Whatever render happens next produces the right tab, with no listener to lose and nothing to keep in step. This deletes the runtime tab sync entirely -- 1590 bytes -- and fixes the back/forward cases that were previously broken. Verified in the showcase: click writes the url, two backs and two forwards each land on the right tab, and a ?tab= deep link opens on it. Co-Authored-By: Claude Opus 5 --- docs/ui-visual-contract-0.8.json | 2 +- packages/csr/src/reactive-runtime.ts | 35 -------------- packages/csr/test/reactive.test.ts | 69 ---------------------------- packages/ui/components/Tabs.wrn | 49 ++++++-------------- packages/ui/test/ui.test.ts | 10 ++++ 5 files changed, 24 insertions(+), 141 deletions(-) diff --git a/docs/ui-visual-contract-0.8.json b/docs/ui-visual-contract-0.8.json index 98cd6aae..dfafe286 100644 --- a/docs/ui-visual-contract-0.8.json +++ b/docs/ui-visual-contract-0.8.json @@ -89,7 +89,7 @@ "packages/ui/components/StrongPassword.wrn": "c7c5ef26ece6170dd7db9882f0dc98cb2e4607f1e5d43eb3fd39e5d15bbd3a2e", "packages/ui/components/StyledIcon.wrn": "4a4504e357dee9dd0418edbe85fc90b824ccdb3752123a2125da95b77bf0b336", "packages/ui/components/Switch.wrn": "ccb74599fab72b0d68b09a7f1f90b7732cb2f9cbed67a84219e897575ce30c28", - "packages/ui/components/Tabs.wrn": "37e125c6f6ec2c3f58a22cd9203cf75de40289c6be39cdbe17afd73e2d82b566", + "packages/ui/components/Tabs.wrn": "4fa0c0854700532960043290700d7cf99663ec41692068101f0aae4c2b1a1181", "packages/ui/components/TextLink.wrn": "30782039293eb36d63b7b3a4f32a71a47177a3a68c7e90184be7cf7b4385eb19", "packages/ui/components/Textarea.wrn": "ddf0b4f124b2cf0c0ab3d820d3ac0085f7c20466e977231949be264cd0cee8cf", "packages/ui/components/TimePicker.wrn": "2e8e7a90f6b6069a07e7ffd2725ba1e1031e84d55a4f1254025befbb314fa695", diff --git a/packages/csr/src/reactive-runtime.ts b/packages/csr/src/reactive-runtime.ts index 78166d30..3f7db28d 100644 --- a/packages/csr/src/reactive-runtime.ts +++ b/packages/csr/src/reactive-runtime.ts @@ -3193,40 +3193,6 @@ export const REACTIVE_RUNTIME = String.raw` syncRovingGroups(); } - /* - * Back/forward for tabs that mirror their selection into the URL. The - * runtime never touches component state -- a popstate listener assigning to - * it would write after the client function returned, and that write is - * dropped. It announces the value instead and the component applies it. - */ - function syncTabsFromUrl() { - var groups = document.querySelectorAll("[data-wrn-tabs-param]"); - for (var index = 0; index < groups.length; index += 1) { - var group = groups[index]; - var param = group.getAttribute("data-wrn-tabs-param"); - if (!param) continue; - var value = new URLSearchParams(window.location.search).get(param); - // Back past the first click lands on a URL with no parameter at all; - // the selection the component started with is the answer there. - if (value === null) value = group.getAttribute("data-wrn-tabs-default"); - if (value === null || value === "") continue; - if (!group.querySelector('[role="tab"][data-value="' + value + '"]')) continue; - // Announce the value; synthesising a click hits nodes a re-render may - // have replaced and left unbound. - group.dispatchEvent( - new CustomEvent("wrnexus:tabs:restore", { detail: { value: value } }), - ); - } - } - - function setupTabUrlSync() { - if (window.__wrnexusTabUrlBound) return; - window.__wrnexusTabUrlBound = true; - window.addEventListener("popstate", syncTabsFromUrl); - // Once after hydration, so a shared link opens on the right tab. - window.setTimeout(syncTabsFromUrl, 0); - } - function hydrateScopes(root) { var host = root || document; @@ -5445,7 +5411,6 @@ export const REACTIVE_RUNTIME = String.raw` setupAnchoredOverlays(); setupModalDialogs(); setupRovingFocus(); - setupTabUrlSync(); window.__wrnexusRepositionAnchored = repositionAnchored; window.__wrnexusHydrateScopes = hydrateScopes; window.__wrnexusInvalidateClientModule = function (url) { clientModuleCache.delete(url); }; diff --git a/packages/csr/test/reactive.test.ts b/packages/csr/test/reactive.test.ts index 01a61474..25ea5c2b 100644 --- a/packages/csr/test/reactive.test.ts +++ b/packages/csr/test/reactive.test.ts @@ -1052,72 +1052,3 @@ test("an empty or false roving attribute opts the group out entirely", () => { a.dispatchEvent(keydown(win, "ArrowRight")); expect(doc.activeElement!.id).toBe("a"); }); - -test("popstate announces the value the url names instead of clicking a tab", () => { - const win = mount( - `
-
- - -
-
`, - ); - const doc = win.document; - const group = doc.querySelector("[data-wrn-tabs-param]") as unknown as HTMLElement; - const seen: string[] = []; - group.addEventListener("wrnexus:tabs:restore", (event) => { - seen.push((event as CustomEvent).detail.value); - }); - - win.location.search = "?tab=two"; - win.dispatchEvent( - new (win as unknown as { Event: new (t: string) => unknown }).Event( - "popstate", - ) as unknown as Parameters[0], - ); - expect(seen).toEqual(["two"]); - - // Back past the first click lands on a url with no parameter at all; the - // component's starting selection is the right answer there. - win.location.search = ""; - win.dispatchEvent( - new (win as unknown as { Event: new (t: string) => unknown }).Event( - "popstate", - ) as unknown as Parameters[0], - ); - expect(seen).toEqual(["two", "one"]); -}); - -test("the url sync announces on every popstate and leaves idempotence to the component", () => { - const win = mount( - `
-
- - -
-
`, - ); - const doc = win.document; - const group = doc.querySelector("[data-wrn-tabs-param]") as unknown as HTMLElement; - const seen: string[] = []; - group.addEventListener("wrnexus:tabs:restore", (event) => { - seen.push((event as CustomEvent).detail.value); - }); - - win.location.search = "?tab=two"; - for (let i = 0; i < 3; i += 1) { - win.dispatchEvent( - new (win as unknown as { Event: new (t: string) => unknown }).Event( - "popstate", - ) as unknown as Parameters[0], - ); - } - - /* - * Deliberately not deduplicated here. Tracking what was last applied meant - * the runtime held state that drifted out of step with the component, which - * silently swallowed real changes. The component compares against its own - * selection instead, which cannot drift. - */ - expect(seen).toEqual(["two", "two", "two"]); -}); diff --git a/packages/ui/components/Tabs.wrn b/packages/ui/components/Tabs.wrn index 112b65a0..ac309768 100644 --- a/packages/ui/components/Tabs.wrn +++ b/packages/ui/components/Tabs.wrn @@ -38,15 +38,23 @@ component Tabs { return String(item.value || item.id || index) } + /* + * In url mode the query parameter is the source of truth, not local state. + * + * The client router owns popstate and swaps the whole page on back and + * forward, which discards component state anyway. Reading the URL means + * the right tab simply falls out of whatever render happens next, with no + * listener to lose and nothing to keep in step. + */ shared function currentValue() { + if (mode === "url" && typeof window !== "undefined" && window.location) { + var fromUrl = new URLSearchParams(window.location.search).get(param || "tab") + return fromUrl ? fromUrl : defaultValue() + } if (activeValue) { return activeValue } - if (active) { - return active - } - var list = itemList() - return list.length ? valueOf(list[0], 0) : "" + return defaultValue() } // The selection this instance started with. The runtime falls back to it @@ -67,34 +75,6 @@ component Tabs { return orientation === "vertical" ? "vertical" : "horizontal" } - // Back and forward arrive here. The runtime announces the value the URL - // now names; applying it must not write history, or stepping back would - // push a new entry and trap the user. - client function applyUrlValue(sourceEvent) { - var detail = sourceEvent ? sourceEvent.detail : null - var value = detail ? detail.value : "" - if (!value) { - return - } - var list = itemList() - var found = -1 - for (var index = 0; index < list.length; index += 1) { - if (valueOf(list[index], index) === value) { - found = index - } - } - if (found === -1) { - return - } - // Idempotent against our own state rather than a DOM attribute the - // re-render owns: popstate can fire for a value already selected. - if (value === currentValue()) { - return - } - activeValue = value - output.change({ value: value, item: list[found], index: found }) - } - client function selectTab(item, index, sourceEvent) { if (item.disabled) { return @@ -123,9 +103,6 @@ component Tabs { data-size='{size}' data-mode='{mode}' data-param='{param}' - data-wrn-tabs-param='{mode === "url" ? param : ""}' - data-wrn-tabs-default='{defaultValue()}' - @wrnexus:tabs:restore='applyUrlValue(event)' >
{ const root = dom.querySelector(".wire-tabs") as HTMLElement; expect(root.getAttribute("data-mode")).toBe("url"); expect(root.getAttribute("data-param")).toBe("tab"); + + /* + * In url mode the query parameter is the source of truth rather than + * component state. The client router owns popstate and swaps the whole page + * on back and forward, discarding component state, so the selection has to + * fall out of the URL for history to work at all. + */ + const source2 = readFileSync(uiComponentPath("Tabs"), "utf8"); + expect(source2).toContain("URLSearchParams(window.location.search)"); + expect(source2).toContain("history.pushState"); }); test("tabs vertical orientation switches the roving axis", async () => {