Repository navigation
Add a phone bottom tab bar - #240
Conversation
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>
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Note 🎁 Summarized by CodeRabbit Free
Comment |
Codex review (gpt-6-luna)Independent read-only review of BlockerNone. Should-fixNone. Nit
Verdict: No confirmed functional defects found; add browser coverage for the keyboard interaction. |
Sonnet 5.5 review (claude-sonnet-5-5)Independent read-only review of BlockerNone Should-fix
Nit
I read the code only. I did not run the app, so layout at 390/820/1440, contrast of 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>
|
Reply to the Sonnet review: fixed on
Also moved the bar to after the footer, move focus to main before hiding a focused tab, scoped |
|
Reply to the Codex review: added the browser coverage in 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
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, andnav.settingslabels 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
mdlater), which is the opposite of a phone tab bar. The breakpoint is therefore belowmd(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 insidemainget a matchingscroll-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
visualViewportoverlap counts as a keyboard only below 768px, only while a text field is focused, and only when the page is not pinch-zoomed (scaleat 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-topis 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:--native-keyboard-heightrun only below 768px.scroll-margin-bottomis scoped tomain [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.tsxbreakpoint classes (P2-4 moves the desktop nav fromlgtomd),OnboardingTour.tsx,sonner.tsx, and Home or History layout. This PR does not change locale strings. If that branch removes themd–lgmenu, the hamburger classes here (md:inline-flex lg:hidden,max-md:hidden) and themd:hiddentab bar need to be merged together.design/debateowl-animations—src/index.cssmotion and the menu dialog transition classes inAppLayout.tsx. Those class strings gainedmax-md:hiddenonly.feat/case-structure-v2— case output. Overlap is limited toAskPage.tsx/AskChat.tsxif that branch also edits the Ask layout.OnboardingTour.tsx(comment only here) andrecipes/auth.spec.ts, which now uses the tab bar on phones and the menu on iPad.nav.libraryand library copy. This PR reuses the key and does not edit locales.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:
npm run lint(eslint, knip, repo rules)npm run typecheck(app, agentic, core)npm testsrc/test/scripts/backend-status.test.tshook timeout inmkdtempunder/tmp. Unrelated to navigation. Same failure as before.npm run buildnpm run test:e2e:smoke(desktop Chromium)e2e/home-navigation.spec.tsande2e/admin-announcements.spec.ts(desktop Chromium)npm run test:edgewas not run.denois not installed. The draft was not merged.To show artifacts inline, enable in settings.