Skip to content

Add a phone bottom tab bar - #240

Merged
Programcode merged 3 commits into
mainfrom
design/bottom-tab-bar
Oct 27, 2026
Merged

Programcode merged 3 commits into
mainfrom
design/bottom-tab-bar

Conversation

@Programcode

@Programcode Programcode commented Oct 27, 2026 •

Copy link
Copy Markdown
Owner

Phones below 768px lose the hamburger and get a fixed bottom tab bar: Tools, Current debate, Library, and Settings. Those are the existing nav.tools, nav.currentDebate, nav.library, and nav.settings labels and the existing routes. History is not a tab. It stays on the Home utilities tile, in the header menu from 768px until the desktop nav, and in the desktop header.

Why 820px stays on the header menu

A four-tab bar fits an 820px iPad portrait width, but it does not fit this navigation. The menu at that width has five items, and dropping History would hide a destination that is still primary there. The header already has a Current debate row, so a Current debate tab would repeat it. The polish study treats 820 as a top-nav width (move the desktop links down to md later), which is the opposite of a phone tab bar. The breakpoint is therefore below md (width < 48rem). At 820 the hamburger and Current debate row are unchanged. At 1440 the header links are unchanged.

Selected tabs use the existing brand tint, not the mint success color. Mint stays the success color.

Clearance, safe area, keyboard

The bar pads with the existing --native-safe-* insets. Its measured height, including the home-indicator inset, is published as --phone-tab-reserve. Below 768px the app shell pads by that amount, the footer’s own safe-area padding is reduced so the two do not stack, and anchors inside main get a matching scroll-margin-bottom. Toasts use the same reserve from 0 to 767px. Sonner’s mobile offset only starts at 600px, so the desktop offset carries the reserve too. At 768px and up the reserve variable is unset.

A visualViewport overlap counts as a keyboard only below 768px, only while a text field is focused, and only when the page is not pinch-zoomed (scale at most 1). Overlap under 120px stays the URL bar. A real keyboard hides the bar, sets the reserve to 0, and sets --native-keyboard-height. If focus is on the bar when it hides, focus moves to main. The listener is not attached at 768px and up, and both inline variables are removed there.

Ask’s phone height is 100dvh - --app-main-top - reserve. --app-main-top is the measured bottom of the header and any banners, so an announcement cannot push the composer under the bar. With a maintenance banner at 390px that top was 205px against a 116px header.

Review follow-up

Asked for in the Sonnet and Codex comments, and fixed on dfadabee:

  • Toast offset covers the whole phone range, including 600–767px.
  • Keyboard listener and --native-keyboard-height run only below 768px.
  • Pinch-zoom does not hide the bar.
  • Ask accounts for announcement banners.
  • The bar is after the footer in DOM order. Press scale is off under reduced motion. scroll-margin-bottom is scoped to main [id]. History, billing, and admin still have no active tab.

Ask with a banner, and a toast at 700px:

Ask on a phone with an announcement above the composer
Ask on iPad portrait with an announcement, header menu kept
Ask on desktop with an announcement, header nav kept
Toast at 700px resting above the tab bar

After shots of Home, Current debate, Library, Settings, History, Generate, and Ask were refreshed at 390, 820, and 1440.

Likely conflicts

This branch only touches the nav shell, the phone Ask height, and the tests and docs that name the menu.

  • design/debateowl-polish — AppLayout.tsx breakpoint classes (P2-4 moves the desktop nav from lg to md), OnboardingTour.tsx, sonner.tsx, and Home or History layout. This PR does not change locale strings. If that branch removes the md–lg menu, the hamburger classes here (md:inline-flex lg:hidden, max-md:hidden) and the md:hidden tab bar need to be merged together.
  • design/debateowl-animations — src/index.css motion and the menu dialog transition classes in AppLayout.tsx. Those class strings gained max-md:hidden only.
  • feat/case-structure-v2 — case output. Overlap is limited to AskPage.tsx / AskChat.tsx if that branch also edits the Ask layout.
  • Onboarding — OnboardingTour.tsx (comment only here) and recipes/auth.spec.ts, which now uses the tab bar on phones and the menu on iPad.
  • Library rename — nav.library and library copy. This PR reuses the key and does not edit locales.
  • DebateOwl rename — the hardcoded “DebateLab” logo and footer in AppLayout.tsx. Those strings are untouched. The hunks are the hamburger class, the menu close at phone width, and the tab bar mount.

