Skip to content

fix(sidebar): persist the open state in localStorage instead of a cookie - #1942

Merged
h4yfans merged 6 commits into
mainfrom
sidebar-open-state-localstorage
Sep 2, 2026
Merged

fix(sidebar): persist the open state in localStorage instead of a cookie#1942
h4yfans merged 6 commits into
mainfrom
sidebar-open-state-localstorage

Conversation

@h4yfans

@h4yfans h4yfans commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Summary

A sidebar the user closed came back open on the next launch and on every vault switch. SidebarProvider in apps/desktop/src/renderer/src/components/ui/sidebar.tsx kept the open state in a sidebar_state cookie. The packaged renderer is loaded with loadFile(), so its origin is file://, and Chromium stores no cookie for that origin. The write was a no-op, the read never matched, and the state initializer fell through to defaultOpen = true every time. The bare catch swallowed the failure, so nothing ever surfaced. A dev run serves the renderer over http://localhost, where cookies work, which is why this shipped.

The vault-switch half of the report has the same single cause. App.tsx:632 renders <SidebarProvider key={vaultPath}>, so opening another vault unmounts and remounts the provider and re-runs that same failing read. One fix covers both symptoms.

The open state now persists through localStorage under sidebar_open. That is the mechanism sidebarWidth already uses twenty lines down in the same component, and it works on file://. The cookie path is deleted rather than kept alongside, so there is one persistence mechanism in this file instead of two. A failed read or write now logs through createLogger('Sidebar').

Scope is global, not per vault, matching the width. The sidebar is a workspace layout preference rather than vault content, so a user who collapses it once should not have to collapse it again in each vault. The tradeoff is that someone who wants a different layout per vault cannot have one. Nobody can have that today either, and the reporter asked for the opposite.

Migration is nothing. A user who never wrote the key gets defaultOpen, exactly as before. No one has a readable sidebar_state cookie on file://, so there is no old value worth carrying over.

Blast radius is small. SidebarProvider is the only reader and writer of the key, and apps/desktop/src/renderer/src/pages, app-sidebar.tsx and the E2E suite all reach the state through useSidebar(). No shared test helper changed. The restart spec relaunches by reusing the first Electron process's own spawnfile and spawnargs, which already carry the resolved binary, the main entry and --user-data-dir, so the second launch is byte-identical to the first. An earlier revision added a userDataDir option to apps/desktop/tests/e2e/utils/electron-lifecycle.ts instead. That is worth recording. Playwright's --only-changed follows the import graph, every spec reaches that util through fixtures.ts, and the changed-E2E job therefore selected all 593 specs and hit its 35 minute cap twice.

Carries the test-only main fix from the calendar-widget PR until it lands; rebasing after that drops it.

Closes #1938

Release note

Memry now remembers a closed sidebar across app restarts and vault switches.

Test plan

The E2E is the load-bearing check here. It runs the built bundle from file://, which is the only surface where this bug exists, so it was proven red before green.

Check Outcome
pnpm --filter @memry/desktop test:renderer on sidebar.test.tsx, before the fix 5 failed, 7 passed
pnpm --filter @memry/desktop test:renderer on sidebar.test.tsx, after the fix 12 passed
pnpm --filter @memry/desktop test:renderer, whole suite 8633 passed, 1 pre-existing failure in calendar-widget-refresh.test.tsx that reproduces identically on origin/main
pnpm test:e2e on sidebar-open-state.e2e.ts with origin/main's sidebar.tsx 2 failed, both Expected: "collapsed" Received: "expanded"
pnpm test:e2e on sidebar-open-state.e2e.ts and sidebar-window-controls.e2e.ts with the fix 6 passed
pnpm --filter @memry/desktop typecheck:web / typecheck:node / typecheck:test pass
pnpm lint 0 errors, no new warnings
pnpm --filter @memry/desktop i18n:check pass
pnpm check:architecture and pnpm check:contracts pass
pnpm docs:impact --base origin/main --strict covered
pnpm docs:build pass
git diff --check clean

The new unit tests cover the read on mount, the fallback when the key was never written, the write on toggle, the remount that a vault switch performs, and the logged failure when localStorage throws on read and on write. Five mutations of the fix were each killed, including one that keeps the behaviour and only breaks propagation by hoisting the read to module scope, which survives first mount and fails the remount case.

@github-actions github-actions Bot added bug Something isn't working documentation Improvements or additions to documentation test labels Sep 1, 2026
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown

React Doctor found no new issues. 🎉

Reviewed by React Doctor for commit 6e7a4cf.

The calendar widget refresh suite faked the clock onto 2026-08-31, the day it was
written. use-today snapshots the local date into module scope at import and re-reads
the wall clock for its first subscriber, so on every later day that fake clock arrives
as a midnight rollover. todayCalendarRange moves, the useCalendarRange query key moves
with it, and the widget fetches a second day during mount, which is the second
getRange call the first test counted.

Derive the fixture clock and its event hours from the real local date instead, so the
mount no longer straddles a day boundary. Local date fields rather than a UTC instant,
because far enough from UTC the two name different days.
@codecov

codecov Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

…ared helper option

Playwright's --only-changed follows the import graph. Every spec reaches
utils/electron-lifecycle.ts through fixtures.ts, so adding an option there made
the changed-E2E job select all 593 specs and exceed its 35 minute limit twice.
Reusing the first launch's spawnfile and spawnargs gives the same relaunch
against the same user-data dir with no shared file touched.
@h4yfans
h4yfans marked this pull request as ready for review September 2, 2026 09:09
@h4yfans
h4yfans merged commit d3beafc into main Sep 2, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working documentation Improvements or additions to documentation test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: A closed sidebar reopens itself on every launch and vault switch

1 participant