fix(nav): prevent top navbar from flashing to vertical layout on load#40781
fix(nav): prevent top navbar from flashing to vertical layout on load#40781aminghadersohi wants to merge 3 commits into
Conversation
Grid.useBreakpoint() returns {} on first render before viewport is
measured, making screens.md undefined (falsy) and causing the nav to
render in 'inline' (vertical) mode before snapping to 'horizontal'.
Using `screens.md !== false` treats the undefined initial state as
true, so the nav starts in horizontal mode and only switches to inline
on genuinely narrow viewports.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Code Review Agent Run #706786Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #40781 +/- ##
==========================================
+ Coverage 64.19% 64.21% +0.02%
==========================================
Files 2666 2666
Lines 143991 143835 -156
Branches 33108 33080 -28
==========================================
- Hits 92428 92358 -70
+ Misses 49950 49871 -79
+ Partials 1613 1606 -7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Adds two tests that cover the scenarios the original fix addressed: - Empty breakpoint object (first paint before viewport is measured) must render horizontal, not inline — regression guard for the flash bug. - Explicit md:false must render inline, validating the mobile layout path. Refactors the module-level antd Grid mock to use a jest.fn() indirection so individual tests can override useBreakpoint's return value. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
Prevents a visible layout flash in the top navigation on initial page load by accounting for Ant Design’s Grid.useBreakpoint() returning an empty screen map on first render, ensuring the desktop navbar renders horizontally on first paint.
Changes:
- Introduce an
isMdderived flag that treatsscreens.md === undefinedas desktop to avoid an initial inline (vertical) render. - Update all
screens.md-based navbar layout decisions (mode, right-menu alignment, submenu dropdown icon) to useisMd. - Add regression tests covering the “unmeasured breakpoints” initial render and the explicit mobile (
md: false) case.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| superset-frontend/src/features/home/Menu.tsx | Switch navbar layout logic to an isMd guard that avoids first-paint breakpoint uncertainty causing a layout flash. |
| superset-frontend/src/features/home/Menu.test.tsx | Improve breakpoint mocking and add tests to lock in the no-flash behavior for the initial {} breakpoint state and mobile behavior. |
| jest.mock('antd', () => ({ | ||
| ...jest.requireActual('antd'), | ||
| Grid: { | ||
| ...jest.requireActual('antd').Grid, | ||
| useBreakpoint: () => ({ md: true }), | ||
| useBreakpoint: () => mockUseBreakpoint(), |
|
The suggestion to reuse the superset-frontend/src/features/home/Menu.test.tsx |
Code Review Agent Run #9f4687Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
Summary
Grid.useBreakpoint()(Ant Design) returns an empty object{}on the first render before viewport breakpoints are measured. This madescreens.mdundefined(falsy), soscreens.md ? 'horizontal' : 'inline'evaluated to'inline'on first paint — rendering the navbar as a vertical stack. After the next render cycle the breakpoint resolved and the nav snapped to horizontal, producing a visible layout flash on every page load.Fix: Replace
screens.mdwithconst isMd = screens.md !== false, which treats the initialundefinedastrue(desktop/horizontal layout) and only switches to mobile/inline layout whenscreens.mdis explicitlyfalse. All threescreens.mdusages inMenu.tsx(mode,align, and the dropdown icon spread) are updated to useisMd.Testing
Tested by loading a Superset workspace and verifying the top navbar renders horizontally on first paint with no layout flash. Responsive behavior (switching to inline on narrow viewports) is preserved.
Checklist
Fixes: sc-108034
🤖 Generated with Claude Code