ReviewOS

stacks/bench-review

Mobile: the site header has no navigation below lg

#90
Closed glennmichael123 opened this 9 days ago ·
9 days ago

What

resources/components/Bench/BenchHeader.stx puts hidden lg:flex on all three of its regions — the nav links, the search box and the auth buttons — and offers no hamburger or any other mobile affordance. Below the lg breakpoint the header is the logo and nothing else.

Verified on the built /judges at 375×812: logo, then straight into the hero. No way to reach Judges, Court Houses, Write a Review, Reviews, search, sign in or sign up from the header.

Why it blocks the mobile track

The iOS app (#79) is a native shell over this same site, so whatever the site does at phone width is what the app does. Shipping it in this state means an app whose only navigation is in-page links.

Starting point

resources/components/Bench/MobileMenu.stx already exists — 67 lines, a slide-over with search, the four main links, an authed section and register/login — but nothing includes it and it does not work as written. It is annotated in-file now rather than deleted, because it records the intended menu.

What has to be fixed before it can be included:

  • isOpen and isAuthenticated are not defined in its own scope and no parent passes them. A component const plus a useStore('auth') reference has to supply both.
  • :if="isOpen" is the wrong binder for a toggle that is false on first render — bindIf drops it and never re-renders. Use :show with a static style="display:none" (stx-standards §7.3, gate check 19).
  • isAuthenticated is read both with and without parens in the same file.
  • <StxLink to="/logout"> points at a route that does not exist. Logout is a store action; BenchHeader calls auth.logout().
  • $dispatch('close') is unverified against this stx version.

Plus the header side: a hamburger button below lg, the open/close state, and a focus trap consistent with ConfirmHost's.

Found while

Auditing stx-standards §10.13 (delete unreferenced components) — the audit surfaced MobileMenu as dead, and checking whether it was safe to delete turned up the gap it was meant to fill.

9 days ago

Verified the fix holds, and found an adjacent defect while doing it (39533b53).

#90 itself is good. Built /judges at 375×812: hamburger present, and the panel carries search, Find Judges, Court Houses, Write a Review, Reviews, Submit a Judge, then the authed section (Notifications, Profile, My reviews, Settings, Sign out). It is role="dialog" + aria-modal with :show and a static display:none, which is the right shape.

The user dropdown beside it was not. It carried role="menu" with seven role="menuitem" children and aria-haspopup="true" on the trigger. That pattern promises roving focus driven by arrow keys — a screen reader announces it — and there is no tabindex anywhere in the file, so it was not merely unimplemented but structurally impossible. Same conclusion the judge profile's "More" menu reached in #58: it is a disclosure of ordinary links, so the roles are gone, aria-expanded stays, and the items remain in normal tab order.

Gate check 25 had passed this. It asked that a file declaring a roving-focus role contain a @keydown somewhere — file-scoped deliberately, since the handler usually sits on children inside a :for — and BenchHeader's search box has one, so the file passed while the dropdown had nothing. It now also requires tabindex management, since that is what roving focus is.

Sign in to comment on this issue.