diff --git a/.changeset/settings-ui-consistency-and-search.md b/.changeset/settings-ui-consistency-and-search.md new file mode 100644 index 0000000000..82ae2b5752 --- /dev/null +++ b/.changeset/settings-ui-consistency-and-search.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": minor +--- + +summary: Settings search now finds and jumps to individual settings, and settings screens share one type scale. +category: feature +dev: Sections render through the shared `settings/` row primitives (`SettingsToggleRow`/`SelectRow`/`NumberRow`/`TextRow`/`TextareaRow`) instead of hand-rolled `form-group`/`checkbox-label` markup; global `.form-group` is unchanged for the 35 non-settings files that use it. Search is indexed from per-section `Section.search.ts` entries aggregated in `settings/search/entries.ts`, replacing the hand-curated `searchableText` keyword arrays as the primary match path (keywords remain a fallback for unmigrated sections). `settings-search-index.test.ts` fails the build when a rendered descriptor `key` is missing from the index. Adds the missing `--font-size-sm`/`--font-size-md` tokens plus `2xs`/`lg`, which were referenced by 12 declarations but never defined. diff --git a/packages/dashboard/app/__tests__/settings-primitives.test.tsx b/packages/dashboard/app/__tests__/settings-primitives.test.tsx index 48c12dc0b6..6b7c0a2165 100644 --- a/packages/dashboard/app/__tests__/settings-primitives.test.tsx +++ b/packages/dashboard/app/__tests__/settings-primitives.test.tsx @@ -38,6 +38,127 @@ describe("SettingsFieldRow", () => { expect(screen.getByRole("alert")).toHaveTextContent("Required"); }); + /* + FNXC:SettingsHelp 2026-07-15-21:10: + Help is deferred behind a "?" beside the label, so these pin the parts that are easy to break silently: + - the trigger exists and toggles (click is the ONLY interaction a touch device has — a hover-only tip is invisible on mobile, and this suite runs in jsdom where hover cannot be simulated anyway); + - the copy stays in the DOM and reachable via `aria-describedby` while closed, because deferring it visually must not remove it from assistive tech, in-page find, or the settings search index; + - the error band is NOT deferred. + */ + it("puts help behind a trigger that toggles on click", () => { + render( + + + , + ); + const trigger = screen.getByRole("button", { name: "Show help" }); + const tip = trigger.closest(".settings-help")!; + + expect(tip).toHaveAttribute("data-open", "false"); + expect(trigger).toHaveAttribute("aria-expanded", "false"); + + fireEvent.click(trigger); + expect(tip).toHaveAttribute("data-open", "true"); + expect(trigger).toHaveAttribute("aria-expanded", "true"); + + fireEvent.click(trigger); + expect(tip).toHaveAttribute("data-open", "false"); + }); + + it("keeps help copy in the accessibility tree while collapsed", () => { + render( + + + , + ); + const trigger = screen.getByRole("button", { name: "Show help" }); + const describedBy = trigger.getAttribute("aria-describedby")!; + const bubble = document.getElementById(describedBy); + + // Present and readable even though the row is collapsed — not display:none. + expect(bubble).not.toBeNull(); + expect(bubble).toHaveTextContent("Pick a theme"); + expect(screen.getByText("Pick a theme")).toBeInTheDocument(); + }); + + it("closes an open tip on Escape", () => { + render( + + + , + ); + const trigger = screen.getByRole("button", { name: "Show help" }); + fireEvent.click(trigger); + expect(trigger.closest(".settings-help")).toHaveAttribute("data-open", "true"); + + fireEvent.keyDown(document, { key: "Escape" }); + expect(trigger.closest(".settings-help")).toHaveAttribute("data-open", "false"); + }); + + it("closes an open tip when pointing elsewhere, so a tap on mobile cannot strand it", () => { + render( + + + , + ); + const trigger = screen.getByRole("button", { name: "Show help" }); + fireEvent.click(trigger); + expect(trigger.closest(".settings-help")).toHaveAttribute("data-open", "true"); + + fireEvent.pointerDown(document.body); + expect(trigger.closest(".settings-help")).toHaveAttribute("data-open", "false"); + }); + + /* + FNXC:SettingsHelp 2026-07-15-22:25: + Only one tip may be open. The outside-pointerdown handler is not sufficient on its own: `click` fires with NO pointer event when a keyboard operator presses Enter/Space on a focused trigger, so opening a second tip that way used to leave the first bubble open underneath it — observed as two overlapping bubbles on a phone-sized viewport. + Asserted with a bare `click()` precisely because that is the no-pointerdown path. + */ + it("closes any other open tip when one opens, including without a pointer event", () => { + render( + <> + + + + + + + , + ); + const [alphaBtn, betaBtn] = screen.getAllByRole("button", { name: "Show help" }); + const alpha = alphaBtn.closest(".settings-help")!; + const beta = betaBtn.closest(".settings-help")!; + + fireEvent.click(alphaBtn); + expect(alpha).toHaveAttribute("data-open", "true"); + + // No pointerdown — the keyboard path. + fireEvent.click(betaBtn); + expect(beta).toHaveAttribute("data-open", "true"); + expect(alpha).toHaveAttribute("data-open", "false"); + }); + + it("renders no help trigger when a row has no help", () => { + render( + + + , + ); + expect(screen.queryByRole("button", { name: "Show help" })).not.toBeInTheDocument(); + }); + + it("keeps the error band inline rather than behind the help trigger", () => { + render( + + + , + ); + // A validation message the operator must go looking for is one they will not see. + const alert = screen.getByRole("alert"); + expect(alert).toHaveTextContent("Required"); + expect(alert.closest(".settings-help")).toBeNull(); + }); + it("renders a scope badge when scope is set", () => { render( @@ -193,6 +314,57 @@ describe("SettingsTextRow", () => { expect(typeof onChange.mock.calls[0][0]).toBe("string"); }); + /* + FNXC:SettingsSecurity 2026-07-15-18:52: + Masking is asserted, not assumed. This primitive hardcoded `type="text"`, which is why every token row (ntfy access token, GitHub/GitLab tokens, Cloudflare tunnel token) had to stay hand-rolled to avoid rendering a stored secret in plain text. + A regression here would not throw and would not look broken in review — the field simply renders the token — so it is pinned by a test rather than left to a reviewer noticing a missing prop. + */ + it("defaults to a text input", () => { + render( {}} />); + expect(screen.getByRole("textbox")).toHaveAttribute("type", "text"); + }); + + it("masks a password row and suppresses autofill by default", () => { + render( + {}} + />, + ); + // A password input is deliberately not exposed with the textbox role. + const input = document.querySelector("#apiToken") as HTMLInputElement; + expect(input).toHaveAttribute("type", "password"); + expect(input).toHaveValue("tk_secret"); + // Without this a browser offers to save the operator's API token. + expect(input).toHaveAttribute("autocomplete", "off"); + }); + + it("lets a descriptor override autocomplete on a password row", () => { + render( + {}} + />, + ); + expect(document.querySelector("#apiToken")).toHaveAttribute("autocomplete", "new-password"); + }); + + it("renders a url row without forcing autocomplete off", () => { + render( + {}} + />, + ); + const input = screen.getByRole("textbox"); + expect(input).toHaveAttribute("type", "url"); + // autocomplete suppression is a secret-bearing concern, not a URL one. + expect(input).not.toHaveAttribute("autocomplete"); + }); + it("emits null when cleared", () => { const onChange = vi.fn(); render(); diff --git a/packages/dashboard/app/__tests__/settings-save-split.test.ts b/packages/dashboard/app/__tests__/settings-save-split.test.ts index 1f6638320a..e0425c1493 100644 --- a/packages/dashboard/app/__tests__/settings-save-split.test.ts +++ b/packages/dashboard/app/__tests__/settings-save-split.test.ts @@ -268,14 +268,14 @@ describe("splitSettingsSave", () => { payload, initialValues: null, initialScopedValues, - activeSection: "global-general", + activeSection: "source-control-global", }); expect(globalPatch).toEqual({ gitlabEnabled: false, gitlabAuthToken: "global-token", gitlabAuthTokenType: "group" }); expect(projectPatch).toEqual({}); }); - it("routes GitLab enable and token settings to project settings outside global general", () => { + it("routes GitLab enable and token settings to project settings outside the global source-control section", () => { const initialScopedValues = { global: { gitlabEnabled: false, gitlabAuthToken: "global-token", gitlabAuthTokenType: "group" }, project: { gitlabEnabled: true }, @@ -291,7 +291,7 @@ describe("splitSettingsSave", () => { payload, initialValues: null, initialScopedValues, - activeSection: "merge", + activeSection: "source-control", }); expect(globalPatch).toEqual({}); @@ -319,7 +319,7 @@ describe("splitSettingsSave", () => { // genuine global edit. initialValues: { gitlabEnabled: true } as never, initialScopedValues, - activeSection: "global-general", + activeSection: "source-control-global", }); expect(globalPatch).toEqual({ gitlabEnabled: true }); @@ -336,7 +336,7 @@ describe("splitSettingsSave", () => { payload: { gitlabEnabled: true }, initialValues: { gitlabEnabled: true } as never, initialScopedValues, - activeSection: "global-general", + activeSection: "source-control-global", }); expect(globalPatch).toEqual({}); @@ -354,7 +354,7 @@ describe("splitSettingsSave", () => { // is explicitly present-but-undefined (unset) — the edit must still land. initialValues: { gitlabEnabled: true } as never, initialScopedValues, - activeSection: "global-general", + activeSection: "source-control-global", }); expect(globalPatch).toEqual({ gitlabEnabled: true }); @@ -375,7 +375,7 @@ describe("splitSettingsSave", () => { payload, initialValues: null, initialScopedValues, - activeSection: "merge", + activeSection: "source-control", }); expect(projectPatch).toEqual({ gitlabAuthToken: null, gitlabAuthTokenType: "personal" }); @@ -673,7 +673,7 @@ describe("splitSettingsSave", () => { payload, initialValues: {} as never, initialScopedValues: { global: {}, project: {} } as never, - activeSection: "global-general", + activeSection: "source-control-global", }); expect(onGlobal.globalPatch).toMatchObject(payload); expect("gitlabInstanceUrl" in onGlobal.projectPatch).toBe(false); @@ -683,7 +683,7 @@ describe("splitSettingsSave", () => { payload, initialValues: {} as never, initialScopedValues: { global: {}, project: {} } as never, - activeSection: "general", + activeSection: "source-control", }); expect("gitlabInstanceUrl" in onProject.globalPatch).toBe(false); expect("gitlabApiBaseUrl" in onProject.globalPatch).toBe(false); @@ -700,7 +700,7 @@ describe("splitSettingsSave", () => { global: { gitlabInstanceUrl: "https://global.example", gitlabApiBaseUrl: "https://global.example/api/v4" }, project: {}, } as never, - activeSection: "global-general", + activeSection: "source-control-global", }); expect(onGlobal.globalPatch).toEqual({ gitlabInstanceUrl: null, gitlabApiBaseUrl: null }); @@ -711,18 +711,18 @@ describe("splitSettingsSave", () => { global: {}, project: { gitlabInstanceUrl: "https://project.example", gitlabApiBaseUrl: "https://project.example/api/v4" }, } as never, - activeSection: "general", + activeSection: "source-control", }); expect(onProject.projectPatch).toEqual({ gitlabInstanceUrl: null, gitlabApiBaseUrl: null }); }); - it("routes githubTrackingDefaultRepo to global only on the global-general section", () => { + it("routes githubTrackingDefaultRepo to global only on the source-control-global section", () => { const payloadGlobal: Record = { githubTrackingDefaultRepo: "org/repo" }; const onGlobal = splitSettingsSave({ payload: payloadGlobal, initialValues: {} as never, initialScopedValues: { global: {}, project: {} } as never, - activeSection: "global-general", + activeSection: "source-control-global", }); expect(onGlobal.globalPatch).toMatchObject({ githubTrackingDefaultRepo: "org/repo" }); expect("githubTrackingDefaultRepo" in onGlobal.projectPatch).toBe(false); @@ -731,11 +731,11 @@ describe("splitSettingsSave", () => { payload: { githubTrackingDefaultRepo: "org/repo" }, initialValues: {} as never, initialScopedValues: { global: {}, project: {} } as never, - activeSection: "general", + activeSection: "source-control", }); expect("githubTrackingDefaultRepo" in onProject.globalPatch).toBe(false); // ...and is instead routed to the project patch on the project-scoped - // "general" section, rather than being dropped or erroring. + // "source-control" section, rather than being dropped or erroring. expect(onProject.projectPatch).toMatchObject({ githubTrackingDefaultRepo: "org/repo" }); }); }); diff --git a/packages/dashboard/app/__tests__/settings-sections.test.tsx b/packages/dashboard/app/__tests__/settings-sections.test.tsx index 1be673362c..9a2cb26a59 100644 --- a/packages/dashboard/app/__tests__/settings-sections.test.tsx +++ b/packages/dashboard/app/__tests__/settings-sections.test.tsx @@ -93,7 +93,6 @@ describe("AppearanceSection", () => { const [hidden, setHidden] = useState(false); return ( { it("round-trips the session-banner toggle through its setter", () => { render(); - const toggle = screen.getByText("Hide AI session notification banners") - .closest("label")! - .querySelector("input[type=checkbox]") as HTMLInputElement; + /* + FNXC:SettingsStyling 2026-07-15-17:35: + Resolved through the label→control association rather than by walking the DOM. The old `getByText(...).closest("label").querySelector("input")` assumed the label ELEMENT wrapped the checkbox, which was the `checkbox-label` markup's shape; the shared row primitive binds `htmlFor`/`id` instead, so the label is now a sibling of the control and the walk returned null. + `getByLabelText` asserts the binding an assistive technology actually uses, so it survives markup changes and additionally fails if that binding is ever broken — which the DOM walk could not detect. + */ + const toggle = screen.getByLabelText("Hide AI session notification banners") as HTMLInputElement; expect(toggle.checked).toBe(false); fireEvent.click(toggle); expect(toggle.checked).toBe(true); @@ -128,7 +130,6 @@ describe("GeneralSection", () => { render( { const [form, setForm] = useState({ allowAbsoluteFileBrowserPaths: false } as SettingsFormState); return ( { const setForm = vi.fn(); render( { it("nests the failure-notification mode field inside the padded provider body", () => { render( { it("shows the ntfy topic field only when ntfy is enabled", () => { const { rerender } = render( { expect(screen.queryByLabelText("ntfy Topic")).not.toBeInTheDocument(); rerender( { }); describe("SecretsSection", () => { - it("renders the scope banner, title, and the SecretsView card", () => { + /* + FNXC:SettingsScope 2026-07-15-18:52: + The scope-banner assertion went with the banner itself: sections no longer take a `scopeBanner` slot, because one section-level scope claim was false wherever a section mixed scopes. Scope now rides on each row's badge. + The rest of the contract — title plus the SecretsView card — is unchanged and still asserted. + */ + it("renders the title and the SecretsView card", () => { render( - } addToast={vi.fn()} />, + , ); - expect(screen.getByTestId("scope-banner")).toBeInTheDocument(); expect(screen.getByText("Secrets")).toBeInTheDocument(); expect(screen.getByTestId("secrets-view")).toBeInTheDocument(); }); @@ -264,7 +264,6 @@ describe("WorktreesSection", () => { const onAdd = vi.fn(); render( { render( { it("keeps an empty copy-file row reachable when the setting is undefined", () => { render( { const updateLaneThinkingValue = vi.fn(); render( { } as SettingsFormState); return ( { it("opts Project Models lane and preset dropdowns into readable menu width", () => { render( { it("colocates summarization model controls with AI summarization settings", () => { render( { it("keeps summarization controls behind the available-models guard", () => { render( { const resetLaneValue = vi.fn(); render( { } as SettingsFormState); return ( { render( { render( { render( { } as SettingsFormState); return ( { describe("PromptsSection", () => { it("renders the title and mounts AgentPromptsManager", () => { render( - , + , ); expect(screen.getByText("Prompts")).toBeInTheDocument(); expect(screen.getByTestId("agent-prompts-manager")).toBeInTheDocument(); @@ -859,7 +845,6 @@ describe("ExperimentalSection", () => { ); return ( :is(.settings-scope-banner, .settings-gitlab-disclosure) { +.settings-content > .settings-gitlab-disclosure { margin-inline: 0; } @@ -917,22 +972,74 @@ The simplified Settings surface needs one consistent reading rhythm across legac } .settings-content h4.settings-section-heading { - font-size: 0.95rem; + font-size: var(--font-size-base); margin-bottom: var(--space-md); } .settings-content h5.settings-section-heading { - font-size: 0.82rem; + font-size: var(--font-size-sm); } +/* +FNXC:SettingsStyling 2026-07-15-20:30: +Bespoke rows keep the same vertical rhythm as migrated ones. +`.settings-field-row` spaces itself with `padding-block: var(--space-sm)`, while a `.form-group` spaced itself with `margin-top: var(--space-md)`. Mixed in one section — which is now the normal case, since some rows deliberately stay bespoke — that produced three different gaps depending on which idioms happened to be adjacent: 16px between two migrated rows, 12px between two bespoke ones, and 20px where they met. +Matching the primitive's `padding-block` (rather than giving the primitive a margin) keeps one idiom: every settings row owns its own space and margins never collapse or compound between them. +`padding-inline: 0` stays — `.settings-content` already owns the horizontal inset, and `.form-group`'s global `0 var(--space-xl)` would double-indent every bespoke row. +*/ .settings-content .form-group { padding-inline: 0; - margin-top: var(--space-md); + margin-top: 0; + padding-block: var(--space-sm); } +/* +FNXC:SettingsStyling 2026-07-15-20:30: +Every label in Settings reads the same way, whether its row is migrated or still bespoke. +Some rows deliberately stay hand-rolled — a `data-testid` the primitives have no slot for, help copy that interleaves `` or a link, a repeating-row editor — and those inherit the GLOBAL `.form-group label` treatment: 12px, weight 600, uppercase, letter-spaced, muted. So a section rendered "PLAN APPROVAL MODE" in caps directly above "Auto-merge conflict retries" in sentence case. Two idioms on one screen is the exact inconsistency the migration set out to remove, and migrating every last row is not achievable (nor desirable — see the bespoke rows' own FNXC notes). +Retuning them here instead of at the global rule is deliberate: `.form-group` is dashboard-wide across 35 non-settings files, and its uppercase treatment is that context's convention, not a bug. This selector is settings-scoped, so it changes Settings only. +Declarations mirror `.settings-field-row-label` — one contract, expressed at whatever specificity each markup idiom needs. +*/ .settings-content .form-group label:not(.checkbox-label) { - font-size: 0.72rem; - line-height: 1.35; + font-size: var(--font-size-sm); + font-weight: 500; + line-height: var(--line-height-tight); + color: var(--text); + text-transform: none; + letter-spacing: normal; + margin-bottom: var(--space-xs); +} + +/* +FNXC:SettingsStyling 2026-07-15-20:30: +Bespoke help text (`` inside a form-group) matches `.settings-field-row-help`: same rung, same colour, same measure. Without this a bespoke row's help sat at 12px/1.4 beside a migrated row's at 12.8px/1.5 — close enough to look like a rendering glitch rather than a choice. +Still applies to the copy that legitimately stays inline: validation messages (`.field-error`) and the explanatory blurbs that describe a whole block rather than one control. +*/ +.settings-content .form-group small, +.settings-content .form-group .form-text { + font-size: var(--font-size-xs); + line-height: 1.5; + color: var(--text-muted); +} + +/* +FNXC:SettingsHelp 2026-07-15-21:40: +The label line for a row that is NOT on the shared primitive, so its help can hang off a "?" exactly like a migrated row's. +A bespoke row is `