From b9fa8dd1eaa28749eb3d4b6ca7d27904dc749355 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 20 Aug 2026 10:42:00 +0200 Subject: [PATCH 1/4] fix(frontend): move the topbar controls into the drawer below 768px At a phone viewport the left nav collapsed into a working hamburger drawer but the topbar did not adapt. `.app-topbar` is a fixed --cudly-topbar-h box, and a `header { flex-direction: column }` rule in the 768px block stacked the brand, the Provider/Account filter chips and #user-info inside it. Every row past the first overflowed past the gradient and painted its white foreground straight onto the page content: the signed-in email, the admin badge, API Docs, Feedback, Logout and both filter chips read as faint ghost text over the dashboard, and none of them were placed anywhere reachable. Measured pre-fix at 390x844: eleven topbar descendants painting below the topbar's 56px bottom edge, the lowest at y=259. The two regions now move into the existing sidebar drawer instead of gaining a second mechanism. app.ts:syncHeaderPlacement relocates the #topbar-filters and #user-info nodes themselves between the topbar and a new #sidebar-extras slot, driven by a matchMedia listener on the same 768px breakpoint the CSS uses, so every chip handle, logout binding and piece of filter state survives the move. The topbar copies are laid out away for the moment before that script runs, so nothing is left painting behind the content. Inside the drawer the chips fill the rail and truncate long account labels, the popover is capped to the rail width, #user-info is re-coloured for the light surface, and every action meets the 44x44 touch minimum. Widening past the breakpoint hands the controls back to the topbar and drops the drawer's body scroll lock. Verified with a Playwright spec at a 390x844 viewport that measures real layout geometry rather than DOM presence, since jsdom resolves no stylesheets and would stay green either way: 8 tests, all 8 failing against the pre-fix bundle and passing after. It asserts that nothing parented to the topbar paints outside it, that the topbar stays as tall as --cudly-topbar-h, that the controls sit off-screen until the drawer opens, that all seven of them are visible and inside the viewport once it does, that the tappable ones are at least 44x44, that the Provider chip opens an on-screen popover and actually applies the filter, and that widening restores the topbar. Also confirmed by eye at 390px on the built bundle. Full suites green: jest 2890, playwright 44, eslint 0 errors, tsc clean. The jest side covers placement only, and the issue-#10 assertion that locked in the old `#user-info { flex-wrap: wrap }` band-aid is replaced by ones describing the relocation. Deferred: the profile modal opened from the drawer leaves the drawer open behind it (it renders above at z-index 1000, so it is usable); the 900px icon-only sidebar collapse between 769px and 900px is untouched. Closes #1779 --- frontend/src/__tests__/mobile-nav.test.ts | 105 +++++++- frontend/src/__tests__/responsive.test.ts | 35 ++- frontend/src/__tests__/setup.ts | 19 ++ frontend/src/app.ts | 63 +++++ frontend/src/index.html | 7 + frontend/src/styles/layout.css | 7 + frontend/src/styles/responsive.css | 111 +++++++-- .../tests-e2e/mobile-header-drawer.spec.ts | 230 ++++++++++++++++++ 8 files changed, 555 insertions(+), 22 deletions(-) create mode 100644 frontend/tests-e2e/mobile-header-drawer.spec.ts diff --git a/frontend/src/__tests__/mobile-nav.test.ts b/frontend/src/__tests__/mobile-nav.test.ts index 694ec5c31..481894e78 100644 --- a/frontend/src/__tests__/mobile-nav.test.ts +++ b/frontend/src/__tests__/mobile-nav.test.ts @@ -5,23 +5,39 @@ * aria-expanded toggle, focus management, body scroll-lock class. */ -import { setupMobileNav } from '../app'; +import { setupMobileNav, syncHeaderPlacement } from '../app'; function buildDOM(): void { document.body.innerHTML = ` - +
+ +

CUDly

