-
Notifications
You must be signed in to change notification settings - Fork 3.4k
feat(desktop): let people turn off the sidebar hover reveal #95523
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -138,13 +138,15 @@ export function GeneralSettings() { | |
| defaultCloudMessagingMode, | ||
| defaultReasoningEffort, | ||
| diffOpenMode, | ||
| revealSidebarOnHover, | ||
| sendMessagesWith, | ||
| setAutoConvertLongText, | ||
| setDefaultInitialTaskMode, | ||
| setDefaultMessagingMode, | ||
| setDefaultCloudMessagingMode, | ||
| setDefaultReasoningEffort, | ||
| setDiffOpenMode, | ||
| setRevealSidebarOnHover, | ||
| setSendMessagesWith, | ||
| } = useSettingsStore(); | ||
|
|
||
|
|
@@ -160,6 +162,18 @@ export function GeneralSettings() { | |
| [theme, setTheme], | ||
| ); | ||
|
|
||
| const handleRevealSidebarOnHoverChange = useCallback( | ||
| (checked: boolean) => { | ||
| track(ANALYTICS_EVENTS.SETTING_CHANGED, { | ||
| setting_name: "reveal_sidebar_on_hover", | ||
| new_value: checked, | ||
| old_value: !checked, | ||
| }); | ||
| setRevealSidebarOnHover(checked); | ||
| }, | ||
| [setRevealSidebarOnHover], | ||
| ); | ||
|
|
||
| const handleAutoConvertLongTextChange = useCallback( | ||
| (value: AutoConvertLongText) => { | ||
| track(ANALYTICS_EVENTS.SETTING_CHANGED, { | ||
|
|
@@ -267,6 +281,18 @@ export function GeneralSettings() { | |
| </SettingsCardRow> | ||
| </SettingsCard> | ||
| )} | ||
| <SettingsCard> | ||
| <SettingsCardRow | ||
| label="Reveal the sidebar on hover" | ||
| description="Slide the collapsed sidebar out when the pointer reaches the left edge. Turn this off to open it only with the toggle or βB." | ||
| > | ||
| <Switch | ||
| size="sm" | ||
| checked={revealSidebarOnHover} | ||
| onCheckedChange={handleRevealSidebarOnHoverChange} | ||
| /> | ||
|
Comment on lines
+289
to
+293
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Give the switch an accessible nameWhy we think it's a valid issue
Issue descriptionThe new Switch has no accessible name. SettingsCardRow renders the visible label as a sibling span, so screen readers announce an unnamed switch. Suggested fixAdd aria-label="Reveal the sidebar on hover" to the Switch. Alternatively, connect the visible label with aria-labelledby. Prompt to fix with AI (copy-paste) |
||
| </SettingsCardRow> | ||
| </SettingsCard> | ||
| </SettingsSection> | ||
|
|
||
| <SettingsSection | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,6 +46,7 @@ import { useInboxDeepLink } from "@posthog/ui/features/inbox/hooks/useInboxDeepL | |
| import { useIntegrations } from "@posthog/ui/features/integrations/useIntegrations"; | ||
| import { useLoopDeepLink } from "@posthog/ui/features/loops/hooks/useLoopDeepLink"; | ||
| import { useScoutDeepLink } from "@posthog/ui/features/scouts/hooks/useScoutDeepLink"; | ||
| import { useSettingsStore } from "@posthog/ui/features/settings/settingsStore"; | ||
| import { useSetupDiscovery } from "@posthog/ui/features/setup/useSetupDiscovery"; | ||
| import { NAV_RAIL_WIDTH } from "@posthog/ui/features/sidebar/constants"; | ||
| import { | ||
|
|
@@ -218,6 +219,7 @@ function RootLayout() { | |
|
|
||
| const toggleSidebar = useSidebarStore((s) => s.toggle); | ||
| const sidebarPeek = useSidebarPeekStore((s) => s.peek); | ||
| const revealSidebarOnHover = useSettingsStore((s) => s.revealSidebarOnHover); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wait for settings hydration before enabling hoverWhy we think it's a valid issue
Issue descriptionelectronStorage hydrates asynchronously. Before hydration finishes, revealSidebarOnHover has its default true value. A user who saved false can still trigger hover during startup. On routes without ChannelsSidebar, the scrim can remain visible because no mounted effect cancels the peek. Suggested fixRead _hasHydrated with the preference. Require both values here and in useSidebarEdgeHoverPeek. Cancel the peek when hydration resolves to false. Add a delayed-hydration test with a saved false value. Prompt to fix with AI (copy-paste) |
||
| // Toggling makes any hover-peek redundant (opening replaces the overlay; | ||
| // closing must not leave it lingering under the pointer). | ||
| const handleToggleSidebar = (): void => { | ||
|
|
@@ -348,7 +350,7 @@ function RootLayout() { | |
| aria-label="Toggle sidebar" | ||
| onClick={handleToggleSidebar} | ||
| onMouseEnter={() => { | ||
| if (!sidebarOpen) beginSidebarPeek(); | ||
| if (revealSidebarOnHover && !sidebarOpen) beginSidebarPeek(); | ||
| }} | ||
| > | ||
| {sidebarOpen ? ( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Render the platform-specific sidebar shortcut
Why we think it's a valid issue
SHORTCUTS.TOGGLE_LEFT_SIDEBARis"mod+b"(packages/ui/src/features/command/keyboard-shortcuts.ts:23), andGlobalEventHandlers.tsx:210binds it withuseHotkeysfrom react-hotkeys-hook, which resolvesmodto Meta on macOS and to Ctrl on other platforms. So the real key on Windows and Linux is Ctrl+B, not theβBin the new description atGeneralSettings.tsx:287.apps/code/electron-builder.ts:104targetsnsisand:118targetsAppImage,deb,rpmβ so non-Mac users do read this row.formatKeymapsmodtoβorCtrlfrom theisMacflag (keyboard-shortcuts.ts:363,packages/ui/src/utils/platform.ts), andNavResizeTooltip.tsx:16renders this exact shortcut withformatHotkey(SHORTCUTS.TOGGLE_LEFT_SIDEBAR). Other copy branches the same way, for exampleProjectSwitcher.tsx:378andReportChatSidebar.tsx:302.Issue description
The description always shows βB. The registered shortcut is mod+b. Windows and Linux users receive an incorrect instruction because their shortcut is Ctrl+B.
Suggested fix
Build the description with formatHotkey(SHORTCUTS.TOGGLE_LEFT_SIDEBAR). This matches NavResizeTooltip and stays correct on each supported platform.
Prompt to fix with AI (copy-paste)