Skip to content

Fixed sidebar flicker during navigation preference updates#26924

Merged
jonatansberg merged 1 commit intomainfrom
ber-3464-prevent-sidebar-flicker-during-navigation-preference-updates
Mar 24, 2026
Merged

Fixed sidebar flicker during navigation preference updates#26924
jonatansberg merged 1 commit intomainfrom
ber-3464-prevent-sidebar-flicker-during-navigation-preference-updates

Conversation

@jonatansberg
Copy link
Copy Markdown
Member

@jonatansberg jonatansberg commented Mar 24, 2026

This fixes a sidebar flicker that happens when expanding or collapsing sidebar sections updates the current user's accessibility preferences. useUserPreferences() would briefly drop its data during unrelated preference updates, which in turn made dependent sidebar UI such as the What's New banner and user-menu indicators disappear and reappear.

The fix keeps the previous user preference data while the accessibility-backed query key changes. I also added one focused regression test at the useUserPreferences() boundary to lock that behavior in place.

Verification:

  • ../../node_modules/.bin/vitest run src/hooks/user-preferences.accessibility-updates.test.tsx from apps/admin
  • ../../node_modules/.bin/eslint src/hooks/user-preferences.ts src/hooks/user-preferences.accessibility-updates.test.tsx from apps/admin

Fixes https://linear.app/ghost/issue/BER-3464/prevent-sidebar-flicker-during-navigation-preference-updates

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 24, 2026

Walkthrough

The pull request adds a test case for the useUserPreferences hook that verifies the interaction between the useUserPreferences query and useEditUserPreferences mutation. The test confirms that when a mutation updates specific user preference fields (such as navigation.expanded.posts), the query data is properly refreshed with the updated values while preserving unrelated fields like whatsNew.lastSeenDate. Additionally, the useUserPreferences hook configuration is updated to include keepPreviousData: true in the query options, enabling the hook to retain previously cached data during query transitions.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main fix: preventing sidebar flicker during navigation preference updates, which directly matches the core change in the pull request.
Description check ✅ Passed The description is directly related to the changeset, explaining the problem being fixed (sidebar flicker), the solution (keeping previous data with keepPreviousData: true), and the regression test added.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ber-3464-prevent-sidebar-flicker-during-navigation-preference-updates

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@jonatansberg jonatansberg force-pushed the ber-3464-prevent-sidebar-flicker-during-navigation-preference-updates branch from 2d97b3a to ebfb201 Compare March 24, 2026 08:12
ref https://linear.app/ghost/issue/BER-3464/prevent-sidebar-flicker-during-navigation-preference-updates
Keep previous user preference data during accessibility-backed query key changes and cover the regression at the useUserPreferences hook boundary so sidebar consumers no longer flicker during unrelated navigation preference updates.
@jonatansberg jonatansberg force-pushed the ber-3464-prevent-sidebar-flicker-during-navigation-preference-updates branch from ebfb201 to 3f246fb Compare March 24, 2026 08:17
@jonatansberg jonatansberg marked this pull request as ready for review March 24, 2026 08:29
@jonatansberg jonatansberg requested review from kevinansfield and rob-ghost and removed request for rob-ghost March 24, 2026 08:31
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
apps/admin/src/hooks/user-preferences.test.tsx (1)

358-358: Make the snapshot check non-vacuous.

Line 358 can pass even when no post-mutation snapshots were recorded. Add a length assertion first so this regression test fails if no render states were observed.

Suggested tightening
-            expect(snapshots).not.toContain(undefined);
+            expect(snapshots.length).toBeGreaterThan(0);
+            expect(snapshots).not.toContain(undefined);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/admin/src/hooks/user-preferences.test.tsx` at line 358, The test's final
assertion only checks that the snapshots array does not contain undefined, which
is vacuous if no snapshots were recorded; update the test in
user-preferences.test.tsx by asserting that the snapshots array has at least one
entry (e.g., expect(snapshots.length).toBeGreaterThan(0)) before the existing
expect(snapshots).not.toContain(undefined) so the regression fails when no
post-mutation render states were observed; target the test's snapshots variable
and the failing assertion that currently reads
expect(snapshots).not.toContain(undefined).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@apps/admin/src/hooks/user-preferences.test.tsx`:
- Line 358: The test's final assertion only checks that the snapshots array does
not contain undefined, which is vacuous if no snapshots were recorded; update
the test in user-preferences.test.tsx by asserting that the snapshots array has
at least one entry (e.g., expect(snapshots.length).toBeGreaterThan(0)) before
the existing expect(snapshots).not.toContain(undefined) so the regression fails
when no post-mutation render states were observed; target the test's snapshots
variable and the failing assertion that currently reads
expect(snapshots).not.toContain(undefined).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6a9cacb1-f4fc-4a2e-bd88-9e3b6b3d2019

📥 Commits

Reviewing files that changed from the base of the PR and between 99b1356 and 3f246fb.

📒 Files selected for processing (2)
  • apps/admin/src/hooks/user-preferences.test.tsx
  • apps/admin/src/hooks/user-preferences.ts

@jonatansberg jonatansberg merged commit 4e4c349 into main Mar 24, 2026
32 checks passed
@jonatansberg jonatansberg deleted the ber-3464-prevent-sidebar-flicker-during-navigation-preference-updates branch March 24, 2026 09:21
cmraible pushed a commit that referenced this pull request Mar 25, 2026
fixes https://linear.app/ghost/issue/BER-3464/prevent-sidebar-flicker-during-navigation-preference-updates

Enabling the `keepPreviousData` flag ensures that query data won't cycle through `undefined` when the derived query re-evaluates.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant