feat(ui): rework shared header into a single app-bar#352
Merged
Conversation
Collapse the two-row header (logo/utility row + nav row) into one app-bar, shared by both the ExtJS shell and the SolidJS /ui shell: - Primary nav (Time tracking, Overview, Evaluation) stays inline; secondary and role-gated items (Extras, Billing, Administration) plus Settings and Help fold into a "More" disclosure menu, with icon+text for Settings and Help. - The segmented System/Light/Dark toggle becomes a single theme-cycle button. A 3-state control can't use aria-pressed, so the current mode and next action are kept in a live-updating accessible name (WCAG 4.1.2). - The Month worktime badge now shows person-days only (e.g. "18.5 PT"); the hours move to the title. Today/Week stay in H:MM. (formatDays added to both the ExtJS main.js and the SolidJS header.ts, with a unit test.) - Below 880px the nav collapses behind a hamburger drawer (WCAG 1.4.10 reflow); the Month badge stays visible down to 480px. 44px targets and focus-visible rings throughout. Menu/drawer behaviour is wired framework-neutrally in a new header-behavior.html.twig (included by the header partial), theme cycling in theme-init.html.twig — both shells get identical behaviour. e2e: the secondary nav links now live in the "More" menu, so navigation helpers open it before clicking; goToSettings/Admin/BillingPage and the affected assertions are updated accordingly. Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the application header into a single app-bar layout, introducing a 'More' overflow menu for secondary items, a mobile hamburger drawer, and a single-button theme cycle. It also updates the Month worktime badge to display person-days only. Feedback on these changes highlights several areas for improvement: resolving accessibility issues caused by using menu roles without arrow key navigation, removing redundant event.stopPropagation() calls, preventing potential memory leaks in SPA contexts by querying DOM elements dynamically in global listeners, and ensuring the mobile user status badge is updated during JS polling.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #352 +/- ##
=========================================
Coverage 83.49% 83.49%
Complexity 2664 2664
=========================================
Files 174 174
Lines 7266 7266
=========================================
Hits 6067 6067
Misses 1199 1199
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:
|
…verflow nav - Close the "More" menu when a link inside it is chosen (SPA links don't reload, so it stayed open) and when the pointer leaves it / focus moves out. - Replace the ambiguous half-circle "system" theme icon with a distinct monitor glyph, so the three cycle states (monitor / sun / moon) are clearly told apart. - Priority-overflow nav: as the viewport narrows the primary items fold into "More" and the Today/Week (then Month) badges collapse, so the theme/user/ logout cluster is never clipped before the hamburger takes over. The hamburger breakpoint drops to 600px; above it the JS governs the bar dynamically (ResizeObserver, no-JS fallback unchanged). Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
…fixes Addresses review feedback on the app-bar header: - Theme icon now reflects the active mode. The icons are <svg> (SVGElement), which doesn't reflect the .hidden IDL property to the attribute, so the CSS [hidden] rule never toggled and the System icon showed in every mode. Toggle the attribute via set/removeAttribute instead. - "More" is now a genuine overflow control, not a fixed bucket. All nav items (incl. Extras/Billing/Administration/Settings/Help) start inline; the script folds them into "More" from the end only as the viewport narrows, and the whole control is hidden when nothing has overflowed. Settings/Help show their icon only once folded into the menu (plain text inline). - "More" opens on hover (in addition to click/keyboard) and no longer closes in the gap between button and menu: the menu is flush (top:100%) and a short close delay bridges the pointer move; it still closes on link-click, on leaving the control, on click-outside and on Escape. e2e: clickHeaderNav() reaches a nav link whether it's inline or folded into "More" (width-dependent); presence assertions use toBeAttached rather than asserting placement. Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
- Drawer logout used .badge-logout, duplicating the desktop one and tripping Playwright strict-mode in login/navigation specs; give it its own .drawer-logout class (shared styling). - ExtJS shell only: the header is adopted into a fixed-height Viewport "north" region (#header/#header-body) whose overflow clipped the "More" dropdown and mobile drawer — set those containers to overflow:visible (no-op on /ui). - openMoreMenu (e2e) hovers to open: a real click moves the pointer onto the button (hover-opens) and then toggles it shut, so hover is the reliable open. - Review findings: drop role="menu"/menuitem" (disclosure pattern, no arrow-key contract was implemented); remove redundant event.stopPropagation(); keep the mobile drawer's user badge in sync via shared .js-user-badge/.js-user-name in both header.ts and main.js (status polling now updates desktop + drawer). Verified with the full local e2e stack: navigation + login specs green, including Settings nav at 1024px on the ExtJS shell. Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
|
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
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.
Reworks the shared page header (rendered by both the ExtJS shell and the SolidJS
/uishell) from two rows into a single app-bar. Design picked interactively: Layout C (app-bar + “More”) · theme cycle button · Settings/Help icon+text.What changed
aria-pressed, so the current mode + next action live in a live-updating accessible name (WCAG 4.1.2); the icon reflects the current mode.18.5 PT); the hours move to thetitle. Today/Week stayH:MM.formatDaysadded to bothassets/js/main.js(ExtJS) andfrontend/src/header.ts(SolidJS), with a unit test.focus-visiblerings,Escape/click-outside close, reduced-motion honoured.templates/partials/header-behavior.html.twig(included by the header partial); theme cycle intheme-init.html.twig. Both shells get identical behaviour; the ExtJS north region reads the header height dynamically, so the one-row collapse needs no height changes.e2e
Secondary nav links now live in the “More” menu, so
navigation.tsgainsopenMoreMenu()andgoToSettings/Admin/BillingPageopen it before clicking; the affected visibility assertions innavigation.spec.ts/error-handling.spec.tswere updated.Verified locally: Vite + Encore production builds, frontend typecheck/lint/84 tests, Twig lint, and a real-CSS visual harness (light/dark/mobile, More menu, drawer). I’ll deploy a review build to tt.netresearch.de next.