+
+
+ + + API Docs + + +
+
`; } +function hamburgerBtn(): HTMLButtonElement { + return document.getElementById('hamburger-btn') as HTMLButtonElement; +} + describe('setupMobileNav', () => { beforeEach(() => { buildDOM(); @@ -151,3 +167,80 @@ describe('setupMobileNav', () => { expect(() => setupMobileNav()).not.toThrow(); }); }); + +/** + * Placement of the topbar controls (issue #1779). + * + * These assert DOM parentage only. Whether the relocated controls are + * visible, on-screen and large enough to tap is a layout question jsdom + * cannot answer -- tests-e2e/mobile-header-drawer.spec.ts measures that in + * a real browser at a 390px viewport. + */ +describe('syncHeaderPlacement', () => { + beforeEach(buildDOM); + afterEach(() => { + document.body.innerHTML = ''; + }); + + const projected = (): HTMLElement[] => [ + document.getElementById('topbar-filters')!, + document.getElementById('user-info')!, + ]; + + function parentIds(): (string | undefined)[] { + return projected().map(el => el.parentElement?.id); + } + + test('narrow moves the filters and the account actions into the drawer slot', () => { + syncHeaderPlacement(true); + expect(parentIds()).toEqual(['sidebar-extras', 'sidebar-extras']); + }); + + test('widening restores both to the topbar, in header order', () => { + syncHeaderPlacement(true); + syncHeaderPlacement(false); + + const topbar = document.querySelector('.app-topbar')!; + expect(projected().every(el => el.parentElement === topbar)).toBe(true); + const ids = Array.from(topbar.children).map(el => el.id).filter(Boolean); + expect(ids.indexOf('topbar-filters')).toBeLessThan(ids.indexOf('user-info')); + }); + + test('repeated narrow calls do not re-append the same nodes', () => { + syncHeaderPlacement(true); + const marker = document.createElement('span'); + projected()[0]!.appendChild(marker); + // A redundant appendChild would tear the subtree out and back in, + // dropping focus and closing any open chip popover. + syncHeaderPlacement(true); + expect(marker.isConnected).toBe(true); + expect(document.getElementById('sidebar-extras')!.children).toHaveLength(2); + }); + + test('no-op when the drawer slot is absent', () => { + const topbar = document.querySelector('.app-topbar')!; + document.getElementById('sidebar-extras')!.remove(); + expect(() => syncHeaderPlacement(true)).not.toThrow(); + expect(projected().every(el => el.parentElement === topbar)).toBe(true); + }); + + test('clicking an account action in the drawer closes it', () => { + setupMobileNav(); + syncHeaderPlacement(true); + hamburgerBtn().click(); + document.getElementById('logout-btn')!.dispatchEvent( + new MouseEvent('click', { button: 0, bubbles: true }), + ); + expect(document.body.classList.contains('sidebar-open')).toBe(false); + }); + + test('using a filter chip in the drawer leaves it open', () => { + setupMobileNav(); + syncHeaderPlacement(true); + hamburgerBtn().click(); + const chip = document.createElement('button'); + document.getElementById('topbar-filters')!.appendChild(chip); + chip.dispatchEvent(new MouseEvent('click', { button: 0, bubbles: true })); + expect(document.body.classList.contains('sidebar-open')).toBe(true); + }); +}); diff --git a/frontend/src/__tests__/responsive.test.ts b/frontend/src/__tests__/responsive.test.ts index 9af1e3eff..2f8603f4d 100644 --- a/frontend/src/__tests__/responsive.test.ts +++ b/frontend/src/__tests__/responsive.test.ts @@ -41,10 +41,41 @@ describe('responsive.css nav wrap rules', () => { expect(tabsRule?.[0] ?? '').not.toMatch(/overflow-x/); }); - it('wraps #user-info inside the ≤768px block so header children do not overflow', () => { + /** + * Issue #10 wrapped #user-info at ≤768px to stop it overflowing the + * header. That never worked: the topbar is a fixed --cudly-topbar-h box, + * so the extra rows painted over the page content instead (issue #1779). + * The regions are relocated into the drawer now, and the topbar copies + * are laid out away rather than left to paint behind the content. + * + * Source-text assertions only -- whether the relocated controls are + * actually visible and tappable is measured in a real browser by + * tests-e2e/mobile-header-drawer.spec.ts. + */ + it('takes #user-info and #topbar-filters out of the topbar at ≤768px', () => { const match = css.match(/@media\s*\(max-width:\s*768px\)\s*{([\s\S]*?)\n}/); expect(match).not.toBeNull(); const body = match?.[1] ?? ''; - expect(body).toMatch(/#user-info\s*{[\s\S]*?flex-wrap:\s*wrap/); + expect(body).toMatch( + /\.app-topbar\s*>\s*#topbar-filters,\s*\.app-topbar\s*>\s*#user-info\s*{[\s\S]*?display:\s*none/, + ); + }); + + it('styles both regions for the drawer slot at ≤768px', () => { + const match = css.match(/@media\s*\(max-width:\s*768px\)\s*{([\s\S]*?)\n}/); + expect(match).not.toBeNull(); + const body = match?.[1] ?? ''; + expect(body).toMatch(/\.app-sidebar-extras\s*{[\s\S]*?display:\s*flex/); + expect(body).toMatch(/\.app-sidebar-extras #topbar-filters/); + expect(body).toMatch(/\.app-sidebar-extras #user-info/); + }); + + it('does not stack the topbar into a column at ≤768px', () => { + const match = css.match(/@media\s*\(max-width:\s*768px\)\s*{([\s\S]*?)\n}/); + expect(match).not.toBeNull(); + const body = match?.[1] ?? ''; + // The `header { flex-direction: column }` rule was the overflow source. + expect(body).not.toMatch(/(^|\n)\s*header\s*{/); + expect(body).toMatch(/\.app-topbar\s*{[\s\S]*?justify-content:\s*flex-start/); }); }); diff --git a/frontend/src/__tests__/setup.ts b/frontend/src/__tests__/setup.ts index 329de7248..a916916ca 100644 --- a/frontend/src/__tests__/setup.ts +++ b/frontend/src/__tests__/setup.ts @@ -82,6 +82,25 @@ if (typeof globalThis.structuredClone === 'undefined') { }) as typeof structuredClone; } +// jsdom evaluates no CSS media queries and so ships no window.matchMedia, +// which setupMobileNav() calls to decide where the topbar controls live. +// Stub it as never-matching: jsdom has no viewport width to honour, so the +// desktop branch is the only honest default. The narrow branch is asserted +// by calling syncHeaderPlacement directly, and measured for real in the +// Playwright suite, which has actual layout. +if (typeof window.matchMedia !== 'function') { + window.matchMedia = ((query: string) => ({ + matches: false, + media: query, + onchange: null, + addListener: jest.fn(), + removeListener: jest.fn(), + addEventListener: jest.fn(), + removeEventListener: jest.fn(), + dispatchEvent: jest.fn(() => false), + })) as unknown as typeof window.matchMedia; +} + // Mock alert and confirm global.alert = jest.fn(); global.confirm = jest.fn(() => true); diff --git a/frontend/src/app.ts b/frontend/src/app.ts index bbeb3f183..a9e5bc53f 100644 --- a/frontend/src/app.ts +++ b/frontend/src/app.ts @@ -653,6 +653,45 @@ async function handleFanOutExecute(buckets: FanOutBucket[]): Promise { } } +/** + * Drawer breakpoint. Must stay in lockstep with the `max-width: 768px` block + * in styles/responsive.css, which lays these same regions out of the topbar. + */ +const MOBILE_NAV_QUERY = '(max-width: 768px)'; + +/** + * Topbar regions with no room in the topbar below the breakpoint, listed in + * the order they appear in the header so restoring them re-appends correctly. + */ +const DRAWER_PROJECTED_IDS = ['topbar-filters', 'user-info'] as const; + +/** + * Move the global filter chips and the account actions between the topbar + * and the drawer (issue #1779). Relocation rather than duplication: the + * chips and the logout/profile handlers are bound to these exact nodes, and + * moving a node keeps its listeners and state. + * + * Idempotent, and a no-op when either endpoint is absent. + */ +export function syncHeaderPlacement(isNarrow: boolean): void { + const topbar = document.querySelector('.app-topbar'); + const extras = document.getElementById('sidebar-extras'); + if (!topbar || !extras) return; + + const target = isNarrow ? extras : topbar; + const elements = DRAWER_PROJECTED_IDS + .map(id => document.getElementById(id)) + .filter((el): el is HTMLElement => el !== null); + + // Re-append all of them whenever any one is misplaced, so the restored + // header keeps DRAWER_PROJECTED_IDS order. Appending an element that is + // already in place would tear its subtree out and back in, dropping focus + // and closing an open chip popover, so the whole pass is skipped instead. + const misplaced = elements.some(el => el.parentElement !== target); + if (!misplaced) return; + for (const el of elements) target.appendChild(el); +} + /** * Wire the mobile navigation drawer (hamburger button + overlay + Escape). * @@ -719,6 +758,30 @@ export function setupMobileNav(): void { } }); + // Projected topbar controls: place them for the current width, then follow + // the breakpoint. Listeners are bound once here, never per relocation. + const mediaQuery = window.matchMedia(MOBILE_NAV_QUERY); + syncHeaderPlacement(mediaQuery.matches); + mediaQuery.addEventListener('change', event => { + syncHeaderPlacement(event.matches); + // Widening past the breakpoint leaves the drawer's scroll lock on + // with no visible drawer and no hamburger to dismiss it. + if (!event.matches && document.body.classList.contains('sidebar-open')) { + closeDrawer(); + } + }); + + // Account actions inside the drawer navigate or end the session, so they + // close it. Delegated from the slot, which outlives every relocation; the + // filter chips are deliberately excluded so picking a filter keeps the + // drawer open for the next one. + const extras = document.getElementById('sidebar-extras'); + extras?.addEventListener('click', (e: MouseEvent) => { + const target = e.target as HTMLElement | null; + if (!target?.closest('#user-info a, #user-info button')) return; + if (document.body.classList.contains('sidebar-open')) closeDrawer(); + }); + // Clicking any sidebar link closes the drawer (SPA navigation) sidebar.querySelectorAll('.tab-btn').forEach(link => { link.addEventListener('click', (e: MouseEvent) => { diff --git a/frontend/src/index.html b/frontend/src/index.html index 1c659dacd..cb019881a 100644 --- a/frontend/src/index.html +++ b/frontend/src/index.html @@ -87,6 +87,13 @@

CUDly

Admin + +