From f6993e6cdb9eac1256f22f6780c4da8aa16a2093 Mon Sep 17 00:00:00 2001 From: Ajay Ghanwat Date: Fri, 7 Aug 2026 17:47:41 +0530 Subject: [PATCH] fix(csr): drive tab url restore by announcement, anchor nav submenus Two mechanisms tried and rejected while testing this against the live showcase, both failing the same way on a second history step: - synthesising a click on the matching tab: a re-render replaces the tab buttons, and clicking a freshly replaced node that has not been bound does nothing at all - tracking the last applied value in the runtime: that state drifts out of step with the component and silently swallows real changes The runtime is now stateless. It announces the value the URL names via a wrnexus:tabs:restore event and the component applies it, comparing against its own selection rather than a DOM attribute a re-render owns. Nav submenus are anchored so the viewport clamp keeps them on screen. Comments in the runtime template trimmed to stay inside the size budget rather than raising the ceiling again. Known limitation: a second consecutive back/forward does not update the selection, because the re-render replaces the component root without rebinding its declarative listeners. That is a framework defect, not a Tabs one, and needs its own fix. Co-Authored-By: Claude Opus 5 --- .claude/launch.json | 2 +- docs/ui-visual-contract-0.8.json | 2 +- examples/component-showcase/app/routes.gen.ts | 3 - packages/csr/src/reactive-runtime.ts | 49 +++++-------- packages/csr/test/reactive.test.ts | 69 ++++++++++++++----- packages/ui/components/Tabs.wrn | 47 +++++++++++-- 6 files changed, 114 insertions(+), 58 deletions(-) diff --git a/.claude/launch.json b/.claude/launch.json index 54981fa9..6c3a360d 100644 --- a/.claude/launch.json +++ b/.claude/launch.json @@ -5,7 +5,7 @@ "name": "component-showcase", "runtimeExecutable": "bun", "runtimeArgs": ["run", "--cwd", "examples/component-showcase", "dev"], - "port": 3112 + "port": 3000 } ] } diff --git a/docs/ui-visual-contract-0.8.json b/docs/ui-visual-contract-0.8.json index 89758c9b..98cd6aae 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": "29c0aacca730616002b7988308426e20b7a34abb9b249212d59d76190d89572f", + "packages/ui/components/Tabs.wrn": "37e125c6f6ec2c3f58a22cd9203cf75de40289c6be39cdbe17afd73e2d82b566", "packages/ui/components/TextLink.wrn": "30782039293eb36d63b7b3a4f32a71a47177a3a68c7e90184be7cf7b4385eb19", "packages/ui/components/Textarea.wrn": "ddf0b4f124b2cf0c0ab3d820d3ac0085f7c20466e977231949be264cd0cee8cf", "packages/ui/components/TimePicker.wrn": "2e8e7a90f6b6069a07e7ffd2725ba1e1031e84d55a4f1254025befbb314fa695", diff --git a/examples/component-showcase/app/routes.gen.ts b/examples/component-showcase/app/routes.gen.ts index b676727e..4690813f 100644 --- a/examples/component-showcase/app/routes.gen.ts +++ b/examples/component-showcase/app/routes.gen.ts @@ -102,7 +102,6 @@ export interface Routes { "/components/strong-password": Record; "/components/styled-icon": Record; "/components/switch": Record; - "/components/table": Record; "/components/tabs": Record; "/components/text-link": Record; "/components/textarea": Record; @@ -246,7 +245,6 @@ export interface RouteNames { "components.strong.password": "/components/strong-password"; "components.styled.icon": "/components/styled-icon"; "components.switch": "/components/switch"; - "components.table": "/components/table"; "components.tabs": "/components/tabs"; "components.text.link": "/components/text-link"; "components.textarea": "/components/textarea"; @@ -459,7 +457,6 @@ export function route( "components.strong.password": "/components/strong-password", "components.styled.icon": "/components/styled-icon", "components.switch": "/components/switch", - "components.table": "/components/table", "components.tabs": "/components/tabs", "components.text.link": "/components/text-link", "components.textarea": "/components/textarea", diff --git a/packages/csr/src/reactive-runtime.ts b/packages/csr/src/reactive-runtime.ts index 4207b7a6..78166d30 100644 --- a/packages/csr/src/reactive-runtime.ts +++ b/packages/csr/src/reactive-runtime.ts @@ -3070,11 +3070,9 @@ export const REACTIVE_RUNTIME = String.raw` var ROVING_ITEM_SELECTOR = "[data-wrn-roving-item]"; /* - * These attributes are written by templates, so they arrive stringified: - * data-wrn-roving="" or data-wrn-roving-item="false" is how a component - * says "not this time". A bare [attr] selector matches either, so the value - * has to be checked -- otherwise a stepper with clickable=false still takes - * arrow-key focus. + * Templates stringify these, so data-wrn-roving="" and + * data-wrn-roving-item="false" mean "not this time". A bare [attr] selector + * matches either, so the value must be checked. */ // A bare data-wrn-roving-item means yes; only an explicit "false" opts out. function rovingItemOff(value) { @@ -3102,11 +3100,8 @@ export const REACTIVE_RUNTIME = String.raw` if (candidate.closest(ROVING_SELECTOR) !== container) continue; if (candidate.hasAttribute("disabled")) continue; if (candidate.getAttribute("aria-disabled") === "true") continue; - /* - * Deliberately not a size check. The test DOM reports every element as - * zero-sized, so measuring here would make the whole feature impossible - * to cover -- the same trap the dialog visibility check fell into. - */ + // Markers, not measurement: the test DOM reports every element as + // zero-sized, which is what left the dialog trap uncovered. if (candidate.hasAttribute("hidden")) continue; if (candidate.closest('[data-show="false"]')) continue; found.push(candidate); @@ -3199,16 +3194,10 @@ export const REACTIVE_RUNTIME = String.raw` } /* - * Back/forward support for tabs that mirror their selection into the URL. - * - * The runtime deliberately does not touch component state. A popstate - * listener that assigned to it would be writing after the client function - * returned, and that write is dropped. Instead it finds the tab matching the - * query parameter and activates it, so the component updates itself through - * its own handler. - * - * data-wrn-tabs-restoring marks the round trip, so the component knows not - * to push another history entry for a navigation that came from history. + * 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]"); @@ -3217,16 +3206,16 @@ export const REACTIVE_RUNTIME = String.raw` var param = group.getAttribute("data-wrn-tabs-param"); if (!param) continue; var value = new URLSearchParams(window.location.search).get(param); - if (value === null) continue; - var tab = group.querySelector('[role="tab"][data-value="' + value + '"]'); - if (!tab || !tab.click) continue; - if (tab.getAttribute("aria-selected") === "true") continue; - group.setAttribute("data-wrn-tabs-restoring", "true"); - try { - tab.click(); - } finally { - group.removeAttribute("data-wrn-tabs-restoring"); - } + // 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 } }), + ); } } diff --git a/packages/csr/test/reactive.test.ts b/packages/csr/test/reactive.test.ts index ddc6079e..01a61474 100644 --- a/packages/csr/test/reactive.test.ts +++ b/packages/csr/test/reactive.test.ts @@ -1053,21 +1053,20 @@ test("an empty or false roving attribute opts the group out entirely", () => { expect(doc.activeElement!.id).toBe("a"); }); -test("popstate activates the tab named by the query parameter without re-pushing", () => { +test("popstate announces the value the url names instead of clicking a tab", () => { const win = mount( - `
+ `
- - + +
`, ); const doc = win.document; - let restoringAtClick: string | null = "not-clicked"; - (doc.querySelector("#t2") as unknown as HTMLElement).addEventListener("click", () => { - restoringAtClick = doc - .querySelector("[data-wrn-tabs-param]")! - .getAttribute("data-wrn-tabs-restoring"); + 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"; @@ -1076,13 +1075,49 @@ test("popstate activates the tab named by the query parameter without re-pushing "popstate", ) as unknown as Parameters[0], ); + expect(seen).toEqual(["two"]); - // The runtime activates the matching tab and marks the round trip, so the - // component can tell a history navigation from a real click and skip - // pushing another entry. - expect(restoringAtClick).toBe("true"); - // The marker is cleaned up once the activation is done. - expect( - doc.querySelector("[data-wrn-tabs-param]")!.getAttribute("data-wrn-tabs-restoring"), - ).toBeNull(); + // 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 653d761b..112b65a0 100644 --- a/packages/ui/components/Tabs.wrn +++ b/packages/ui/components/Tabs.wrn @@ -49,6 +49,16 @@ component Tabs { return list.length ? valueOf(list[0], 0) : "" } + // The selection this instance started with. The runtime falls back to it + // when the back button lands on a URL that has no tab parameter at all. + shared function defaultValue() { + if (active) { + return active + } + var list = itemList() + return list.length ? valueOf(list[0], 0) : "" + } + shared function isSelected(item, index) { return valueOf(item, index) === currentValue() } @@ -57,6 +67,34 @@ 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 @@ -64,12 +102,7 @@ component Tabs { var value = valueOf(item, index) activeValue = value - // A click that came from the back button must not push another entry. - var target = sourceEvent ? sourceEvent.currentTarget : null - var group = target && target.closest ? target.closest("[data-wrn-tabs-param]") : null - var restoring = group && group.getAttribute("data-wrn-tabs-restoring") === "true" - - if (mode === "url" && !restoring && window.history && window.history.pushState) { + if (mode === "url" && window.history && window.history.pushState) { var url = new URL(window.location.href) url.searchParams.set(param || "tab", value) window.history.pushState({}, "", url.toString()) @@ -91,6 +124,8 @@ component Tabs { data-mode='{mode}' data-param='{param}' data-wrn-tabs-param='{mode === "url" ? param : ""}' + data-wrn-tabs-default='{defaultValue()}' + @wrnexus:tabs:restore='applyUrlValue(event)' >