feat: add tablet layout with icons#221
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a tablet-specific breakpoint (769–1024px): CSS implements a two-column tablet layout; chat component gains isTablet detection, a tablet render path that composes map/settings + icons bar + chat, and integrates MapDataProvider/useMapData usage for that path. Changes
Sequence Diagram(s)sequenceDiagram
participant Window
participant Chat
participant Layout
Window->>Chat: load / resize event
Chat->>Chat: checkDevice(width)\nset isMobile/isTablet/desktop
alt isTablet
Chat->>Layout: render tablet layout (map + icons bar + chat)
else isMobile
Chat->>Layout: render mobile layout
else
Chat->>Layout: render desktop layout
end
sequenceDiagram
participant Chat
participant MapDataProvider
participant Server as updateDrawingContext
Chat->>MapDataProvider: useMapData()
MapDataProvider-->>Chat: drawnFeatures
Chat->>Chat: effect on drawnFeatures change
Chat->>Server: updateDrawingContext(chatId, drawnFeatures)
Server-->>Chat: response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these settings in your CodeRabbit configuration. 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||||
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
components/chat.tsx (1)
63-69: Do not import server actions into a client component — stop calling updateDrawingContext directly from components/chat.tsxVerified: lib/actions/chat.ts exports updateDrawingContext and contains 'use server' (server action), so importing/calling it inside a 'use client' component will fail. The current effect also doesn't await/catch the Promise and skips clearing when features become empty.
Files to fix:
- components/chat.tsx — current useEffect (lines ~63–69) calls updateDrawingContext from the client.
- lib/actions/chat.ts — export async function updateDrawingContext(...) with 'use server' (lines ~165–169).
Suggested change (call a server API or move invocation to a Server Component; ensure await, error handling, debounce, and clear-on-empty):
- useEffect(() => { - if (id && mapData.drawnFeatures && mapData.drawnFeatures.length > 0) { - console.log('Chat.tsx: drawnFeatures changed, calling updateDrawingContext', mapData.drawnFeatures); - updateDrawingContext(id, mapData.drawnFeatures); - } - }, [id, mapData.drawnFeatures]); + useEffect(() => { + if (!id || !mapData) return; + const features = mapData.drawnFeatures ?? []; + + const handle = setTimeout(async () => { + try { + await fetch('/api/chat/drawing', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ chatId: id, drawnFeatures: features }), + }); + } catch (err) { + console.error('Failed to update drawing context', err); + } + }, 250); + + return () => clearTimeout(handle); + }, [id, mapData?.drawnFeatures]);Action items:
- Implement a POST API route (e.g., /api/chat/drawing) that calls lib/actions/chat.updateDrawingContext server action, or
- Move the update call into a Server Component and pass a server action/handler into the client.
- Ensure empty feature arrays are sent to clear server state, debounce rapid updates, await the call, and handle errors.
🧹 Nitpick comments (4)
app/globals.css (2)
229-276: Use dynamic viewport units to avoid 100vh issues on mobile/tablet browsers.100vh can cause content to be clipped under the browser chrome (especially iOS Safari). Prefer 100dvh where supported to ensure full-viewport height.
- .tablet-layout-container { - display: flex; - height: 100vh; + .tablet-layout-container { + display: flex; + height: 100dvh; /* fallback in a cascaded rule below if needed */ width: 100%; background-color: hsl(var(--background)); } - .tablet-map-and-icons-section { - width: 50%; - display: flex; - flex-direction: column; - height: 100vh; + .tablet-map-and-icons-section { + width: 50%; + display: flex; + flex-direction: column; + height: 100dvh; } - .tablet-chat-section { + .tablet-chat-section { width: 50%; display: flex; flex-direction: column; - height: 100vh; + height: 100dvh; border-left: 1px solid hsl(var(--border)); padding: 12px; overflow-y: auto; }If you need broader fallback support, keep a preceding 100vh rule and override with 100dvh later.
229-276: Align breakpoints across CSS and TS logic (tablet: 769–1024; mobile: ≤768).The tablet media query here is 769–1024, which matches the TS breakpoint logic. However, the mobile CSS block above uses max-width: 1024px (not part of this change), while the TS logic treats mobile as ≤768. This drift can confuse future maintenance. Consider updating the mobile CSS breakpoint to max-width: 768px to match the TS logic, or derive classes via Tailwind’s breakpoints to centralize breakpoints.
components/chat.tsx (2)
36-45: Prefer matchMedia over manual innerWidth checks and consider throttling resize.matchMedia keeps JS detection aligned with CSS queries and avoids tight resize loops. Also, resize can fire frequently—consider throttling to reduce re-renders.
- useEffect(() => { - const checkDevice = () => { - const width = window.innerWidth; - setIsMobile(width <= 768); - setIsTablet(width > 768 && width <= 1024); - }; - checkDevice(); - window.addEventListener('resize', checkDevice); - return () => window.removeEventListener('resize', checkDevice); - }, []); + useEffect(() => { + const mqMobile = window.matchMedia('(max-width: 768px)'); + const mqTablet = window.matchMedia('(min-width: 769px) and (max-width: 1024px)'); + + const update = () => { + setIsMobile(mqMobile.matches); + setIsTablet(mqTablet.matches); + }; + update(); + + // Prefer `change` events on MediaQueryList when available + mqMobile.addEventListener?.('change', update); + mqTablet.addEventListener?.('change', update); + + // Fallback to resize for older browsers + const onResize = () => update(); + window.addEventListener('resize', onResize, { passive: true }); + + return () => { + mqMobile.removeEventListener?.('change', update); + mqTablet.removeEventListener?.('change', update); + window.removeEventListener('resize', onResize); + }; + }, []);
101-121: Minor: consider ChatPanel placement and scroll dynamics on tablet.You render ChatMessages above ChatPanel for tablet, while on mobile ChatPanel is a fixed input area above messages. Ensure the tablet chat section has sufficient flex/overflow to keep the input visible and maintain expected UX parity, especially with the 60px icons bar.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (2)
app/globals.css(1 hunks)components/chat.tsx(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
components/chat.tsx (5)
components/map/map-data-context.tsx (2)
MapDataProvider(26-34)MapData(7-17)components/settings/settings-view.tsx (1)
SettingsView(8-35)components/mobile-icons-bar.tsx (1)
MobileIconsBar(20-55)components/chat-panel.tsx (1)
ChatPanel(21-187)app/page.tsx (1)
Page(9-18)
🔇 Additional comments (1)
app/globals.css (1)
229-276: Tablet layout structure is coherent and maps cleanly to the JSX.The 2-column split with a dedicated map + icons stack on the left and chat on the right is easy to reason about. Class naming is consistent with the JSX usage in chat.tsx.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
components/chat.tsx (1)
61-69: Do not import and call a Server Action from a Client Component; pass it as a prop (and dedupe calls).Calling updateDrawingContext directly from a 'use client' component is not supported and will fail the build in Next.js. Also, the effect will spam the server on every intermediate draw event and logs feature data to the console.
Action items:
- Pass the server action down from a Server Component (e.g., app/page.tsx) as a prop to Chat and invoke it here.
- Remove the console.log.
- Dedupe the payload to avoid duplicate writes (optional but recommended).
- Make the effect null-safe with optional chaining and include proper dependencies.
Apply this diff here:
- const { mapData } = useMapData(); + const { mapData } = useMapData(); + const drawnFeatures = mapData?.drawnFeatures; // useEffect to call the server action when drawnFeatures changes useEffect(() => { - if (id && mapData.drawnFeatures && mapData.drawnFeatures.length > 0) { - console.log('Chat.tsx: drawnFeatures changed, calling updateDrawingContext', mapData.drawnFeatures); - updateDrawingContext(id, mapData.drawnFeatures); - } - }, [id, mapData.drawnFeatures]); + if (!id || !drawnFeatures?.length) return; + // Call a server action passed from a Server Component (see page.tsx) to avoid importing server-only code in a Client Component. + void onUpdateDrawingContext?.(id, drawnFeatures); + }, [id, drawnFeatures, onUpdateDrawingContext]);Changes outside this hunk (to wire the action properly):
Update the props type and usage in this file:
- Add to ChatProps: onUpdateDrawingContext?: (id: string, drawn: any[]) => Promise
- Use it as shown in the effect.
In app/page.tsx (Server Component), pass the action:
// app/page.tsx (excerpt) import { updateDrawingContext } from '@/lib/actions/chat' export default async function Page() { const id = nanoid() return ( <MapDataProvider> <Chat id={id} onUpdateDrawingContext={updateDrawingContext} /> </MapDataProvider> ) }Optional hardening:
- Debounce and/or dedupe the payload to reduce DB churn (e.g., track last payload with a ref and skip if unchanged).
- Add try/catch around the awaited call for error logging/telemetry.
♻️ Duplicate comments (3)
components/chat.tsx (3)
74-98: Remove nested MapDataProvider — it breaks context synchronization (duplicate of prior review).Chat uses useMapData above, relying on the page-level provider. Wrapping the mobile subtree in a new MapDataProvider splits the context, so Mapbox/MobileIconsBar write to a different MapData than the effect observes. Tablet path is already fixed; apply the same fix here.
Apply this diff:
- <MapDataProvider> {/* Add Provider */} <div className="mobile-layout-container"> <div className="mobile-map-section"> {activeView ? <SettingsView /> : <Mapbox />} </div> <div className="mobile-icons-bar"> <MobileIconsBar /> </div> <div className="mobile-chat-input-area"> <ChatPanel messages={messages} input={input} setInput={setInput} /> </div> <div className="mobile-chat-messages-area"> {showEmptyScreen ? ( <EmptyScreen submitMessage={message => { setInput(message) }} /> ) : ( <ChatMessages messages={messages} /> )} </div> </div> - </MapDataProvider>
123-139: Remove nested MapDataProvider in desktop branch — keep a single authoritative provider (duplicate of prior review).Same issue as the mobile branch: nested providers desynchronize context from the effect above.
Apply this diff:
- return ( - <MapDataProvider> {/* Add Provider */} + return ( <div className="flex justify-start items-start"> {/* This is the new div for scrolling */} <div className="w-1/2 flex flex-col space-y-3 md:space-y-4 px-8 sm:px-12 pt-12 md:pt-14 pb-4 h-[calc(100vh-0.5in)] overflow-y-auto"> {/* TODO: Add EmptyScreen for desktop if needed */} <ChatMessages messages={messages} /> <ChatPanel messages={messages} input={input} setInput={setInput} /> </div> <div className="w-1/2 p-4 fixed h:[calc(100vh-0.5in)] top-0 right-0 mt-[0.5in]" style={{ zIndex: 10 }} // Added z-index > {activeView ? <SettingsView /> : <Mapbox />} </div> </div> - </MapDataProvider> - ); + );
36-45: Standardize breakpoints via a shared hook/constants; avoid drift with other components (duplicate of prior review).This file uses ≤768 for mobile and 769–1024 for tablet, while ChatPanel uses ≤1024 as “mobile”. This will cause tablet to render mobile behavior inside ChatPanel. Extract a shared useDevice or breakpoint constants and use them across components.
Minimal drop-in improvement here (optional): use matchMedia listeners instead of resize to avoid layout thrash and simplify logic.
- const checkDevice = () => { - const width = window.innerWidth; - setIsMobile(width <= 768); - setIsTablet(width > 768 && width <= 1024); - }; - - checkDevice(); - window.addEventListener('resize', checkDevice); - return () => window.removeEventListener('resize', checkDevice); + const MOBILE_MAX = 768; + const TABLET_MAX = 1024; + const mMobile = window.matchMedia(`(max-width: ${MOBILE_MAX}px)`); + const mTablet = window.matchMedia(`(min-width: ${MOBILE_MAX + 1}px) and (max-width: ${TABLET_MAX}px)`); + const apply = () => { + setIsMobile(mMobile.matches); + setIsTablet(mTablet.matches); + }; + apply(); + mMobile.addEventListener('change', apply); + mTablet.addEventListener('change', apply); + return () => { + mMobile.removeEventListener('change', apply); + mTablet.removeEventListener('change', apply); + };Repository-wide follow-up:
- Create src/lib/breakpoints.ts (MOBILE_MAX = 768, TABLET_MAX = 1024) or hooks/useDevice.ts returning { isMobile, isTablet, isDesktop }.
- Replace ad-hoc innerWidth checks with the shared source of truth (ChatPanel, mapbox-map, profile-toggle, etc.).
You can verify remaining hard-coded checks with:
#!/bin/bash # Find width-based checks to unify breakpoints throughout the repo. rg -n -A2 -i 'matchMedia|innerWidth|isMobile|mobile' components | sed -n '1,200p'
🧹 Nitpick comments (2)
components/chat.tsx (2)
25-26: Avoid initial layout flash on first render.Initializing isMobile/isTablet to false renders the desktop branch before the effect runs, causing a visible flicker on mobile/tablet. Initialize to null and gate render until computed.
- const [isMobile, setIsMobile] = useState(false); - const [isTablet, setIsTablet] = useState(false); + const [isMobile, setIsMobile] = useState<boolean | null>(null); + const [isTablet, setIsTablet] = useState<boolean | null>(null);Then early-return until both are resolved (place this before the conditional branches):
if (isMobile === null || isTablet === null) return null;
101-119: Tablet path looks good; ensure ChatPanel behavior isn’t “mobile” on tablets.Given ChatPanel currently treats ≤1024 as mobile, it may render the mobile UI in this tablet layout. After standardizing breakpoints, confirm ChatPanel renders the intended tablet-desktop behavior here. Also consider renaming MobileIconsBar to a neutral name if it’s shared on tablet.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
components/chat.tsx(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
components/chat.tsx (8)
components/settings/settings-view.tsx (1)
SettingsView(8-35)components/map/mapbox-map.tsx (1)
Mapbox(17-614)components/mobile-icons-bar.tsx (1)
MobileIconsBar(20-55)components/chat-messages.tsx (1)
ChatMessages(11-70)lib/db/schema.ts (1)
messages(26-37)components/chat-panel.tsx (1)
ChatPanel(21-187)app/page.tsx (1)
Page(9-18)lib/actions/chat.ts (1)
updateDrawingContext(165-205)
The icons in the tablet layout were overflowing and not all visible. This change adjusts the CSS for the tablet icon bar to ensure all icons are visible and correctly aligned.
2 participants
PR Type
Enhancement
Description
Add responsive tablet layout with 50/50 split design
Implement device detection for mobile, tablet, and desktop
Create dedicated tablet icons bar with horizontal scrolling
Refactor layout logic for better responsive behavior
Diagram Walkthrough
File Walkthrough
globals.css
Add tablet-specific CSS layout stylesapp/globals.css
chat.tsx
Implement tablet layout and device detectioncomponents/chat.tsx
isTabletstate for tablet device detectionSummary by CodeRabbit
New Features
Style
Improved Responsive Behavior