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)' >