-
Notifications
You must be signed in to change notification settings - Fork 206
Fix reference sidebar flash by server-rendering directory #1094
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: main
Are you sure you want to change the base?
Conversation
|
Thanks! I will try to review this some time in the next month. If any other reviewers gets here first, please by all means feel free to review ahead of me. |
|
I pulled the changes to test them locally, but I noticed some unexpected UX on the page, and it seems like we’re still seeing the flash in the original nav (screenshot below). Screen.Recording.2026-01-20.at.06.20.01.movIt looks like the 'flashing' persists because the issue might be rooted deeper in how the global NavPanels component handles initial resizing on the client side (the desktop isOpen state being false by default): https://github.com/processing/p5.js-website/blob/main/src/components/Nav/NavPanels.tsx Since this affects every page and not just the Reference section, I think we might need to shift our focus to that core component to truly solve the flash issue described in #477. |
|
Thanks for the detailed investigation and for pointing me to NavPanels.tsx. |
|
Thanks again for pointing me to NavPanels.tsx — that was the key. I’m leaving this PR in draft as context for the earlier investigation. Happy to close it if you prefer. |
|
@rakesh2OO5 it is probably easier to follow the discussion if you continue to commit to the same PR rather than opening new ones, as it contains everything to one place. |
|
That makes sense — thanks for the guidance. |
79c0e32 to
6d456fa
Compare
|
Thanks for the guidance earlier — I’ve now consolidated the fix into this PR with a minimal diff (+36/-26) that addresses the root hydration mismatch in NavPanels and removes the sidebar flash without changing existing behavior. @coseeian if you have a moment, I’d really appreciate a review on this updated version. Happy to iterate if anything needs adjustment. |
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.
I pulled this PR down and test locally by navigation to /reference/p5/arc/. Noticed regression on mobile. It looks like the changes to address issue on desktop are affecting mobile nav behavior.
Expected: Nav should be closed by default.
Actual: menu nav keep opened, and reference nav closed with flash.
Screen.Recording.2026-01-23.at.07.09.50.mov
Please have a look. Thanks.
|
Thanks for flagging this — agreed, mobile behavior should stay consistent with the expected default (nav closed). |
Fixes #477
Summary
Server-renders the reference directory to prevent the initial sidebar flash and layout shift caused by client-only hydration.
Changes
Result