diff --git a/apps/web/src/components/settings/SettingsSidebarNav.tsx b/apps/web/src/components/settings/SettingsSidebarNav.tsx index 9547a14c8bdd..6cd1acaae63d 100644 --- a/apps/web/src/components/settings/SettingsSidebarNav.tsx +++ b/apps/web/src/components/settings/SettingsSidebarNav.tsx @@ -45,8 +45,9 @@ import { import { SidebarUtilityMenu } from "../sidebar/SidebarChrome"; import { scrollToSettingsTarget } from "./settingsLayout"; import { - getVisibleSettingsSectionIds, + getActiveSettingsSectionId, observeSettingsSectionVisibility, + type SettingsSectionVisibilityScope, type SettingsSectionVisibilityState, } from "./settingsSectionVisibility"; import { @@ -154,6 +155,10 @@ export function SettingsSidebarNav({ pathname }: { pathname: string }) { const [sectionVisibility, setSectionVisibility] = useState( null, ); + const [preferredSection, setPreferredSection] = useState<{ + scope: SettingsSectionVisibilityScope | null; + targetId: string; + } | null>(null); const searchableItems = useAvailableSettingsSearchItems(); const results = useMemo(() => searchSettings(query, searchableItems), [query, searchableItems]); const isSearching = query.trim().length > 0; @@ -169,10 +174,11 @@ export function SettingsSidebarNav({ pathname }: { pathname: string }) { const pageSections = path ? SETTINGS_PAGE_SECTIONS[path] : undefined; return path && pageSections ? { path, pageSections } : null; }, [resolvedPathname]); - const visiblePageSectionIds = getVisibleSettingsSectionIds({ + const activePageSectionId = getActiveSettingsSectionId({ activePath: activeSettingsPath, scope: observedVisibilityScope, visibility: sectionVisibility, + preferredSection, }); useEffect(() => { @@ -249,6 +255,7 @@ export function SettingsSidebarNav({ pathname }: { pathname: string }) { ); const handlePageSectionClick = useCallback( (to: SettingsPath, targetId: string) => { + setPreferredSection({ scope: observedVisibilityScope, targetId }); if (isMobile) { setOpenMobile(false); } @@ -263,7 +270,7 @@ export function SettingsSidebarNav({ pathname }: { pathname: string }) { state: { settingsTargetHighlight: false }, }); }, - [isMobile, navigate, pathname, setOpenMobile], + [isMobile, navigate, observedVisibilityScope, pathname, setOpenMobile], ); const clearSearch = useCallback(() => { setQuery(""); @@ -435,10 +442,12 @@ export function SettingsSidebarNav({ pathname }: { pathname: string }) { } size="sm" - data-visible={visiblePageSectionIds.has(section.targetId)} + aria-current={ + activePageSectionId === section.targetId ? "location" : undefined + } className={cn( "w-full text-sidebar-muted-foreground/65", - visiblePageSectionIds.has(section.targetId) && + activePageSectionId === section.targetId && "font-medium text-sidebar-foreground", )} onClick={() => handlePageSectionClick(item.to, section.targetId)} diff --git a/apps/web/src/components/settings/settingsLayout.test.tsx b/apps/web/src/components/settings/settingsLayout.test.tsx index 714a77dbb562..45408ac2faa4 100644 --- a/apps/web/src/components/settings/settingsLayout.test.tsx +++ b/apps/web/src/components/settings/settingsLayout.test.tsx @@ -25,7 +25,7 @@ describe("settings search targets", () => { expect(markup).not.toContain("settings-search-target-pulse"); }); - it("scrolls directly to a section header and restarts the destination pulse", () => { + it.each([true, false])("scrolls to a section header with highlight=%s", (highlight) => { const sectionScrollIntoView = vi.fn(); const headerScrollIntoView = vi.fn(); const focus = vi.fn(); @@ -48,16 +48,21 @@ describe("settings search targets", () => { matchMedia: vi.fn(() => ({ matches: false })), }); - expect(scrollToSettingsTarget("providers")).toBe(true); + expect(scrollToSettingsTarget("providers", { highlight })).toBe(true); expect(headerScrollIntoView).toHaveBeenCalledWith({ behavior: "smooth", - block: "center", + block: highlight ? "center" : "start", }); expect(sectionScrollIntoView).not.toHaveBeenCalled(); expect(focus).toHaveBeenCalledWith({ preventScroll: true }); expect(remove).toHaveBeenCalledWith("settings-search-target-pulse"); - expect(add).toHaveBeenCalledWith("settings-search-target-pulse"); - expect(addEventListener).toHaveBeenCalledWith("blur", expect.any(Function), { once: true }); + if (highlight) { + expect(add).toHaveBeenCalledWith("settings-search-target-pulse"); + expect(addEventListener).toHaveBeenCalledWith("blur", expect.any(Function), { once: true }); + } else { + expect(add).not.toHaveBeenCalled(); + expect(addEventListener).not.toHaveBeenCalled(); + } }); it("does not animate the destination when reduced motion is requested", () => { diff --git a/apps/web/src/components/settings/settingsLayout.tsx b/apps/web/src/components/settings/settingsLayout.tsx index 246607340751..cedb774dd6ad 100644 --- a/apps/web/src/components/settings/settingsLayout.tsx +++ b/apps/web/src/components/settings/settingsLayout.tsx @@ -71,7 +71,7 @@ function scrollAndFocusSettingsTarget(target: HTMLElement, highlight = true): vo scrollTarget.scrollIntoView({ behavior: prefersReducedMotion ? "auto" : "smooth", - block: "center", + block: highlight ? "center" : "start", }); target.focus({ preventScroll: true }); target.classList.remove("settings-search-target-pulse"); diff --git a/apps/web/src/components/settings/settingsSectionVisibility.test.ts b/apps/web/src/components/settings/settingsSectionVisibility.test.ts index 90656ad7073a..8e50334c2b8b 100644 --- a/apps/web/src/components/settings/settingsSectionVisibility.test.ts +++ b/apps/web/src/components/settings/settingsSectionVisibility.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it, vi } from "vite-plus/test"; import { - getVisibleSettingsSectionIds, + getActiveSettingsSectionId, observeSettingsSectionVisibility, type SettingsSectionVisibilityEnvironment, } from "./settingsSectionVisibility"; @@ -83,28 +83,54 @@ describe("settings section visibility", () => { }; expect( - getVisibleSettingsSectionIds({ + getActiveSettingsSectionId({ activePath: "/settings/general", scope: firstGeneralVisit, visibility: firstVisibility, }), - ).toEqual(new Set(["text-generation"])); + ).toBe("text-generation"); expect( - getVisibleSettingsSectionIds({ + getActiveSettingsSectionId({ activePath: "/settings/providers", scope: null, visibility: firstVisibility, }), - ).toEqual(new Set()); + ).toBeUndefined(); const secondGeneralVisit = { path: "/settings/general" }; expect( - getVisibleSettingsSectionIds({ + getActiveSettingsSectionId({ activePath: "/settings/general", scope: secondGeneralVisit, visibility: firstVisibility, }), - ).toEqual(new Set()); + ).toBeUndefined(); + }); + + it("selects one visible section, preferring the clicked destination within the current visit", () => { + const scope = { path: "/settings/general" }; + const state = { + activePath: scope.path, + scope, + visibility: { + scope, + targetIds: new Set(["behavior", "projects-and-threads", "confirmations"]), + }, + }; + const preferredSection = { scope, targetId: "projects-and-threads" }; + + expect(getActiveSettingsSectionId(state)).toBe("behavior"); + expect(getActiveSettingsSectionId({ ...state, preferredSection })).toBe("projects-and-threads"); + expect( + getActiveSettingsSectionId({ + ...state, + preferredSection: { ...preferredSection, scope: { path: scope.path } }, + }), + ).toBe("behavior"); + state.visibility.targetIds.delete("projects-and-threads"); + expect(getActiveSettingsSectionId({ ...state, preferredSection })).toBe("behavior"); + state.visibility.targetIds.clear(); + expect(getActiveSettingsSectionId({ ...state, preferredSection })).toBeUndefined(); }); it("accumulates visible sections and emits them in sidebar order", () => { diff --git a/apps/web/src/components/settings/settingsSectionVisibility.ts b/apps/web/src/components/settings/settingsSectionVisibility.ts index 76b1fc943256..5bc6b3f177af 100644 --- a/apps/web/src/components/settings/settingsSectionVisibility.ts +++ b/apps/web/src/components/settings/settingsSectionVisibility.ts @@ -12,21 +12,27 @@ export type SettingsSectionVisibilityState = { readonly targetIds: ReadonlySet; }; -const EMPTY_VISIBLE_SETTINGS_SECTION_IDS: ReadonlySet = new Set(); - -export function getVisibleSettingsSectionIds({ +export function getActiveSettingsSectionId({ activePath, scope, visibility, + preferredSection, }: { readonly activePath: string | undefined; readonly scope: SettingsSectionVisibilityScope | null; readonly visibility: SettingsSectionVisibilityState | null; -}): ReadonlySet { + readonly preferredSection?: { + readonly scope: SettingsSectionVisibilityScope | null; + readonly targetId: string; + } | null; +}): string | undefined { if (!scope || activePath !== scope.path || visibility?.scope !== scope) { - return EMPTY_VISIBLE_SETTINGS_SECTION_IDS; + return undefined; + } + if (preferredSection?.scope === scope && visibility.targetIds.has(preferredSection.targetId)) { + return preferredSection.targetId; } - return visibility.targetIds; + return visibility.targetIds.values().next().value; } type ElementObserver = {