feat(web): consolidated single-component header for Unraid 7.3+ (fixes mobile overlap)#2037
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a consolidated standalone header with server hydration, array capacity usage, shared logo/version controls, prioritized registry wiring, responsive styling, notification mounting changes, theme banner synchronization, and focused tests. ChangesHeader consolidation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Header
participant ServerStore
participant ArrayUsage
participant Apollo
Header->>ServerStore: hydrate server prop
Header->>ArrayUsage: render when enabled
ArrayUsage->>Apollo: poll ArrayCapacity
Apollo-->>ArrayUsage: return capacity metrics
ArrayUsage-->>Header: render usage or offline state
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
Member
Author
|
Companion webgui gate/mount PR: unraid/webgui#2689 (renders |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2037 +/- ##
==========================================
+ Coverage 52.92% 53.07% +0.15%
==========================================
Files 1035 1041 +6
Lines 72122 72425 +303
Branches 8314 8354 +40
==========================================
+ Hits 38167 38440 +273
- Misses 33828 33858 +30
Partials 127 127 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
web/src/components/Header/HeaderVersion.vue (1)
36-36: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove redundant comments.
Several comments here restate what the code already expresses (e.g. "Initialize all stores", "Track if we've loaded the versions yet", "Use lazy query and only load when dropdown is opened"). Consider dropping the ones that don't add clarity.
As per coding guidelines: "Never add comments unless they are needed for clarity of function".
Also applies to: 45-51, 59-59
🤖 Prompt for 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. In `@web/src/components/Header/HeaderVersion.vue` at line 36, Remove the redundant comments in HeaderVersion.vue that only restate the surrounding code, including the ones near the store initialization and the versions-loading logic. Keep only comments that add non-obvious clarification, and delete the rest around the relevant setup and query code in the HeaderVersion component.Source: Coding guidelines
web/src/components/Header/HeaderLogo.vue (1)
8-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
querySelector('.logo')removes the class from only the first match.If more than one legacy
.logoelement can exist in the host DOM, only the first is cleaned up. If a single element is guaranteed this is fine; otherwise usequerySelectorAlland iterate. Also note this mutates DOM outside the component's own tree, which is fragile if the surrounding markup changes.🤖 Prompt for 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. In `@web/src/components/Header/HeaderLogo.vue` around lines 8 - 13, The cleanup in HeaderLogo’s onMounted/nextTick block only removes the legacy class from the first `.logo` match, so update it to handle all matching legacy elements if multiple can exist by iterating over the full result set instead of a single querySelector result. Also keep the logic scoped to the component’s own rendered markup where possible, since querying the global document is brittle; use the HeaderLogo wrapper or a local ref if available.web/src/components/Header/ArrayUsage.vue (1)
22-27: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winQuery errors are silently swallowed.
Only
resultis destructured; if the query errors, the bar simply hides (hasDatabecomes falsy) with no diagnostic signal. Consider also destructuringerrorand logging it for observability.♻️ Proposed fix
-const { result } = useQuery(ARRAY_CAPACITY_QUERY, null, { +const { result, error } = useQuery(ARRAY_CAPACITY_QUERY, null, { pollInterval: 30_000, fetchPolicy: 'cache-and-network', }); + +watch(error, (err) => { + if (err) console.error('[ArrayUsage] failed to fetch array capacity', err); +});🤖 Prompt for 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. In `@web/src/components/Header/ArrayUsage.vue` around lines 22 - 27, The array capacity query in ArrayUsage.vue is swallowing failures because only useQuery(...).result is used and any query error just makes the bar disappear. Update the useQuery destructuring to also capture error, then add logging/diagnostic handling in the same component so query failures are observable even when array data is missing. Keep the existing computed array usage, but wire error handling around ARRAY_CAPACITY_QUERY in ArrayUsage.
🤖 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/Header.test.ts`:
- Around line 197-201: The Header test is over-constrained by checking the exact
thrown message text. Update the test in Header.test.ts so it only asserts that
mounting Header without the server prop throws, using the existing mount/Header
setup, and remove the exact string expectation to make the assertion resilient
to wording changes.
In `@web/src/components/Header.standalone.vue`:
- Around line 98-102: The Header.standalone.vue template is rendering the server
description through v-html without sanitization, which can expose stored XSS.
Update the description flow used by the header: sanitize or escape the value
before it reaches the template, and ensure the serverStore description
assignment from data.description does not pass raw HTML through unchanged. Keep
the change localized around the description handling in the header component and
the store field that feeds it.
In `@web/src/components/Header/HeaderVersion.vue`:
- Around line 83-85: The openApiChangelog helper in HeaderVersion.vue opens a
new tab with window.open but does not prevent reverse tabnabbing. Update the
window.open call to explicitly use noopener (and keep _blank) so the GitHub
releases page cannot access window.opener; the fix belongs in the
openApiChangelog function.
---
Nitpick comments:
In `@web/src/components/Header/ArrayUsage.vue`:
- Around line 22-27: The array capacity query in ArrayUsage.vue is swallowing
failures because only useQuery(...).result is used and any query error just
makes the bar disappear. Update the useQuery destructuring to also capture
error, then add logging/diagnostic handling in the same component so query
failures are observable even when array data is missing. Keep the existing
computed array usage, but wire error handling around ARRAY_CAPACITY_QUERY in
ArrayUsage.
In `@web/src/components/Header/HeaderLogo.vue`:
- Around line 8-13: The cleanup in HeaderLogo’s onMounted/nextTick block only
removes the legacy class from the first `.logo` match, so update it to handle
all matching legacy elements if multiple can exist by iterating over the full
result set instead of a single querySelector result. Also keep the logic scoped
to the component’s own rendered markup where possible, since querying the global
document is brittle; use the HeaderLogo wrapper or a local ref if available.
In `@web/src/components/Header/HeaderVersion.vue`:
- Line 36: Remove the redundant comments in HeaderVersion.vue that only restate
the surrounding code, including the ones near the store initialization and the
versions-loading logic. Keep only comments that add non-obvious clarification,
and delete the rest around the relevant setup and query code in the
HeaderVersion component.
🪄 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
Run ID: f09bf1e7-2145-4745-b4b2-4bc9635fd693
📒 Files selected for processing (14)
web/__test__/components/Header.test.tsweb/__test__/components/Wrapper/component-registry.test.tsweb/components.d.tsweb/src/components/Header.standalone.vueweb/src/components/Header/ArrayUsage.vueweb/src/components/Header/HeaderLogo.vueweb/src/components/Header/HeaderVersion.vueweb/src/components/Header/arrayCapacity.query.tsweb/src/components/Header/useServerProp.tsweb/src/components/HeaderOsVersion.standalone.vueweb/src/components/Wrapper/component-registry.tsweb/src/composables/gql/gql.tsweb/src/composables/gql/graphql.tsweb/src/locales/en.json
Comment on lines
+197
to
+201
| it('throws when the server prop is missing', () => { | ||
| expect(() => mount(Header, { props: {}, global: { plugins: [pinia], stubs } })).toThrow( | ||
| 'Server data not present' | ||
| ); | ||
| }); |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Avoid asserting the exact error message text.
Per coding guidelines, tests should assert that an error is thrown, not the exact wording, to avoid breaking on incidental message changes.
✏️ Proposed fix
it('throws when the server prop is missing', () => {
- expect(() => mount(Header, { props: {}, global: { plugins: [pinia], stubs } })).toThrow(
- 'Server data not present'
- );
+ expect(() => mount(Header, { props: {}, global: { plugins: [pinia], stubs } })).toThrow();
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it('throws when the server prop is missing', () => { | |
| expect(() => mount(Header, { props: {}, global: { plugins: [pinia], stubs } })).toThrow( | |
| 'Server data not present' | |
| ); | |
| }); | |
| it('throws when the server prop is missing', () => { | |
| expect(() => mount(Header, { props: {}, global: { plugins: [pinia], stubs } })).toThrow(); | |
| }); |
🤖 Prompt for 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.
In `@web/__test__/components/Header.test.ts` around lines 197 - 201, The Header
test is over-constrained by checking the exact thrown message text. Update the
test in Header.test.ts so it only asserts that mounting Header without the
server prop throws, using the existing mount/Header setup, and remove the exact
string expectation to make the assertion resilient to wording changes.
Source: Coding guidelines
Comment on lines
+98
to
+102
| <template v-if="description && theme?.descriptionShow"> | ||
| <span | ||
| class="hidden truncate text-right text-base md:!inline-flex md:!items-center" | ||
| v-html="description" | ||
| /> |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if description is sanitized anywhere before or during storage/hydration
rg -n -A3 -B3 'description' web/src/components/Header/useServerProp.ts web/src/store/server.ts 2>/dev/null
rg -n 'sanitize|DOMPurify' web/src -g '*.ts' -g '*.vue'Repository: unraid/api
Length of output: 7246
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant files first
ast-grep outline web/src/components/Header.standalone.vue --view expanded
ast-grep outline web/src/store/server.ts --view expanded
ast-grep outline web/src/components/Header/useServerProp.ts --view expanded 2>/dev/null || true
# Read the relevant sections around description flow and template rendering
sed -n '1,180p' web/src/components/Header.standalone.vue
sed -n '1,220p' web/src/components/Header/useServerProp.ts
sed -n '1040,1085p' web/src/store/server.ts
# Look for any sanitization or HTML escaping specifically tied to description/header rendering
rg -n -A3 -B3 'v-html|description|sanitize|DOMPurify|escapeHtml|innerHTML' web/src/components web/src/store web/src/composables web/src/helpers -g '*.ts' -g '*.vue'Repository: unraid/api
Length of output: 50367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the server contract and any description-related helpers in smaller slices
sed -n '1,220p' web/types/server.ts
sed -n '1,260p' web/src/store/server.ts
sed -n '1,120p' web/src/components/UserProfile.standalone.vue
# Focused search for the server description field and sanitization around the path
rg -n -A2 -B2 'descriptionShow|server\.description|data\.description|description' web/types web/src/components/Header web/src/components/UserProfile web/src/store web/src/composables web/src/helpers -g '*.ts' -g '*.vue'Repository: unraid/api
Length of output: 50366
Sanitize the server description before v-html
web/src/components/Header.standalone.vue:98-102 renders serverStore.description as raw HTML, and the store copies data.description through unchanged. If that field accepts admin-entered HTML, this is stored XSS for every viewer; escape it or sanitize it first.
🤖 Prompt for 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.
In `@web/src/components/Header.standalone.vue` around lines 98 - 102, The
Header.standalone.vue template is rendering the server description through
v-html without sanitization, which can expose stored XSS. Update the description
flow used by the header: sanitize or escape the value before it reaches the
template, and ensure the serverStore description assignment from
data.description does not pass raw HTML through unchanged. Keep the change
localized around the description handling in the header component and the store
field that feeds it.
Comment on lines
+83
to
+85
| const openApiChangelog = () => { | ||
| window.open('https://github.com/unraid/api/releases', '_blank'); | ||
| }; |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Add noopener to window.open to prevent reverse tabnabbing.
Unlike anchor target="_blank" (where modern browsers imply noopener), window.open does not apply it automatically, so the opened page can reach back via window.opener.
🛡️ Proposed fix
const openApiChangelog = () => {
- window.open('https://github.com/unraid/api/releases', '_blank');
+ window.open('https://github.com/unraid/api/releases', '_blank', 'noopener');
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const openApiChangelog = () => { | |
| window.open('https://github.com/unraid/api/releases', '_blank'); | |
| }; | |
| const openApiChangelog = () => { | |
| window.open('https://github.com/unraid/api/releases', '_blank', 'noopener'); | |
| }; |
🤖 Prompt for 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.
In `@web/src/components/Header/HeaderVersion.vue` around lines 83 - 85, The
openApiChangelog helper in HeaderVersion.vue opens a new tab with window.open
but does not prevent reverse tabnabbing. Update the window.open call to
explicitly use noopener (and keep _blank) so the GitHub releases page cannot
access window.opener; the fix belongs in the openApiChangelog function.
Contributor
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/src/components/Header.standalone.vue`:
- Around line 151-192: The header grid in Header.standalone.vue is still fixed
to a two-column layout at all widths, so it needs responsive behavior added
here. Update the .unraid-header-shell styles to use a single-column stacked
layout on mobile, then apply the existing left/right split only at the sm
breakpoint and above. Keep the placement rules for .uh-meta-right, .uh-logo,
.uh-version, and .uh-nav-right aligned with that breakpoint switch.
🪄 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
Run ID: da0bc6e8-02ad-4205-9de4-3665d11d32c1
📒 Files selected for processing (4)
web/src/assets/main.cssweb/src/components/Header.standalone.vueweb/src/components/Header/HeaderLogo.vueweb/src/themes/types.d.ts
✅ Files skipped from review due to trivial changes (2)
- web/src/assets/main.css
- web/src/themes/types.d.ts
Contributor
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/src/components/Notifications/Sidebar.vue (1)
142-161: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd test coverage for the deferred-mount timing.
The double-
requestAnimationFramegating (mount only if still open) is subtle race-prone logic with no visible test coverage. A test exercising rapid open/close toggling and assertingTabsrenders only after the frames resolve (and stays hidden if closed before they resolve) would guard against regressions.🤖 Prompt for 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. In `@web/src/components/Notifications/Sidebar.vue` around lines 142 - 161, Add tests for the deferred mounting logic around the open watcher and bodyMounted state in Sidebar.vue. Verify Tabs remains unrendered until both requestAnimationFrame callbacks resolve after opening, and verify rapid close before those frames resolve prevents Tabs from rendering.
🤖 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.
Nitpick comments:
In `@web/src/components/Notifications/Sidebar.vue`:
- Around line 142-161: Add tests for the deferred mounting logic around the open
watcher and bodyMounted state in Sidebar.vue. Verify Tabs remains unrendered
until both requestAnimationFrame callbacks resolve after opening, and verify
rapid close before those frames resolve prevents Tabs from rendering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 74ff0ff3-f8d5-4e2f-af42-65867ea4e48b
📒 Files selected for processing (4)
unraid-ui/src/components/common/sheet/SheetContent.vueunraid-ui/src/components/common/sheet/sheet.variants.tsweb/src/components/Header.standalone.vueweb/src/components/Notifications/Sidebar.vue
🚧 Files skipped from review as they are similar to previous changes (1)
- web/src/components/Header.standalone.vue
Contributor
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
5 similar comments
Contributor
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
Contributor
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
Contributor
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
Contributor
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
Contributor
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
elibosley
added a commit
to elibosley/webgui
that referenced
this pull request
Jul 22, 2026
Render the single consolidated header web component (<unraid-header>, provided by the unraid-api web build) on Unraid 7.3 and later, passing the server state JSON the same way myservers2.php does for <unraid-user-profile>. On releases before 7.3 the existing multi-component header is unchanged: <unraid-header-os-version> + the sidebar array-usage graph + the myservers2.php user-profile include. Gate uses version_compare against /etc/unraid-version. The consolidated header owns the whole header layout, fixing the mobile overlap of the reboot banner, server title, and notification bell. Companion to unraid/api#2037.
Introduce <unraid-header> (Header.standalone.vue) which owns the entire header in one web component: Unraid logo, OS/API version dropdown and reboot/update banner, server name, server status, notifications bell, and the account dropdown/avatar. The header was previously split across <unraid-header-os-version> and <unraid-user-profile>, with the user-profile cluster absolutely positioned over the logo/version area. On narrow screens the reboot banner, server title, and notification bell overlapped. The consolidated component lays everything out in a single mobile-first flex flow (stacked on mobile, two clusters justified apart on sm+), removing the overlap. Logo + version + reboot banner logic is extracted into a shared Header/HeaderVersion.vue used by both the new header and the legacy HeaderOsVersion.standalone.vue, so there is a single source of truth. The legacy multi-component header stays intact as the < 7.3 fallback. The consolidated header intentionally does not mount the deprecated CriticalNotifications popover (removed by the persistent-notifications work, #2033); persistent alerts now live in the bell. Gating lives in webgui Header.php via version_compare (separate change).
Replace the legacy webgui my_usage() widget (jQuery-injected into #array-usage-sidenav for sidebar themes) with a self-contained ArrayUsage.vue inside <unraid-header>, driven by the GraphQL array.capacity field. Shown when webgui passes show-array-usage (sidebar theme + usage display enabled), matching where the legacy widget appeared. Color thresholds follow the legacy usage_color() defaults (70% warning / 90% critical).
Rework the consolidated header onto a CSS grid so a single account-actions block (bell + menu/avatar) repositions across the sm breakpoint without duplicating stateful components: - mobile: logo and account icons share the top row (app-style), then version + reboot banner, then array usage/status, then server name. - sm+: two columns — logo/version left, status over name+actions right (the prior desktop look). Splits the logo into Header/HeaderLogo.vue so it can sit on the account row on mobile; HeaderVersion.vue now renders only the version dropdown + reboot banner. HeaderOsVersion.standalone.vue (the < 7.3 fallback) composes both and is visually unchanged. Fixes the over-tall, left-adrift mobile header from the first pass.
<unraid-header> defaulted to display:inline, so the header grid shrink-wrapped
and the account controls bunched against the logo instead of spanning the row.
Add a global unraid-header.unapi { display:block; width:100% } rule (matching the
existing unraid-modals.unapi precedent).
Move the server status (uptime + license tier) up: it shares the top row with the logo and account controls once there's horizontal room (>=480px), and sits just under them on the narrowest screens. Version and server name drop down below. Tightens the vertical rhythm.
Use a single two-row grid at every breakpoint instead of a separate mobile arrangement: logo top-left, uptime/license status top-right, version bottom-left, and name + notifications + account menu bottom-right. The logo column is minmax(0,1fr) and the logo image is max-w-full so it shrinks on narrow screens rather than overflowing, keeping the desktop structure responsive.
The compact-header rework renamed the root grid class to unraid-header-shell and its areas to uh-meta-right / uh-logo / uh-version / uh-nav-right (with uh-name nested), but Header.test.ts still asserted the old unraid-header-grid / uh-meta / uh-status / uh-actions names, failing CI. Update the layout assertion to the current structure; the no-overlap invariant is unchanged.
The consolidated header renders inside webgui's #header, which defines semantic classes that collide with Tailwind utilities: '#header .block' paints a gray box (background var(--gray-120) + padding + float) and '#header .text-right' flips alignment to left and floats/pads. The logo link used the 'block' utility, so it got the gray rectangle; two status/description nodes used 'text-right'. Swap to non-colliding equivalents: logo link uses 'flex' (still block-level, no #header override), and 'text-right' -> 'text-end'. Removing 'block' also fixes the same rectangle on the legacy <7.3 header, which shares HeaderLogo.
The header web component mounts client-side after its module bundle loads, so its content snapped in on page load. Add a one-shot CSS fade-in on the shell (which is only created when Vue mounts), with a prefers-reduced-motion guard. JS-free so it can never leave the header stuck hidden.
A fade on every full page load reads as lag in a multi-page app. Replaced by a server-rendered light-DOM logo fallback in webgui Header.php (paints before the web component mounts, then is replaced), which removes the pop-in without a fade.
The header had two banner-gradient mechanisms: an edges-only #header.image::before (shows the banner image in the middle, fades only the left/right for text legibility) and a full-coverage .unraid-banner-gradient-layer div. A theme-class refactor dropped the .has-banner-gradient trigger, killing the edge gradient and leaving the full-coverage overlay as the only active one -- so the gradient darkened the entire banner image. Re-apply .has-banner-gradient on <html> from the theme store (mirrors the dark class) so the intended edge gradient activates, and remove the redundant full-coverage div from both the consolidated and legacy headers.
The banner-gradient legibility overlay used a #header.image::before edge gradient in main.css, but the Vue build scopes those rules to .unapi #header.image::before. #header is the webgui element outside the .unapi scope, so the selector never matched and the overlay never rendered -- leaving the uptime/registration line, server name, and account controls low-contrast on a bright banner. Render the gradient as an in-scope .unraid-banner-gradient-layer div inside Header.standalone.vue (the class main.css already paints with var(--banner-gradient), self-gating when the gradient is off), positioned behind the content columns so the right of the header darkens like the legacy header.
Below sm the uptime and registration state stacked cramped on the right, making the header taller than the banner image (letterbox bars) and wasting vertical space. Make the meta row a full-width top strip that splits uptime to the left and the registration state to the right on one line. Over a banner image the text uses white + a drop-shadow for legibility (the right-only banner gradient never reached the left-aligned uptime); on a solid-color header the theme text color is kept. Collapses to the existing right-aligned cluster at sm+.
Re-apply -webkit-text-size-adjust:100% on the .unapi scope root so phones stop inflating large single-column blocks (the notifications sidebar) beyond their authored sizes. Tighten notification item + timestamp text sizes and drop the oversized sidebar title.
## Summary Version dropdown rows now apply matching semantic foreground and background colors on hover and keyboard focus, preserving readable contrast in White and Azure themes. ## Why This Exists The version dropdown used theme-specific hover backgrounds without explicitly setting the corresponding foreground color. In light themes, that could leave hovered text unreadable. ## Resolution Use the shared accent and accent-foreground semantic tokens for both hovered and focused version rows. This keeps the interaction state aligned with the active theme rather than relying on gray color utilities. ## Reviewer Considerations - The change is deliberately scoped to the two copyable version rows; it does not alter the shared dropdown primitive or unrelated menus. - Hover and keyboard-focus states receive the same token pair for consistent pointer and keyboard contrast. ## Behavior Changes - Hovered or focused Unraid OS and API version rows remain legible in White and Azure themes. ## Implementation Summary - Replace fixed gray hover backgrounds with semantic accent background and foreground utility classes in the header version dropdown. ## Verification - `pnpm --dir web run pretest` (passed; UI build completed with pre-existing TypeScript diagnostics in `Accordion.vue`) - `pnpm --dir web exec vitest run __test__/components/HeaderOsVersion.test.ts` (3 passed) - `pnpm --dir web exec eslint src/components/Header/HeaderVersion.vue` (passed) - `pnpm --dir web exec prettier --check src/components/Header/HeaderVersion.vue` (passed) - `git diff --check` (passed) ## Risk Low; the change only applies established semantic colors to existing interaction states.
elibosley
force-pushed
the
feat/consolidated-header
branch
from
July 22, 2026 12:55
541b2b4 to
8f89e69
Compare
The state-error account trigger rendered the 'Fix Error' label next to the alert icon, crowding the compact mobile header. Hide the label below sm so mobile shows just the alert icon (and hamburger); the full label stays at sm+.
In compact header placements the uptime rendered the full breakdown
("Uptime 8 days 12 hours 34 minutes"), crowding the meta row. Add a
largest-unit-only short diff to the dateTime helper and, when shortText is set,
display just that ("Uptime 8 days") while surfacing the full uptime and boot
date in the hover title tooltip. Non-compact placements are unchanged.
Replace the native title tooltip on the short uptime with a styled @unraid/ui Tooltip that shows the full uptime breakdown and boot date on hover. Forward attrs onto the trigger element (inheritAttrs: false) so the display keeps its classes; expire/full placements keep their plain native title.
The top meta row (uptime/registration/purchase) sat flush against the right edge while the account trigger below has 8px of internal right padding, so the rows' right edges did not line up. Give the desktop meta row a matching 0.5rem right inset (reset on mobile where it goes full-width).
Two header alignment fixes: - Merge the logo and version into one vertically-centered block instead of floating the logo in the middle grid band with the version pinned to the bottom, which left an awkward gap above the logo on desktop. The group now centers against the full header (matching the legacy header); on mobile it sits below the full-width meta strip. - Drop the mobile meta row's left padding so the uptime lines up with the logo's left edge (keep the right padding for the registration state). Update the layout test for the merged logo/version block.
On mobile the whole header content was vertically centered, leaving too much padding above the uptime. Stretch the host to the full header height and let the shell fill it, so the meta strip sits just below the top and the version mirrors it just above the bottom, with the logo centered in the middle row and aligned to the account controls. The version is wrapped so it can be floated out of the logo block's flow (the block goes static on mobile) and anchored bottom-left; otherwise the logo+version stack can't center-align against the taller controls. Desktop keeps the centered logo+version group.
The mounted logo re-rendered the webgui boot logo's <img> as a fresh <img> of the same src. That asset ships without Cache-Control, so the browser revalidates it over the network on mount, blanking the logo for a frame. Inline the gradient SVG (identical paths to the theme variant, identical gradient to the asset) so the mounted logo paints immediately with no network round-trip. Same rendering, no fetch.
… flash) The mount engine cleared each element (replaceChildren) before rendering, but the Vue render can paint a frame or more later, leaving the element visibly empty in between — a blank flash, most noticeable on the header where a server-rendered boot logo is replaced. Render into a fresh, layout-transparent (display: contents) child container appended after any existing placeholder so Vue still gets a clean container to mount into, keep the placeholder visible until the real content lands, then remove it via a MutationObserver. Elements without a placeholder keep the original clear-then-render path.
Apply non-blocking findings from the consolidated-header AI review: - Only take the display:contents buffer path for a meaningful placeholder (element or non-whitespace text), so multi-line custom-element whitespace no longer changes the mount path for every other component. - Capture the 5s safety-net timer and clear it once the observer swaps in real content; tear down the observer, timer, and buffer on unmount. - Type headerLogoStyle as HeaderLogoStyle | '' instead of collapsing to string. - Document the 55% banner-gradient split, the sm/639.98px breakpoint, and the placeholder fallback constant; correct the useServerProp docstring. - Add mount-engine tests for meaningful-vs-whitespace placeholder handling.
The webgui version gate ships the consolidated <unraid-header> on 7.4+; update the matching code comments so they don't drift from the gate.
Caught by AI review: main.css still read 7.3+ while the gate and the other comments moved to 7.4+.
Contributor
🔄 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! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Consolidates the Unraid header into a single web component (
<unraid-header>,Header.standalone.vue) that owns the entire header and its responsive layout, and fixes the broken mobile header where the "Reboot Required for Update" banner, the server-name title, and the notification bell overlapped on narrow screens.Gated to Unraid 7.3+; older releases fall back to the existing multi-component header. The gate lives in
webguiHeader.php(see companion PR).Before: header split across components
HeaderOsVersion.standalone.vue(<unraid-header-os-version>)Header.php#array-usage-sidenavHeader.phpUserProfile.standalone.vue(<unraid-user-profile>)myservers2.phpUserProfilewasabsolute top-0 right-0 pl-[30%]positioned over the logo/version area. On narrow screens the reboot banner (left) and the title + bell (absolutely positioned right) collided.After: one component, one layout
<unraid-header>composes all of it in a single mobile-first flex flow:flex-col— logo/version/banner stacks above server-status/name/bell/avatar (no overlap; the bell/avatar rowflex-wraps).sm+:flex-row … justify-between— logo cluster left, account cluster right (the prior desktop look).No cluster is absolutely positioned anymore, which is the actual fix.
Reuse / no duplication
Header/HeaderVersion.vue, now used by both<unraid-header>and the legacyHeaderOsVersion.standalone.vue— single source of truth.Notifications/Sidebar.vue), and the account dropdown are the existingUserProfile/*children, reused as-is.Header/useServerProp.ts.Backwards compatibility (7.3+ gate)
<unraid-header-os-version>+<unraid-user-profile>components are unchanged and remain the< 7.3fallback.version_compare($osVersion, '7.3', '>=')inwebguiHeader.php(companion PR), mirroring the docker-overview 7.3+ approach.Coordination with persistent-notifications (#2033)
The consolidated header does not mount the deprecated
CriticalNotificationspopover. Per #2033 (OS-471) persistent/"pinned" alerts now live in the notification bell, so the header uses only the surviving bell (Notifications/Sidebar.vue) and does not reintroduce the popover being retired.Testing
Header.test.ts(new): renders all regions in one component; hydrates server store; asserts the responsive layout invariant (mobile-firstflex-col→sm:flex-row, no absolute-positioned cluster); LAN-IP copy.component-registry.test.tspriority-order assertion.HeaderOsVersion/UserProfiletests still pass unchanged (extraction preserves output).pnpm buildsucceeds —<unraid-header>compiles to its own chunk;HeaderVersionis a shared chunk.Note
Live mobile rendering should be spot-checked once the dev server is running (I don't start it per repo convention). Companion webgui PR wires the mount + gate.
Summary by CodeRabbit