Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 25 additions & 23 deletions apps/web/src/components/settings/SettingsPanels.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2187,31 +2187,33 @@ export function GeneralSettingsPanel() {
}
/>

<SettingsRow
{...searchableSetting("archive-confirmation")}
description="Require a second click on the inline archive action before a thread is archived."
resetAction={
settings.confirmThreadArchive !== DEFAULT_UNIFIED_SETTINGS.confirmThreadArchive ? (
<SettingResetButton
label="archive confirmation"
onClick={() =>
updateSettings({
confirmThreadArchive: DEFAULT_UNIFIED_SETTINGS.confirmThreadArchive,
})
{settings.legacySidebarEnabled ? (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

confirmThreadArchive is not legacy-sidebar-only: the default sidebar (Sidebar.tsx:1689 → archive case at :3150) and the chat header thread menu (useThreadActionMenu.ts:86:260, used by ChatHeader.tsx) both still gate archive on it. Hiding the row here (and filtering archive-confirmation out of the search index) leaves that confirmation dialog reachable but unconfigurable — a user who turned it on under the legacy sidebar and then switched back is stuck with it, with no row and no search hit to turn it off.

Smallest fix: either keep the row rendered unconditionally (and drop requiresLegacySidebar from the search item), or also scope the runtime behavior — make Sidebar.tsx and useThreadActionMenu ignore confirmThreadArchive when the legacy sidebar is off — so the control and the behavior are hidden together.

If the row does stay conditional, please gate it on the shared useLegacySidebarEnabled() hook rather than raw settings.legacySidebarEnabled; SettingsSidebarNav uses the hook, and its hydration guard exists precisely so this decision has one source of truth.

Posted via Macroscope — UI Consistency

<SettingsRow
{...searchableSetting("archive-confirmation")}
description="Require a second click on the inline archive action before a thread is archived."
resetAction={
settings.confirmThreadArchive !== DEFAULT_UNIFIED_SETTINGS.confirmThreadArchive ? (
<SettingResetButton
label="archive confirmation"
onClick={() =>
updateSettings({
confirmThreadArchive: DEFAULT_UNIFIED_SETTINGS.confirmThreadArchive,
})
}
/>
) : null
}
control={
<Switch
checked={settings.confirmThreadArchive}
onCheckedChange={(checked) =>
updateSettings({ confirmThreadArchive: Boolean(checked) })
}
aria-label="Confirm thread archiving"
/>
) : null
}
control={
<Switch
checked={settings.confirmThreadArchive}
onCheckedChange={(checked) =>
updateSettings({ confirmThreadArchive: Boolean(checked) })
}
aria-label="Confirm thread archiving"
/>
}
/>
}
/>
) : null}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hidden archive setting still applies

Medium Severity

Hiding Archive confirmation when the legacy sidebar is off leaves confirmThreadArchive live on the default sidebar. Context-menu archive still shows a confirm dialog when the flag is on, with no remaining control to change it.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 9a8cb71. Configure here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no context menu archive in new sidebar
image


<SettingsRow
{...searchableSetting("delete-confirmation")}
Expand Down
33 changes: 22 additions & 11 deletions apps/web/src/components/settings/SettingsSidebarNav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,15 @@ import {
useSidebar,
} from "../ui/sidebar";
import { T3ConnectSidebarAvatar, T3ConnectSidebarSignIn } from "../clerk/T3ConnectSidebarSignIn";
import { useLegacySidebarEnabled } from "../../hooks/useSettings";
import { scrollToSettingsTarget } from "./settingsLayout";
import {
clampSettingsSearchResultIndex,
searchSettings,
SETTINGS_SECTION_LABELS,
type SettingsPath,
type SettingsSearchItem,
visibleSettingsSearchItems,
} from "./settingsSearch";

const SETTINGS_SECTION_ICONS: Readonly<
Expand Down Expand Up @@ -77,17 +80,25 @@ export function SettingsSidebarNav({ pathname }: { pathname: string }) {
const searchInputRef = useRef<HTMLInputElement>(null);
const [query, setQuery] = useState("");
const [activeResultIndex, setActiveResultIndex] = useState(0);
const results = useMemo(() => searchSettings(query), [query]);
const legacySidebarEnabled = useLegacySidebarEnabled();
const results = useMemo(
() => searchSettings(query, visibleSettingsSearchItems(legacySidebarEnabled)),
[legacySidebarEnabled, query],
);
Comment thread
macroscopeapp[bot] marked this conversation as resolved.
const resolvedActiveResultIndex = clampSettingsSearchResultIndex(
activeResultIndex,
results.length,
);
const isSearching = query.trim().length > 0;
const hasResults = results.length > 0;

useEffect(() => {
const result = results[activeResultIndex];
const result = results[resolvedActiveResultIndex];
if (!result) return;
document
.getElementById(`settings-search-result-${result.id}`)
?.scrollIntoView({ block: "nearest" });
}, [activeResultIndex, results]);
}, [resolvedActiveResultIndex, results]);

