fix(web): hide archive confirmation in settings with new sidebar - #6226
ipanasenko wants to merge 4 commits into
UI Consistency: 1 issue found
apps/web/src/components/settings/SettingsPanels.tsx — The archive-confirmation row is now hidden (and removed from the settings search index) when the legacy sidebar is off, but confirmThreadArchive is still honored by the default sidebar (Sidebar.tsx archive menu action) and by the chat header thread menu (useThreadActionMenu). The confirmation dialog therefore stays active with no visible control and no search result to disable it, which strands users who enabled it under the legacy sidebar.
Suggested smallest fix: keep the row rendered (and drop requiresLegacySidebar), or scope the runtime behavior to the legacy sidebar as well so the control and the behavior disappear together. If the row remains conditional, gate it on the shared useLegacySidebarEnabled() hook — as SettingsSidebarNav does — instead of the raw settings.legacySidebarEnabled value.
Details
Note
Your check run agent prompt is: .macroscope/check-run-agents/ui-consistency.md
More information about how Check Run Agents work can be found in our Help Center.
Files examined: apps/web/src/components/settings/SettingsPanels.tsx, apps/web/src/components/settings/SettingsSidebarNav.tsx, apps/web/src/components/settings/settingsSearch.ts (+ test), plus consumers traced outside the diff: apps/web/src/components/Sidebar.tsx, apps/web/src/components/LegacySidebar.tsx, apps/web/src/hooks/useThreadActionMenu.ts, apps/web/src/components/chat/ChatHeader.tsx, apps/web/src/hooks/useSettings.ts, packages/contracts/src/settings.ts.
Not flagged:
clampSettingsSearchResultIndexis correct for the empty-result case (Math.min(0, -1)clamped to0) and is covered by the added tests.- Replacing the functional
setActiveResultIndexupdaters with the clamped value in the arrow-key handler is behaviorally equivalent for discrete keyboard events; no keyboard regression identified. - No shared-primitive, Tailwind ownership, theme-variant, scroll/mask, or accessibility changes are introduced by this diff (
aria-label,aria-activedescendant,aria-selected, androle="option"wiring are preserved and consistently use the resolved index).
🔍 Agent activity — 1 tool call
💭 Looking at the clamping logic for the settings search index and how it handles edge cases like when there are no results. I'm also checking whether hidden settings that don't appear in the UI are still being included in the search results, which could cause the index to point to items that aren't actually visible. Now I need to examine the actual component files to see if there are UI consistency problems — like a settings section becoming empty after filtering, leaving orphaned headings or spacing issues. I should also verify that the
useLegacySidebarEnabledhook is properly defined and being used consistently. I need to check the useLegacySidebarEnabled hook and the legacySidebarEnabled setting in the SettingsPanels component. I'm noticing a hydration mismatch: the settings panel readssettings.legacySidebarEnableddirectly, but the search index uses theuseLegacySidebarEnabled()hook which waits for hydration. This means the panel could show different visibility than the search before hydration completes, defeating the purpose of the hook which exists to prevent that flicker in the first place. I should also trace whereconfirmThreadArchiveis actually consumed — whether the non-legacy sidebar uses it too — and verify whatsettingsobject the GeneralSettingsPanel is reading from, since that determines whether the divergence is real or if they're already aligned. I'm noticing thatconfirmThreadArchiveis referenced in multiple places—the new default sidebar at line 1689, and also inuseThreadActionMenu.tsat line 86. This means hiding the archive setting when the legacy sidebar is disabled could be incorrect, since the setting still controls archive behavior in the default sidebar. I need to check those specific sections in both files to understand the full scope of how this setting is being used. I'm realizing there's a real regression here — theconfirmThreadArchivesetting is still being used by both the default sidebar and the thread action menu, but hiding the setting when the legacy sidebar is disabled means users lose the ability to control this behavior even though it still applies. If someone had enabled this confirmation while using the legacy sidebar and then switched, the setting would persist but become inaccessible, which is a concrete problem. The setting's description mentions requiring a second click on the inline archive action, though I'm wondering if the legacy sidebar implementation works differently than I'm thinking. So the real issue is that hiding theconfirmThreadArchivesetting when the legacy sidebar is disabled actually breaks the configuration—users who had it enabled and switch to the default sidebar still get confirmation dialogs in both the default sidebar's thread menu and the chat header, but now they can't control that behavior anymore. This feels like it crosses into functional consistency territory beyond just UI consistency. Thedelete-confirmationrow stays visible regardless, and both settings affect both sidebars, so there's an inconsistency in what the panel actually controls. There's also a potential hydration mismatch: the panel checkssettings.legacySidebarEnableddirectly while search usesuseLegacySidebarEnabled()which is hydration-aware. Before hydration,usePrimarySettings()might merge client settings differently, so a legacy user could see the row hidden pre-hydration but then visible post-hydration once the hook resolves. Actually, tracing through it more carefully — both ultimately read from the same hydrated client settings store, so the window where they'd diverge is pretty narrow. The real issue is just that they're reading the same value through different paths. I'd recommend having the panel use the shareduseLegacySidebarEnabled()hook instead for a single source of truth. Now looking at how the search-index filtering works —visibleSettingsSearchItemsfilters beforesearchSettingsruns, butdesktopOnlyis handled insidesearchSettingsitself. Two different mechanisms for the same filtering concept, thoughrequiresLegacySidebarcould follow the same pattern. It's a consistency thing, not a real bug, so I'll skip it. There's a potential issue with the active result index cycling logic — it's using a captured value instead of a functional updater now, so if two keydowns fire before a re-render, both compute from the same stale index. That said, React batches updates in the same tick, and separate keydown events trigger separate renders, so in practice it's fine. Key repeat events are discrete and sync-flush in React 18, so the risk is low. Still, swapping out the functional updater is a subtle regression for rapid keyboard navigation, and the check explicitly covers keyboard behavior preservation. A cleaner fix would be to keep the functional updater and clamp inside it — that way the logic stays reactive and handles both the increment and the bounds check properly. I should also check whetheractiveResultIndexgets reset when the query changes. That's a pre-existing issue, though the clamping from this change does help when results shrink. Looking at the PR branch name "fix/hide-archive-confirmation-setting," it seems the intent was to hide the setting only for the legacy sidebar's inline archive action, but the problem is that both the default Sidebar and ChatHeader also readconfirmThreadArchiveand display confirmation dialogs. So hiding the setting entirely doesn't match what the description claims. I need to check what LegacySidebar actually does and whether the default sidebar has its own inline archive action to understand the scope of this issue. The concrete issue is that with the legacy sidebar disabled, the default sidebar's thread menu and chat header thread menu still gate archive actions onconfirmThreadArchive, making an active confirmation dialog unconfigurable for users who enabled it before switching sidebars—a real behavioral regression. The smallest fix would be to keep the row visible or scope the confirmation logic specifically to the legacy sidebar. Additionally, the search index hides the item entirely, so users can't even discover it. There's also a secondary consistency problem: the panel checks the rawsettings.legacySidebarEnabledwhile search uses the hydration-awareuseLegacySidebarEnabled()hook. If the hiding stays, they should use the same source. During hydration, the panel and search index could disagree since the panel doesn't use the shared hook designed for this decision. I should verify howusePrimarySettingsanduseClientSettingsValuehandle hydration to state this accurately. The snapshot defaults to false until hydration completes, so there's no real divergence in practice. But there's a timing issue: ifreplaceClientSettingsSnapshotruns beforesetClientSettingsHydrated(true), there's a window where the value is true but hydrated is false—the panel would render the row while search hides it. Since both trigger separate listener updates inuseSyncExternalStore, React could render between them. These updates happen inside an async function rather than an event handler, so React 18's auto-batching should batch them together anyway, making this a weak concern. I'll add a secondary note about this alongside the main comment on the regression, and mention the shared hook consistency as part of the fix. The keyboard behavior change isn't worth the noise since discrete events flush synchronously, so I'll skip that. Let me focus on the core finding and check whether the hiding is actually a regression by looking at how the legacy sidebar handles the archive action. The archive action is inline in the legacy sidebar but becomes a menu action with a native confirm dialog in the new one. The setting still gates it, so hiding the toggle leaves orphaned behavior—this is a concrete regression. The setting also affects the ChatHeader archive action regardless of sidebar state, which strengthens the finding. I'll post the main comment on line 2190 in SettingsPanels.tsx where the conditional renders, and keep it focused on this one root cause rather than splitting into multiple comments.
1. modify_pr — One finding: hiding the archive-confirmation row leaves the behavior it controls active in the default (non-legacy) UI.
Agent Credits: 75 credits