fix(mobile): improve native app feel on mobile browsers#121
fix(mobile): improve native app feel on mobile browsers#121qard-spacebot wants to merge 8 commits intospacedriveapp:mainfrom
Conversation
0ec498e to
be79fcc
Compare
Add vitest test infrastructure and fix tab bar overflow on narrow viewports. - Install vitest, jsdom, @testing-library/react and @testing-library/jest-dom - Add vitest.config.ts with jsdom environment and @ path alias - Add src/test/setup.ts importing jest-dom matchers - Add AgentTabs.test.tsx asserting overflow-x-auto and no-scrollbar classes are present (tests fail before the fix) - Add overflow-x-auto and no-scrollbar to the AgentTabs container div so tabs are swipeable on mobile with no visible scrollbar chrome Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Without shrink-0 and an initial width, framer-motion did not set the
sidebar width inline style before first paint, causing the content area
to consume the full viewport width. Adding initial={{ width }} ensures
the correct width is applied synchronously, shrink-0 prevents flex from
compressing the sidebar, and min-w-0 on the content div allows it to
shrink below its content size.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
iOS Safari zooms the page when focusing any input with font-size < 16px. Applies font-size: 16px to all inputs and textareas at viewports ≤768px so desktop styles are entirely unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Tailwind utility classes (.text-sm etc.) have class-level specificity (0,1,0) which wins over bare element selectors (0,0,1). Adding !important ensures the 16px floor takes effect on mobile regardless of what font-size utility is applied to the input. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Responsive section navigation: desktop shows w-52 vertical sidebar, mobile shows horizontal overflow-x-auto tab bar with group dividers. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace inline sidebar with shared SettingSectionNav component. Outer wrapper gains flex-col md:flex-row for responsive stacking. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace inline two-group sidebar with shared SettingSectionNav component. Outer wrapper gains flex-col md:flex-row for responsive stacking. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
be79fcc to
bc4f42f
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a responsive SettingSectionNav component with tests, integrates it into Settings and AgentConfig routes, updates sidebar and agent tabs styling/animation, introduces Vitest config and test setup, adds tests for Sidebar and AgentTabs, and adds a responsive input font-size rule. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
interface/src/components/SettingSectionNav.tsx (1)
25-25: Use a more stable group key than label alone.Using
group.labelas the fragment key can collide if two groups share a label. Considerkey={${group.label}-${groupIndex}}or adding an explicit group id.Also applies to: 50-50
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interface/src/components/SettingSectionNav.tsx` at line 25, The React Fragment uses group.label as the key which can collide; update the key in the JSX inside SettingSectionNav (the <React.Fragment key={...}> instances) to use a stable unique identifier such as combining group.label with the index (e.g., `${group.label}-${groupIndex}`) or preferably an explicit group.id if available, and apply the same change to the other Fragment occurrence referenced in the diff to avoid duplicate keys.interface/src/components/SettingSectionNav.test.tsx (1)
34-102: Add an interaction test for section change callback.Current suite is strong on structure/styling, but it should also assert that clicking a section calls
onSectionChangewith the correct section id.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interface/src/components/SettingSectionNav.test.tsx` around lines 34 - 102, Add an interaction test in the SettingSectionNav suite that verifies the onSectionChange prop is invoked with the correct section id when a section button is clicked: render <SettingSectionNav groups={singleGroup} activeSection="a" onSectionChange={handler} /> with handler = vi.fn(), locate a specific button (e.g., the second section button via container.children[0].querySelectorAll("button")[1] or the mobile/button counterpart), simulate a click using fireEvent.click or userEvent.click, and assert handler was called once with the expected section id (e.g., "b") using expect(handler).toHaveBeenCalledWith("b").interface/src/routes/Settings.tsx (1)
555-563: Considermin-w-0on the content column for safer flex shrinking.With
md:flex-row, this helps prevent occasional horizontal overflow when descendants have large intrinsic widths.Suggested patch
- <div className="flex min-h-0 flex-1 flex-col overflow-hidden"> + <div className="flex min-h-0 min-w-0 flex-1 flex-col overflow-hidden">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interface/src/routes/Settings.tsx` around lines 555 - 563, The right-hand content column can overflow because its flex child needs a token to allow shrinking; add the CSS utility "min-w-0" to the content container div (the element that currently has className "flex min-h-0 flex-1 flex-col overflow-hidden") so the column can correctly shrink when using md:flex-row; update the JSX around SettingSectionNav / the content div (which interacts with SECTIONS, activeSection, and handleSectionChange) to include min-w-0 in that div's className.interface/src/routes/AgentConfig.tsx (1)
175-201: Addmin-w-0to the editor flex pane to harden desktop overflow behavior.After switching to
md:flex-row, the content pane should explicitly allow shrinking to avoid wide child content forcing overflow.Suggested patch
- <div className="flex flex-1 flex-col overflow-hidden"> + <div className="flex min-w-0 flex-1 flex-col overflow-hidden">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interface/src/routes/AgentConfig.tsx` around lines 175 - 201, The editor flex pane (the div rendering the editor: currently with className "flex flex-1 flex-col overflow-hidden") can’t shrink on md:flex-row and causes horizontal overflow; update that element to include the "min-w-0" utility so the pane is allowed to shrink (i.e., add min-w-0 to the className on the editor container where SettingSectionNav, activeSection and handleSectionChange are used).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@interface/src/components/AgentTabs.test.tsx`:
- Around line 13-17: The test in AgentTabs.test.tsx asserts the wrong number of
anchor links: the component AgentTabs now renders 11 links but the test uses
expect(links).toHaveLength(10); update the assertion to
expect(links).toHaveLength(11) (or compute the expected count from AgentTabs'
exported tabs constant if available) so the test matches the current rendering
of AgentTabs.
In `@interface/src/components/SettingSectionNav.tsx`:
- Around line 33-36: The SettingSidebarButton instances (used with props
onSectionChange and activeSection) are missing an explicit HTML button type and
may submit enclosing forms; update the component usage in SettingSectionNav so
each <SettingSidebarButton ...> includes type="button" (or ensure
SettingSidebarButton forwards a default type="button") to prevent accidental
form submissions when clicked.
---
Nitpick comments:
In `@interface/src/components/SettingSectionNav.test.tsx`:
- Around line 34-102: Add an interaction test in the SettingSectionNav suite
that verifies the onSectionChange prop is invoked with the correct section id
when a section button is clicked: render <SettingSectionNav groups={singleGroup}
activeSection="a" onSectionChange={handler} /> with handler = vi.fn(), locate a
specific button (e.g., the second section button via
container.children[0].querySelectorAll("button")[1] or the mobile/button
counterpart), simulate a click using fireEvent.click or userEvent.click, and
assert handler was called once with the expected section id (e.g., "b") using
expect(handler).toHaveBeenCalledWith("b").
In `@interface/src/components/SettingSectionNav.tsx`:
- Line 25: The React Fragment uses group.label as the key which can collide;
update the key in the JSX inside SettingSectionNav (the <React.Fragment
key={...}> instances) to use a stable unique identifier such as combining
group.label with the index (e.g., `${group.label}-${groupIndex}`) or preferably
an explicit group.id if available, and apply the same change to the other
Fragment occurrence referenced in the diff to avoid duplicate keys.
In `@interface/src/routes/AgentConfig.tsx`:
- Around line 175-201: The editor flex pane (the div rendering the editor:
currently with className "flex flex-1 flex-col overflow-hidden") can’t shrink on
md:flex-row and causes horizontal overflow; update that element to include the
"min-w-0" utility so the pane is allowed to shrink (i.e., add min-w-0 to the
className on the editor container where SettingSectionNav, activeSection and
handleSectionChange are used).
In `@interface/src/routes/Settings.tsx`:
- Around line 555-563: The right-hand content column can overflow because its
flex child needs a token to allow shrinking; add the CSS utility "min-w-0" to
the content container div (the element that currently has className "flex
min-h-0 flex-1 flex-col overflow-hidden") so the column can correctly shrink
when using md:flex-row; update the JSX around SettingSectionNav / the content
div (which interacts with SECTIONS, activeSection, and handleSectionChange) to
include min-w-0 in that div's className.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
interface/package-lock.jsonis excluded by!**/package-lock.json,!**/*.jsoninterface/package.jsonis excluded by!**/*.json
📒 Files selected for processing (12)
interface/src/components/AgentTabs.test.tsxinterface/src/components/AgentTabs.tsxinterface/src/components/SettingSectionNav.test.tsxinterface/src/components/SettingSectionNav.tsxinterface/src/components/Sidebar.test.tsxinterface/src/components/Sidebar.tsxinterface/src/router.tsxinterface/src/routes/AgentConfig.tsxinterface/src/routes/Settings.tsxinterface/src/test/setup.tsinterface/src/ui/style/style.scssinterface/vitest.config.ts
- Update tab-count assertion from 10 to 11 to match current AgentTabs - Add type="button" to SettingSidebarButton and button in SettingSectionNav to prevent accidental form submission Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Goal
Make the web UI feel like a native app when used on a mobile browser. Three issues prevented this: the tab bar overflowed off-screen with no way to scroll, the sidebar was not factored into the page layout so content spanned the full viewport width, and focusing any text input triggered iOS Safari's auto-zoom, breaking the fixed-layout feel.
Summary
overflow-x-auto no-scrollbarto theAgentTabscontainer so tabs are swipeable on narrow viewportsshrink-0andinitial={{ width }}to the sidebarmotion.navso framer-motion sets the correct pixel width on first paint, andmin-w-0to the content div — previously the content area consumed the full viewport width on all pagesfont-size: 16px \!importanttoinputandtextareaatmax-width: 768pxso mobile viewports are not zoomed when the chat input (or any other input) is focused; desktop styles are unchangedTest plan
cd interface && npm test— 9 tests pass🤖 Generated with Claude Code