useEffect(() => {
const handleKeyDown = (event: globalThis.KeyboardEvent) => {
Expand Down Expand Up @@ -160,21 +171,21 @@ export function SettingsSidebarNav({ pathname }: { pathname: string }) {
if (results.length === 0) return;
if (event.key === "ArrowDown") {
event.preventDefault();
setActiveResultIndex((index) => (index + 1) % results.length);
setActiveResultIndex((resolvedActiveResultIndex + 1) % results.length);
return;
}
if (event.key === "ArrowUp") {
event.preventDefault();
setActiveResultIndex((index) => (index - 1 + results.length) % results.length);
setActiveResultIndex((resolvedActiveResultIndex - 1 + results.length) % results.length);
return;
}
if (event.key === "Enter") {
event.preventDefault();
const result = results[activeResultIndex];
const result = results[resolvedActiveResultIndex];
if (result) handleSearchResultClick(result);
}
},
[activeResultIndex, clearSearch, handleSearchResultClick, isSearching, results],
[clearSearch, handleSearchResultClick, isSearching, resolvedActiveResultIndex, results],
);
const handleBackClick = useCallback(() => {
if (isMobile) {
Expand Down Expand Up @@ -211,8 +222,8 @@ export function SettingsSidebarNav({ pathname }: { pathname: string }) {
aria-expanded={isSearching && hasResults}
aria-controls={isSearching && hasResults ? "settings-search-results" : undefined}
aria-activedescendant={
isSearching && results[activeResultIndex]
? `settings-search-result-${results[activeResultIndex].id}`
isSearching && results[resolvedActiveResultIndex]
? `settings-search-result-${results[resolvedActiveResultIndex].id}`
: undefined
}
className="min-w-0 flex-1 [&_[data-slot=input]]:h-auto [&_[data-slot=input]]:p-0 [&_[data-slot=input]]:leading-normal [&_[data-slot=input]]:text-sm [&_[data-slot=input]]:font-medium [&_[data-slot=input]]:text-sidebar-foreground [&_[data-slot=input]]:placeholder:text-sidebar-muted-foreground"
Expand Down Expand Up @@ -255,10 +266,10 @@ export function SettingsSidebarNav({ pathname }: { pathname: string }) {
<SidebarMenuButton
id={`settings-search-result-${item.id}`}
role="option"
aria-selected={index === activeResultIndex}
aria-selected={index === resolvedActiveResultIndex}
tabIndex={-1}
size="sm"
isActive={index === activeResultIndex}
isActive={index === resolvedActiveResultIndex}
className="h-auto min-h-10 items-start gap-2 rounded-md px-2 py-2 text-left hover:bg-sidebar-row-hover hover:text-sidebar-foreground"
onMouseMove={() => setActiveResultIndex(index)}
onClick={() => handleSearchResultClick(item)}
Expand Down
17 changes: 17 additions & 0 deletions apps/web/src/components/settings/settingsSearch.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import { describe, expect, it } from "vite-plus/test";

import {
clampSettingsSearchResultIndex,
searchableSetting,
searchSettings,
SETTINGS_SEARCH_ITEMS,
type SettingsSearchItem,
visibleSettingsSearchItems,
} from "./settingsSearch";

const ITEMS: ReadonlyArray<SettingsSearchItem> = [
Expand Down Expand Up @@ -70,6 +72,21 @@ describe("searchSettings", () => {
expect(new Set(ids).size).toBe(ids.length);
});

it("only exposes archive confirmation for the legacy sidebar", () => {
expect(
visibleSettingsSearchItems(false).some((item) => item.id === "archive-confirmation"),
).toBe(false);
expect(
visibleSettingsSearchItems(true).some((item) => item.id === "archive-confirmation"),
).toBe(true);
});

it("clamps the active result after visible settings shrink", () => {
expect(clampSettingsSearchResultIndex(1, 1)).toBe(0);
expect(clampSettingsSearchResultIndex(1, 2)).toBe(1);
expect(clampSettingsSearchResultIndex(0, 0)).toBe(0);
});

it("serves anchor props to panels from the catalog", () => {
expect(searchableSetting("word-wrap")).toEqual({ id: "word-wrap", title: "Word wrap" });
expect(searchableSetting("archive")).toEqual({ id: "archive", title: "Archived threads" });
Expand Down
14 changes: 14 additions & 0 deletions apps/web/src/components/settings/settingsSearch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export interface SettingsSearchItem {
readonly title: string;
readonly to: SettingsPath;
readonly targetId?: string;
readonly requiresLegacySidebar?: boolean;
// Its row only renders in the desktop app, so a browser result would land on
// an anchor that isn't there.
readonly desktopOnly?: boolean;
Expand Down Expand Up @@ -148,6 +149,7 @@ export const SETTINGS_SEARCH_ITEMS = [
id: "archive-confirmation",
title: "Archive confirmation",
to: "/settings/general",
requiresLegacySidebar: true,
},
{
id: "delete-confirmation",
Expand Down Expand Up @@ -240,6 +242,10 @@ function normalizeSearchText(value: string): string {
.trim();
}

export function clampSettingsSearchResultIndex(index: number, resultCount: number): number {
return Math.max(0, Math.min(index, resultCount - 1));
}

export function searchSettings(
query: string,
items: ReadonlyArray<SettingsSearchItem> = SETTINGS_SEARCH_ITEMS,
Expand All @@ -253,3 +259,11 @@ export function searchSettings(
normalizeSearchText(item.title).includes(normalizedQuery),
);
}

export function visibleSettingsSearchItems(
legacySidebarEnabled: boolean,
items: ReadonlyArray<SettingsSearchItem> = SETTINGS_SEARCH_ITEMS,
): ReadonlyArray<SettingsSearchItem> {
if (legacySidebarEnabled) return items;
return items.filter((item) => item.requiresLegacySidebar !== true);
}
Loading