fix: improve chat layout for landscape mobile viewports#503
fix: improve chat layout for landscape mobile viewports#503AnthonyRonning merged 1 commit intomasterfrom
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Deploying maple with
|
| Latest commit: |
63b7ed2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://83cf30b1.maple-ca8.pages.dev |
| Branch Preview URL: | https://devin-1777918021-landscape-m.maple-ca8.pages.dev |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
🐛 1 issue in files not directly in the diff
🐛 ProjectDetailView not updated with landscape mobile detection, causing sidebar to open as fixed overlay on landscape phones (frontend/src/components/ProjectDetailView.tsx:148-152)
The PR updates UnifiedChat.tsx, Sidebar.tsx, and _auth.chat.$chatId.tsx to use isCompactLayout = isMobile || isLandscapeMobile for landscape mobile handling, but ProjectDetailView.tsx was missed. It still only uses useIsMobile() at frontend/src/components/ProjectDetailView.tsx:148. On landscape phones (width > 768px, height < 500px), isMobile is false, so isSidebarOpen initializes to true at line 152. However, the Sidebar component was universally updated to use landscape-short:fixed (frontend/src/components/Sidebar.tsx:203), meaning the sidebar renders as a fixed-position overlay on landscape phones. This creates a broken initial state where the sidebar covers the content as a fixed overlay that the user didn't request. In contrast, UnifiedChat.tsx:1471 correctly starts the sidebar closed on landscape phones and has a dedicated effect (UnifiedChat.tsx:1514-1518) to close it when rotating to landscape.
View 6 additional findings in Devin Review.
|
Fixed |
Testing Results — Landscape Mobile LayoutTested locally (localhost:5173) with Chrome DevTools device toolbar across three viewport sizes. All tests passed. Landscape Mobile (850×400) — Primary Flow
Regression: Desktop (1280×800)
Regression: Portrait Mobile (375×812)
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Test Results — Portrait Mobile Header Fix (commit 9bcd610)Tested on localhost:5173 with Playwright CDP viewport resizing. Portrait Mobile (375×812) — PRIMARY TEST — PASSEDRow 1: Hamburger toggle + MAPLE wordmark + new-chat pencil icon Landscape Mobile (850×400) — PASSEDSingle row: hamburger + MAPLE wordmark + title + pencil icon all on one line. Sidebar closed, compact spacing. Desktop Regression (1280×800) — PASSEDSidebar open, centered title, standard desktop layout — unchanged from master. |
- Add useIsLandscapeMobile hook to detect landscape orientation on short screens (orientation: landscape, max-height: 500px) - Add landscape-short Tailwind screen for CSS-only responsive overrides - Default sidebar to closed in landscape mobile (was auto-opening since viewport width > 768px, consuming ~300px of limited screen space) - Use compact single-row header in landscape mobile to save vertical space - Reduce message padding, input area size, and spacing in landscape mode - Sidebar overlays content instead of pushing it in landscape mode - Normal mobile portrait and desktop layouts are unaffected Co-Authored-By: tony@opensecret.cloud <TonyGiorgio@protonmail.com> fix: use isCompactLayout for auto-focus and ChatHistoryList isMobile prop - Skip auto-focus on landscape mobile to prevent virtual keyboard popup - Pass isCompactLayout to ChatHistoryList so touch features (long-press, context menus) work correctly on landscape phones Co-Authored-By: tony@opensecret.cloud <TonyGiorgio@protonmail.com> fix: use isCompactLayout in handleKeyDown to prevent Enter-submit on landscape phones Co-Authored-By: tony@opensecret.cloud <TonyGiorgio@protonmail.com> fix: use isCompactLayout for New Chat button in archived chat view Co-Authored-By: tony@opensecret.cloud <TonyGiorgio@protonmail.com> fix: add landscape mobile handling to ProjectDetailView - Use isCompactLayout for sidebar default state - Add effect to close sidebar when rotating to landscape Co-Authored-By: tony@opensecret.cloud <TonyGiorgio@protonmail.com> fix: restore two-row header for portrait mobile, keep single-row for landscape - Portrait mobile: two-row header (toggle + wordmark on first row, title on second) - Landscape mobile: single-row header with wordmark + title (has enough room) - Desktop: unchanged single-row centered header Co-Authored-By: tony@opensecret.cloud <TonyGiorgio@protonmail.com> fix: check landscape before portrait in header, add sidebar auto-close to archived view - Reorder header conditional to check isLandscapeMobile first so small phones in landscape get the single-row compact header - Add missing useEffect to auto-close sidebar on rotation in archived chat view, matching UnifiedChat and ProjectDetailView Co-Authored-By: tony@opensecret.cloud <TonyGiorgio@protonmail.com>
AnthonyRonning
force-pushed
the
devin/1777918021-landscape-mobile-layout
branch
from
9bcd610 to
63b7ed2
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughA new landscape-mobile detection hook and Tailwind breakpoint enable responsive layout adjustments across the chat UI. Components now combine mobile and landscape-mobile states into a unified compact-layout flag, automatically closing sidebars during orientation changes and applying spacing/height adjustments via new ChangesLandscape-Mobile Responsive Layout
Sequence DiagramsequenceDiagram
participant Device as Mobile Device
participant Hook as useIsLandscapeMobile Hook
participant Component as Component Logic
participant UI as UI Render Layer
Device->>Hook: Screen rotates to landscape<br/>(orientation: landscape,<br/>max-height: 500px)
Hook->>Hook: MediaQueryList "change" event fires
Hook->>Component: Hook state updates to true
Component->>Component: isCompactLayout = true<br/>(isMobile || isLandscapeMobile)
Component->>Component: Effect detects landscape-mobile<br/>with open sidebar
Component->>Component: Sidebar auto-closes
Component->>UI: Re-render with updated state
UI->>UI: Apply landscape-short classes<br/>(padding, spacing, height)
UI->>Device: Display adjusted layout
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/ProjectDetailView.tsx`:
- Around line 156-160: The effect currently only watches isLandscapeMobile;
change it to key off isCompactLayout so the sidebar auto-closes for any compact
layout. In the useEffect (function surrounding
isLandscapeMobile/isSidebarOpen/setIsSidebarOpen) replace the condition to check
isCompactLayout (e.g., if (isCompactLayout && isSidebarOpen)
setIsSidebarOpen(false)) and update the dependency array to [isCompactLayout]
(and include isSidebarOpen if you prefer explicitness) instead of
[isLandscapeMobile]; keep useEffect and setIsSidebarOpen as the referenced
symbols.
In `@frontend/src/components/UnifiedChat.tsx`:
- Around line 1514-1518: The useEffect that auto-closes the sidebar only checks
isLandscapeMobile; update it to also consider the compact layout flag so
portrait-mobile transitions close the sidebar too: change the conditional inside
the useEffect to if ((isLandscapeMobile || isCompactLayout) && isSidebarOpen)
setIsSidebarOpen(false), and update the dependency array to [isLandscapeMobile,
isCompactLayout, isSidebarOpen] (remove the eslint-disable-line) so the effect
runs when either layout flag or sidebar state changes; modify the effect
surrounding function name useEffect and the referenced symbols
isLandscapeMobile, isCompactLayout, isSidebarOpen, and setIsSidebarOpen
accordingly.
In `@frontend/src/routes/_auth.chat`.$chatId.tsx:
- Around line 173-177: The sidebar auto-close effect currently only watches
isLandscapeMobile; update the useEffect that contains isLandscapeMobile to also
consider isCompactLayout and change the condition to close the sidebar when the
layout becomes compact (e.g., if (isCompactLayout && isSidebarOpen)
setIsSidebarOpen(false)). Modify the dependency array to include isCompactLayout
(and keep isLandscapeMobile if still needed) so transitions like
desktop→portrait-mobile trigger the overlay-close behavior; reference the
useEffect, isLandscapeMobile, isCompactLayout, isSidebarOpen, and
setIsSidebarOpen identifiers when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8ad9d7e4-d062-445c-9734-a65257cf3e46
📒 Files selected for processing (6)
frontend/src/components/ProjectDetailView.tsxfrontend/src/components/Sidebar.tsxfrontend/src/components/UnifiedChat.tsxfrontend/src/routes/_auth.chat.$chatId.tsxfrontend/src/utils/utils.tsfrontend/tailwind.config.js
| useEffect(() => { | ||
| if (isLandscapeMobile && isSidebarOpen) { | ||
| setIsSidebarOpen(false); | ||
| } | ||
| }, [isLandscapeMobile]); // eslint-disable-line react-hooks/exhaustive-deps |
There was a problem hiding this comment.
Sidebar auto-close should key off compact layout, not landscape-only.
On Line 157, the effect only reacts to isLandscapeMobile. If the viewport transitions from desktop to portrait mobile, isCompactLayout becomes true but the sidebar can remain open.
Proposed fix
- useEffect(() => {
- if (isLandscapeMobile && isSidebarOpen) {
- setIsSidebarOpen(false);
- }
- }, [isLandscapeMobile]); // eslint-disable-line react-hooks/exhaustive-deps
+ useEffect(() => {
+ if (isCompactLayout && isSidebarOpen) {
+ setIsSidebarOpen(false);
+ }
+ }, [isCompactLayout, isSidebarOpen]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| if (isLandscapeMobile && isSidebarOpen) { | |
| setIsSidebarOpen(false); | |
| } | |
| }, [isLandscapeMobile]); // eslint-disable-line react-hooks/exhaustive-deps | |
| useEffect(() => { | |
| if (isCompactLayout && isSidebarOpen) { | |
| setIsSidebarOpen(false); | |
| } | |
| }, [isCompactLayout, isSidebarOpen]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/ProjectDetailView.tsx` around lines 156 - 160, The
effect currently only watches isLandscapeMobile; change it to key off
isCompactLayout so the sidebar auto-closes for any compact layout. In the
useEffect (function surrounding
isLandscapeMobile/isSidebarOpen/setIsSidebarOpen) replace the condition to check
isCompactLayout (e.g., if (isCompactLayout && isSidebarOpen)
setIsSidebarOpen(false)) and update the dependency array to [isCompactLayout]
(and include isSidebarOpen if you prefer explicitness) instead of
[isLandscapeMobile]; keep useEffect and setIsSidebarOpen as the referenced
symbols.
| useEffect(() => { | ||
| if (isLandscapeMobile && isSidebarOpen) { | ||
| setIsSidebarOpen(false); | ||
| } | ||
| }, [isLandscapeMobile]); // eslint-disable-line react-hooks/exhaustive-deps |
There was a problem hiding this comment.
Compact-mode sidebar behavior should include portrait mobile transitions.
On Line 1515, auto-close is tied only to isLandscapeMobile. The compact rules now include portrait mobile too (isCompactLayout), so desktop-to-portrait resize can leave the sidebar open unexpectedly.
Proposed fix
- useEffect(() => {
- if (isLandscapeMobile && isSidebarOpen) {
- setIsSidebarOpen(false);
- }
- }, [isLandscapeMobile]); // eslint-disable-line react-hooks/exhaustive-deps
+ useEffect(() => {
+ if (isCompactLayout && isSidebarOpen) {
+ setIsSidebarOpen(false);
+ }
+ }, [isCompactLayout, isSidebarOpen]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/UnifiedChat.tsx` around lines 1514 - 1518, The
useEffect that auto-closes the sidebar only checks isLandscapeMobile; update it
to also consider the compact layout flag so portrait-mobile transitions close
the sidebar too: change the conditional inside the useEffect to if
((isLandscapeMobile || isCompactLayout) && isSidebarOpen)
setIsSidebarOpen(false), and update the dependency array to [isLandscapeMobile,
isCompactLayout, isSidebarOpen] (remove the eslint-disable-line) so the effect
runs when either layout flag or sidebar state changes; modify the effect
surrounding function name useEffect and the referenced symbols
isLandscapeMobile, isCompactLayout, isSidebarOpen, and setIsSidebarOpen
accordingly.
| useEffect(() => { | ||
| if (isLandscapeMobile && isSidebarOpen) { | ||
| setIsSidebarOpen(false); | ||
| } | ||
| }, [isLandscapeMobile]); // eslint-disable-line react-hooks/exhaustive-deps |
There was a problem hiding this comment.
Use isCompactLayout for sidebar auto-close on viewport transitions.
On Line 174, the close logic only watches isLandscapeMobile. That misses desktop-to-portrait-mobile transitions where compact behavior should also close/overlay the sidebar.
Proposed fix
- useEffect(() => {
- if (isLandscapeMobile && isSidebarOpen) {
- setIsSidebarOpen(false);
- }
- }, [isLandscapeMobile]); // eslint-disable-line react-hooks/exhaustive-deps
+ useEffect(() => {
+ if (isCompactLayout && isSidebarOpen) {
+ setIsSidebarOpen(false);
+ }
+ }, [isCompactLayout, isSidebarOpen]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| if (isLandscapeMobile && isSidebarOpen) { | |
| setIsSidebarOpen(false); | |
| } | |
| }, [isLandscapeMobile]); // eslint-disable-line react-hooks/exhaustive-deps | |
| useEffect(() => { | |
| if (isCompactLayout && isSidebarOpen) { | |
| setIsSidebarOpen(false); | |
| } | |
| }, [isCompactLayout, isSidebarOpen]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/routes/_auth.chat`.$chatId.tsx around lines 173 - 177, The
sidebar auto-close effect currently only watches isLandscapeMobile; update the
useEffect that contains isLandscapeMobile to also consider isCompactLayout and
change the condition to close the sidebar when the layout becomes compact (e.g.,
if (isCompactLayout && isSidebarOpen) setIsSidebarOpen(false)). Modify the
dependency array to include isCompactLayout (and keep isLandscapeMobile if still
needed) so transitions like desktop→portrait-mobile trigger the overlay-close
behavior; reference the useEffect, isLandscapeMobile, isCompactLayout,
isSidebarOpen, and setIsSidebarOpen identifiers when making the change.
1 participant
Summary
Fixes cramped landscape mobile layout by detecting landscape orientation on short screens and applying a compact layout. Portrait mobile keeps the original two-row header (wordmark + title on separate lines). Desktop is unchanged.
Detection: New
useIsLandscapeMobile()hook using(orientation: landscape) and (max-height: 500px)— catches phones but not tablets/laptops.Layout behavior:
Compact layout shared behavior (landscape + portrait mobile): sidebar defaults closed/overlays as drawer, Enter inserts newline, virtual keyboard auto-focus suppressed, touch features in sidebar.
Landscape-only spacing (via
landscape-short:Tailwind classes): compact message padding, smaller empty-state title, reduced textarea heights, tighter vertical spacing.Review & Testing Checklist for Human
Recommended test plan: Open the preview deploy on a real phone, test in both portrait and landscape orientations. Alternatively, use Chrome DevTools device toolbar (Ctrl+Shift+M) at the viewport sizes listed above.
Notes
just buildfails locally on master due to a pre-existing ESM/CJS mismatch intailwind.config.js. CI/Cloudflare Pages builds pass fine.isLandscapeMobilebeforeisMobileto ensure small phones in landscape get the compact single-row header (more specific condition first).Link to Devin session: https://app.devin.ai/sessions/5f78b75e83a94439904d641d034eda8c
Requested by: @AnthonyRonning
Summary by CodeRabbit
Improvements
Style