fix(web): stop header logo resizing on mount, narrow banner gradient - #2059
Conversation
Resolves two QA-blocking visual regressions in the consolidated header. - The mounted logo carried max-w-full, so on sidebar themes (gray/azure) at mobile widths it compressed ~9% (140px -> 127px) after mount, while the server-rendered boot placeholder in webGUI Header.php stayed 140px. Use shrink-0 and a fixed width so the two agree; the nav side already truncates, so it absorbs the space (verified: no horizontal overflow). - The banner gradient covered the right 45% (576px at 1280px), roughly 1.8x the legacy header's min(30%, 320px) edge gradient, visibly obscuring users' banner images. Match the legacy width. Adds a theme-gated snapshot test covering all four shipped themes in the configuration webGUI ships them in (sidebar themes carry the array-usage bar, top-nav themes do not), plus guards on both regressions above. Verified on devgen.local (7.3.2) across white/black/gray/azure at 1280px and 390px: boot-vs-mounted logo delta 0px (<=0.01px sub-pixel), logo width stable, gradient 320px, no overflow.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe header now uses a bounded, right-aligned banner gradient and prevents themed logos from shrinking. New tests cover all shipped themes, layout variants, logo styles, fixed sizing, gradient bounds, and disabled-gradient behavior. ChangesHeader theme layout
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/__test__/components/HeaderThemes.test.ts`:
- Around line 141-145: Update the createTestingPinia call in the beforeEach
setup to use stubActions: false, allowing themeStore.setTheme(...) to execute
and mutate the configured theme state used by the mounted Header assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8fcf578c-75b6-4143-a6c6-7a6004c3238d
⛔ Files ignored due to path filters (1)
web/__test__/components/__snapshots__/HeaderThemes.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
web/__test__/components/HeaderThemes.test.tsweb/src/components/Header.standalone.vueweb/src/components/Header/HeaderLogo.vue
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0c90da232c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
|
|
||
| beforeEach(() => { | ||
| provideApolloClient(new ApolloClient({ cache: new InMemoryCache() })); | ||
| pinia = createTestingPinia({ createSpy: vi.fn, initialState: { server: { ...server } } }); |
There was a problem hiding this comment.
Run setTheme instead of stubbing it
createTestingPinia stubs actions by default, so each later themeStore.setTheme(...) call is only recorded and leaves the store in its default white, banner-disabled state. Consequently, the black/gray/azure cases never mount with their requested themes, and the banner-disabled test passes without exercising its transition; pass stubActions: false or initialize the theme state directly so these theme-gated tests validate the intended behavior.
Useful? React with 👍 / 👎.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2059 +/- ##
==========================================
+ Coverage 53.07% 53.13% +0.06%
==========================================
Files 1041 1041
Lines 72425 72425
Branches 8354 8354
==========================================
+ Hits 38440 38484 +44
+ Misses 33858 33814 -44
Partials 127 127 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
createTestingPinia stubs actions by default, so themeStore.setTheme() was a no-op and every case silently ran against the default theme (white, banner disabled) — the per-theme coverage was not testing anything, and the banner-gradient assertion passed trivially. Pass stubActions: false so setTheme mutates the store, and record the banner-driven .uh-meta-over-banner state in the layout signature so a regression here fails the snapshot instead of going unnoticed (verified: reverting the flag now fails all four theme snapshots).
🔄 PR Merged - Plugin Redirected to StagingThis PR has been merged and the preview plugin has been updated to redirect to the staging version. For users testing this PR:
Staging URL: Thank you for testing! 🚀 |
Resolves two QA-blocking visual regressions on OS-620 (consolidated
<unraid-header>), found in QA of webGUI PR #2701.What
max-w-full, so on gray/azure at mobile widths it compressed ~9% (140px → 127px) after mount, while the server-rendered boot placeholder in webGUIHeader.phpstayed 140px. Nowshrink-0with a fixed width, so the two agree. The nav side already truncates and absorbs the space — verified no horizontal overflow.min(30%, 320px)edge gradient. Now matches the legacy width.Pairs with unraid/webgui#2701, which carries the light-DOM boot-placeholder side.
Tests
New theme-gated snapshot test covering all four shipped themes in the configuration webGUI ships them in (sidebar themes carry the array-usage bar, top-nav themes do not), plus explicit guards on both regressions above. Verified the guards actually fail when the bugs are reintroduced.
Verification
Measured on devgen.local (Unraid 7.3.2) across white/black/gray/azure at 1280px and 390px:
Refs OS-620
Summary by CodeRabbit
Bug Fixes
Tests