Checks

Re-run after the review fixes:

Check Result
npm run lint (eslint, knip, repo rules) passed
npm run typecheck (app, agentic, core) passed
npm test 666 passed, 1 failed: src/test/scripts/backend-status.test.ts hook timeout in mkdtemp under /tmp. Unrelated to navigation. Same failure as before.
npm run build passed
npm run test:e2e:smoke (desktop Chromium) 47 passed
e2e/home-navigation.spec.ts and e2e/admin-announcements.spec.ts (desktop Chromium) 15 passed, including keyboard hide/return, pinch-zoom, Ask with a banner, and the 700px toast

npm run test:edge was not run. deno is not installed. The draft was not merged.

To show artifacts inline, enable in settings.

Open in Web Open in Cursor 

Phones below 768px navigate with Tools, Current debate, Library, and Settings. The header menu stays from 768px until the desktop nav, so iPad portrait still includes History.

Co-authored-by: Gáborjáni-Szabó Ádám <Programcode@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Oct 27, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Note

🎁 Summarized by CodeRabbit Free

Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Essentials by visiting https://app.coderabbit.ai/settings/billing.

Comment @coderabbitai help to get the list of available commands.

@Programcode

Copy link
Copy Markdown
Owner Author

Codex review (gpt-6-luna)

Independent read-only review of git diff origin/main...HEAD at 7f5efdf, run via codex-cli 0.164.0 on the VPS. Comment only, not an approval.

Blocker

None.

Should-fix

None.

Nit

  • src/components/layout/phone-tab-bar.test.ts:4 tests the keyboard overlap calculation, but the changed behavior that hides the bar and adjusts its measured reserve has no browser coverage. Add an interaction check for the bar hiding and returning, and for the Ask composer remaining visible above the keyboard.

Verdict: No confirmed functional defects found; add browser coverage for the keyboard interaction.

@Programcode

Copy link
Copy Markdown
Owner Author

Sonnet 5.5 review (claude-sonnet-5-5)

Independent read-only review of git diff origin/main...HEAD at 7f5efdf, run via Claude Code 2.1.309 on the VPS. Comment only, not an approval.

Blocker

None

Should-fix

  • src/components/ui/sonner.tsx:19 – Sonner's mobileOffset only applies at 600px and below. The tab bar shows up to 767px. At 600–767px (large phones in landscape, small foldables) toasts use the desktop offset (24px plus safe-bottom). They sit on top of the bar and cover the tabs, because Sonner's z-index is far above 40. Fix: also pass the reserve into offset.bottom, e.g. calc(24px + max(var(--phone-tab-reserve,0px), var(--native-safe-bottom,0px))). That is safe on desktop because JS sets the reserve to 0 at ≥768px.
  • src/components/layout/PhoneTabBar.tsx:66-93 and src/index.css:1013 – --native-keyboard-height was only read before, never set. The bar is always mounted, so the keyboard/viewport listener now runs at every width and writes that variable. .native-main-content then adds 12% of the shrink as bottom padding on tablet and desktop too. This is untested, outside the stated scope, and it can't be switched off at ≥768px. Fix: skip the listener, the variable and the resize observer when matchMedia('(min-width:48rem)') matches, or drop the variable. The bar doesn't use it.
  • src/components/layout/PhoneTabBar.tsx:68-70 – The visualViewport heuristic also fires on pinch-zoom. keyboardOverlap(innerHeight, visualViewport.height, offsetTop) is layout − visual − offsetTop, and pinch-zoom shrinks the visual height. At about 1.5× zoom on an 844px viewport the overlap is about 280px, which is ≥120. The bar then hides and the page reserve drops to 0, with no keyboard open. A zoomed-in user loses navigation. Fix: ignore the overlap when viewport.scale > 1, or only treat it as a keyboard while an editable element has focus (document.activeElement is an input, textarea or contenteditable).
  • src/pages/AskPage.tsx:476 – The Ask container height is 100dvh − --app-header-bottom − --phone-tab-reserve. AppLayout renders the announcement banner and the other banners between the header and <main> (see the {announcements[0] && …} block after <PhoneTabBar />), and the calc doesn't subtract them. While a banner is showing, the composer's bottom edge is pushed down by the banner's height, so it starts partly under the bar. Fix: use a flex column from the layout root instead of a dvh calc, or measure the main content's top offset.

Nit

  • src/components/layout/AppLayout.tsx (the <PhoneTabBar /> placement, after </header>) – The bar comes before <main> in DOM order, so keyboard and screen-reader users on a phone tab through the bottom bar before the page content. It is also visually at the bottom. The skip link softens this. Consider rendering it after <footer> (it is fixed, so layout doesn't change) so tab order matches visual order.
  • src/components/layout/PhoneTabBar.tsx:120 – keyboardOpen && 'hidden' unmounts focus when a tab link is focused as the keyboard opens. This is minor, but it is worth keeping in mind for focus return.
  • src/index.css:1073-1077 – [id] { scroll-margin-bottom } is a global attribute selector with the same specificity as utilities. If this block is outside a @layer, it overrides any element-level scroll-margin-bottom. Scope it to main [id], or put it in @layer base.
  • src/components/layout/PhoneTabBar.tsx:21-24 – active:scale-[0.97] and the transform transition aren't gated for prefers-reduced-motion. It is small, but add motion-reduce:transition-none motion-reduce:active:scale-100, or confirm that a global reduced-motion rule already covers it.
  • src/components/layout/PhoneTabBar.tsx:56-60 – Active-tab logic is correct for the routes that exist: /app/ask and the other debate tools map to debate when a debate exists and to tools otherwise, and the exact-match test works for library and settings. /app/history, /app/billing (which redirects to settings) and the admin pages show no active tab. That is acceptable, but a one-line comment would stop it looking like an oversight. The new unit test covers only keyboardOverlap. That is fine under the repo's test rules, but nothing covers the active-tab mapping, and navSectionFor is already pure.
  • The locale labels fit: EN "Current debate", HU "Aktuális vita", ES "Debate actual". Each wraps to two lines at most in a 97px column at text-xs. Reusing nav.mobileNavigation ("Mobile navigation") as the landmark label is fine. The header menu's nav is not rendered below md, so there is no duplicate landmark.

I read the code only. I did not run the app, so layout at 390/820/1440, contrast of text-primary on bg-brand-tint, and real-device keyboard behaviour are unverified.

Verdict: Sound overall, but fix the toast-over-bar overlap at 600–767px, the pinch-zoom false "keyboard open", the keyboard variable leaking to ≥768px, and the Ask height under banners before merging.

The toast offset now includes the bar up to 767px. The keyboard listener runs only below 768px, ignores pinch-zoom, and Ask sizes itself from the chrome above the page so a banner cannot push the composer under the bar.

Co-authored-by: Gáborjáni-Szabó Ádám <Programcode@users.noreply.github.com>
@cursor

cursor Bot commented Oct 27, 2026

Copy link
Copy Markdown

Reply to the Sonnet review: fixed on dfadabee.

  • Toasts: both offset.bottom and mobileOffset include the tab reserve, so the 600–767px range uses it. At 700px the toaster’s used bottom was 82px (24px plus the 58px bar) and the toast rests above the tabs.
  • Keyboard variable: the visualViewport listener is attached only below 768px. --native-keyboard-height and the inline reserve are removed at md and up, so main does not grow a keyboard pad there.
  • Pinch-zoom: a shrink counts as a keyboard only when a text field is focused and visualViewport.scale is not above 1.
  • Ask banners: the phone height is 100dvh - --app-main-top - reserve. --app-main-top is the measured bottom of the header and banners. With a maintenance banner it was 205px against a 116px header, and Send stays above the bar.

Also moved the bar to after the footer, move focus to main before hiding a focused tab, scoped scroll-margin-bottom to main [id], and disabled the press scale under reduced motion. History, billing, and admin still have no active tab.

@cursor

cursor Bot commented Oct 27, 2026

Copy link
Copy Markdown

Reply to the Codex review: added the browser coverage in e2e/home-navigation.spec.ts.

The bar hides when a text field’s keyboard opens and comes back when it closes. Pinch-zoom does not hide it. The Ask composer stays above the bar while an announcement is showing. A toast at 700px rests above the tabs. iPad width does not keep a keyboard pad.

# Conflicts:
#	e2e/admin-announcements.spec.ts
#	e2e/home-navigation.spec.ts
#	e2e/navigation-performance.spec.ts
#	src/components/layout/AppLayout.tsx
@Programcode
Programcode marked this pull request as ready for review October 27, 2026 11:24
Copilot AI balanced review requested due to automatic review settings October 27, 2026 11:24
@Programcode
Programcode merged commit f1fa967 into main Oct 27, 2026
2 of 9 checks passed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants