feat(web): responsive mobile UI with hamburger menu#1180
Closed
tboome33 wants to merge 24 commits intocoleam00:devfrom
Closed
feat(web): responsive mobile UI with hamburger menu#1180tboome33 wants to merge 24 commits intocoleam00:devfrom
tboome33 wants to merge 24 commits intocoleam00:devfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughLayout adds a local mobile-nav state and provides it via a new MobileNavContext; TopNav can open the mobile drawer. ChatPage gains a mobile conversations drawer and responsive behaviors. API types/functions, global CSS, and MessageInput layout were also updated. Changes
Sequence DiagramsequenceDiagram
actor User
participant TopNav
participant MobileNavContext
participant Layout
participant MobileDrawer
User->>TopNav: Click hamburger
TopNav->>MobileNavContext: setOpen(true)
MobileNavContext->>Layout: context open = true
Layout->>MobileDrawer: render backdrop + drawer (visible)
MobileDrawer-->>User: drawer slides in
User->>MobileDrawer: Click backdrop or nav item or X
MobileDrawer->>MobileNavContext: setOpen(false)
MobileNavContext->>Layout: context open = false
Layout->>MobileDrawer: hide backdrop + drawer
MobileDrawer-->>User: drawer slides out
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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: 2
🧹 Nitpick comments (2)
packages/web/src/components/layout/Layout.tsx (2)
31-39: Consider adding focus trap for accessibility.The drawer implementation looks good visually, but for full keyboard accessibility, consider adding a focus trap so that keyboard focus stays within the drawer while it's open. Libraries like
@radix-ui/react-dialogorfocus-trap-reactcan help, or you could use the native<dialog>element.This is optional but would improve the experience for keyboard and screen reader users.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/components/layout/Layout.tsx` around lines 31 - 39, The mobile drawer in the Layout component (the aside rendered when open is controlled by the open prop) needs a focus trap to keep keyboard focus inside while open; integrate a library like focus-trap-react or replace the aside with a Radix Dialog to wrap the drawer contents, initialize the trap when open is true and deactivate on close, ensure aria-modal/role="dialog" and initial focus are set and that focus is returned to the triggering element on close (reference the Layout component, the aside element using cn(...) and the open prop to locate where to add the trap).
8-12: Consider extracting shared navigation items to reduce duplication.The
navItemsarray here duplicates thetabsarray inTopNav.tsx(lines 8-13). While not strictly necessary, extracting these to a shared constant would ensure the navigation structure stays in sync between desktop tabs and mobile drawer.💡 Example shared navigation config
Create a shared config file (e.g.,
@/lib/navigation.ts):import { MessageSquare, LayoutDashboard, Workflow, Settings } from 'lucide-react'; import type { LucideIcon } from 'lucide-react'; export interface NavItem { to: string; end: boolean; icon: LucideIcon; label: string; } export const mainNavItems: readonly NavItem[] = [ { to: '/chat', end: false, icon: MessageSquare, label: 'Chat' }, { to: '/dashboard', end: true, icon: LayoutDashboard, label: 'Dashboard' }, { to: '/workflows', end: false, icon: Workflow, label: 'Workflows' }, ] as const; export const settingsNavItem: NavItem = { to: '/settings', end: false, icon: Settings, label: 'Settings' };Then import in both
Layout.tsxandTopNav.tsx.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/components/layout/Layout.tsx` around lines 8 - 12, navItems in Layout.tsx duplicates the tabs list in TopNav.tsx; extract the shared navigation structure into a single exported constant (e.g., a readonly array of NavItem objects) and import it in both Layout.tsx and TopNav.tsx so both use the same source of truth. Create a small NavItem shape (to, end, icon, label), move the existing entries (the items currently in navItems and tabs) into that shared export, update Layout.tsx to use navItems from the shared module and update TopNav.tsx to import the same array (and keep any component-specific filtering or additions local). Ensure you keep the original symbol names used in each file (navItems in Layout.tsx and tabs in TopNav.tsx) by assigning them to the imported shared constant.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/web/src/components/layout/Layout.tsx`:
- Around line 49-55: The aria-label on the close button in Layout.tsx is in
French ("Fermer le menu"); update the button element that calls setOpen(false)
and renders <X .../> to use an English aria-label such as "Close menu" (or the
project's standard English wording) to keep accessibility text consistent with
the UI language while leaving the onClick, className, and icon unchanged.
In `@packages/web/src/components/layout/TopNav.tsx`:
- Around line 36-43: The mobile hamburger button in TopNav uses a French
aria-label ("Ouvrir le menu") which is inconsistent with the app's English UI;
update the button's aria-label (the element with onClick={() => { setOpen(true);
}} and the <Menu /> icon) to an English string such as "Open menu" to restore
accessibility and language consistency.
---
Nitpick comments:
In `@packages/web/src/components/layout/Layout.tsx`:
- Around line 31-39: The mobile drawer in the Layout component (the aside
rendered when open is controlled by the open prop) needs a focus trap to keep
keyboard focus inside while open; integrate a library like focus-trap-react or
replace the aside with a Radix Dialog to wrap the drawer contents, initialize
the trap when open is true and deactivate on close, ensure
aria-modal/role="dialog" and initial focus are set and that focus is returned to
the triggering element on close (reference the Layout component, the aside
element using cn(...) and the open prop to locate where to add the trap).
- Around line 8-12: navItems in Layout.tsx duplicates the tabs list in
TopNav.tsx; extract the shared navigation structure into a single exported
constant (e.g., a readonly array of NavItem objects) and import it in both
Layout.tsx and TopNav.tsx so both use the same source of truth. Create a small
NavItem shape (to, end, icon, label), move the existing entries (the items
currently in navItems and tabs) into that shared export, update Layout.tsx to
use navItems from the shared module and update TopNav.tsx to import the same
array (and keep any component-specific filtering or additions local). Ensure you
keep the original symbol names used in each file (navItems in Layout.tsx and
tabs in TopNav.tsx) by assigning them to the imported shared constant.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 735569db-6d84-46ca-97d2-f76f682e2acb
📒 Files selected for processing (4)
packages/web/src/components/layout/Layout.tsxpackages/web/src/components/layout/TopNav.tsxpackages/web/src/contexts/MobileNavContext.tsxpackages/web/src/routes/ChatPage.tsx
Replace window.location.hostname direct-connect with localhost-only check. When accessed via Cloudflare tunnel, use relative URL so requests go through Vite proxy which forwards to the backend (port 3090 not directly accessible).
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/web/src/lib/api.ts`:
- Around line 21-23: Replace the top-level SSE_BASE_URL constant with a lazy
function getSSEBaseUrl() that guards access to window using typeof window !==
'undefined'; inside the function check import.meta.env.DEV and only then read
window.location.hostname === 'localhost' to return `http://localhost:${apiPort}`
or '' as fallback, so module evaluation during SSR/tests won't touch window;
update any callers to invoke getSSEBaseUrl() instead of referencing
SSE_BASE_URL.
- Line 44: Client-server contract mismatch: the client uses allowEnvKeys /
allow_env_keys but the server schemas and handlers (addCodebaseBodySchema,
codebaseSchema, and no PATCH handler for /api/codebases/:id) don’t support it.
Fix by either removing allowEnvKeys from the client API or adding full server
support: update addCodebaseBodySchema to accept allow_env_keys, extend
codebaseSchema to include allow_env_keys in responses, implement a PATCH handler
for /api/codebases/:id that validates and persists allow_env_keys to the DB (and
add a DB column/migration if needed), and ensure the client and server use the
same snake_case/key names in request/response contracts.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8ac64a18-11a6-4f41-be55-9e4811d3d4b0
📒 Files selected for processing (1)
packages/web/src/lib/api.ts
Comment on lines
21
to
23
| export const SSE_BASE_URL = import.meta.env.DEV && window.location.hostname === 'localhost' | ||
| ? `http://localhost:${apiPort}` | ||
| : ''; |
There was a problem hiding this comment.
Guard module-load window access for SSR/tests.
Line 21 can throw when this module is evaluated without window (SSR/tests). Use a lazy helper with a typeof window guard.
Suggested fix
const apiPort = (import.meta.env.VITE_API_PORT as string | undefined) ?? '3090';
-export const SSE_BASE_URL = import.meta.env.DEV && window.location.hostname === 'localhost'
- ? `http://localhost:${apiPort}`
- : '';
+function getSSEBaseUrl(): string {
+ const hostname = typeof window !== 'undefined' ? window.location.hostname : 'localhost';
+ return import.meta.env.DEV && hostname === 'localhost' ? `http://localhost:${apiPort}` : '';
+}
+
+export const SSE_BASE_URL = getSSEBaseUrl();Based on learnings: implement SSE URL via a lazy getSSEBaseUrl() with typeof window !== 'undefined' and 'localhost' fallback to avoid SSR/test crashes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/lib/api.ts` around lines 21 - 23, Replace the top-level
SSE_BASE_URL constant with a lazy function getSSEBaseUrl() that guards access to
window using typeof window !== 'undefined'; inside the function check
import.meta.env.DEV and only then read window.location.hostname === 'localhost'
to return `http://localhost:${apiPort}` or '' as fallback, so module evaluation
during SSR/tests won't touch window; update any callers to invoke
getSSEBaseUrl() instead of referencing SSE_BASE_URL.
| repository_url: string | null; | ||
| default_cwd: string; | ||
| ai_assistant_type: string; | ||
| allow_env_keys: boolean; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify backend contract support for allowEnvKeys / allow_env_keys
set -euo pipefail
echo "== Server schemas =="
rg -n --type=ts -C3 'codebaseSchema|addCodebaseBodySchema|allowEnvKeys|allow_env_keys' packages/server/src/routes/schemas
echo "== Server API routes =="
rg -n --type=ts -C3 '/api/codebases|registerOpenApiRoute|allowEnvKeys|allow_env_keys' packages/server/src/routes/api.ts
echo "== Generated web API types =="
rg -n --type=ts -C2 'allow_env_keys|Codebase' packages/web/src/lib/api.generated.tsRepository: coleam00/Archon
Length of output: 15737
🏁 Script executed:
# Check the web client file at the mentioned lines
cat -n packages/web/src/lib/api.ts | sed -n '40,50p;160,185p'Repository: coleam00/Archon
Length of output: 1455
🏁 Script executed:
# Read the complete codebase schema file
cat -n packages/server/src/routes/schemas/codebase.schemas.ts | head -50Repository: coleam00/Archon
Length of output: 2013
🏁 Script executed:
# Search for generated API type files in the web package
fd -t f "api" packages/web/src/lib/ | head -20Repository: coleam00/Archon
Length of output: 126
🏁 Script executed:
# Search for allowEnvKeys or allow_env_keys in web package
rg -n 'allowEnvKeys|allow_env_keys' packages/web/src/ -t tsRepository: coleam00/Archon
Length of output: 307
🏁 Script executed:
# Search for updateCodebase schema or PATCH handling
rg -n 'updateCodebaseBodySchema|updateCodebaseSchema|PATCH.*codebases' packages/server/src/routes/ -t tsRepository: coleam00/Archon
Length of output: 41
🏁 Script executed:
# Search for any mention of allowEnvKeys on server side
rg -n 'allowEnvKeys|allow_env_keys' packages/server/src/ -t tsRepository: coleam00/Archon
Length of output: 41
Fix client-server contract misalignment for allowEnvKeys field.
The client sends and expects allowEnvKeys across three operations, but the server provides no support:
- POST /api/codebases (line 164): Client sends optional
allowEnvKeysbut server'saddCodebaseBodySchemaonly acceptsurlandpath—field is silently ignored. - PATCH /api/codebases/:id (line 175): Client function sends
allowEnvKeys, but server has no PATCH handler or schema defined. - Response contract (line 44): Client's
CodebaseResponseexpects mandatoryallow_env_keysfield, but server'scodebaseSchemadoes not include it.
Either add full server support (schema fields, handlers, database column if needed) or remove allowEnvKeys from the client API until the backend is ready.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/lib/api.ts` at line 44, Client-server contract mismatch: the
client uses allowEnvKeys / allow_env_keys but the server schemas and handlers
(addCodebaseBodySchema, codebaseSchema, and no PATCH handler for
/api/codebases/:id) don’t support it. Fix by either removing allowEnvKeys from
the client API or adding full server support: update addCodebaseBodySchema to
accept allow_env_keys, extend codebaseSchema to include allow_env_keys in
responses, implement a PATCH handler for /api/codebases/:id that validates and
persists allow_env_keys to the DB (and add a DB column/migration if needed), and
ensure the client and server use the same snake_case/key names in
request/response contracts.
Adds a slide-in conversations panel accessible from mobile via a 'Conversations' button in the chat topbar. Also fixes horizontal scroll parasite on mobile by adding overflow-x: hidden to html/body/#root.
Author
Nouveaux commits ajoutés (patch responsive mobile — phase 2)eat(mobile): add conversation drawer and overflow-x fix for mobile
\ ix(mobile): prevent horizontal scroll with overflow-x hidden on html/body/#root\
Tests visuels effectués
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/web/src/routes/ChatPage.tsx`:
- Line 234: The aria-label text "Fermer le panneau conversations" in the
ChatPage component is in French while the UI is English; locate the occurrences
of aria-label="Fermer le panneau conversations" inside the ChatPage React
component (both instances reported) and replace them with an English assistive
label such as "Close conversations panel" so the screen-reader text matches the
page locale; ensure both occurrences (the one around the first reported location
and the second around the later reported location) are updated to the identical
English string for consistency.
- Around line 206-216: The drawer width uses the persisted desktop width
directly (style on the container using width and flexShrink) which can exceed
the viewport on mobile; modify ChatPage.tsx to cap the applied width on small
screens by taking the min of the saved width and the current viewport width (or
apply a CSS maxWidth: '100vw') when mobileConvOpen is used, ensuring the drawer
never exceeds window.innerWidth and still respects PANEL_MAX for desktop; update
the width calculation (the expression that builds style={{ width:
`${String(width)}px`, flexShrink: 0 }}) to use Math.min(width,
window.innerWidth) or a responsive maxWidth fallback so controls aren’t clipped
on mobile.
- Around line 150-175: The handleAddSubmit function currently uses
.then/.catch/.finally promise chains; refactor it to be an async function using
try/catch/finally to follow async/await style: mark handleAddSubmit as async,
await addCodebase(input) inside a try block, call
queryClient.invalidateQueries({ queryKey: ['codebases'] }),
setSelectedProjectId(codebase.id), setShowAddInput(false), setAddValue('') and
clear addError on success, setAddError(err.message) in the catch, and reset
setAddLoading(false) in finally; keep existing logic for determining isLocalPath
and input and preserve dependencies (addValue, addLoading, queryClient,
setSelectedProjectId) in the hook declaration.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2d91c397-82b5-47e1-9634-e8643836de0e
📒 Files selected for processing (2)
packages/web/src/index.csspackages/web/src/routes/ChatPage.tsx
✅ Files skipped from review due to trivial changes (1)
- packages/web/src/index.css
Comment on lines
+150
to
+175
| const handleAddSubmit = useCallback((): void => { | ||
| const trimmed = addValue.trim(); | ||
| if (!trimmed || addLoading) return; | ||
|
|
||
| setAddLoading(true); | ||
| setAddError(null); | ||
|
|
||
| const isLocalPath = | ||
| trimmed.startsWith('/') || trimmed.startsWith('~') || /^[A-Za-z]:[/\\]/.test(trimmed); | ||
| const input = isLocalPath ? { path: trimmed } : { url: trimmed }; | ||
|
|
||
| void addCodebase(input) | ||
| .then(codebase => { | ||
| void queryClient.invalidateQueries({ queryKey: ['codebases'] }); | ||
| setSelectedProjectId(codebase.id); | ||
| setShowAddInput(false); | ||
| setAddValue(''); | ||
| setAddError(null); | ||
| }) | ||
| .catch((err: Error) => { | ||
| setAddError(err.message); | ||
| }) | ||
| .finally(() => { | ||
| setAddLoading(false); | ||
| }); | ||
| }, [addValue, addLoading, queryClient, setSelectedProjectId]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify promise-chain usage in this file (should be zero after refactor).
rg -nP --type=tsx '\.then\s*\(|\.catch\s*\(|\.finally\s*\(' packages/web/src/routes/ChatPage.tsxRepository: coleam00/Archon
Length of output: 86
🏁 Script executed:
#!/bin/bash
# First, let's check if the file exists and see its size
wc -l packages/web/src/routes/ChatPage.tsx
# Search for promise chains using correct syntax (tsx is treated as ts in ripgrep)
rg -n '\.then\s*\(|\.catch\s*\(|\.finally\s*\(' packages/web/src/routes/ChatPage.tsx
# Also read the specific lines mentioned in the review (150-175 and 177-181)
echo "=== Lines 150-175 ==="
sed -n '150,175p' packages/web/src/routes/ChatPage.tsx
echo "=== Lines 177-181 ==="
sed -n '177,181p' packages/web/src/routes/ChatPage.tsxRepository: coleam00/Archon
Length of output: 1277
Use async/await in handleAddSubmit (avoid promise chains).
Lines 150-175 use .then()/.catch()/.finally() chains which violate the async/await guideline. Convert to async/await with try/catch for clearer control flow.
Proposed refactor
- const handleAddSubmit = useCallback((): void => {
+ const handleAddSubmit = useCallback(async (): Promise<void> => {
const trimmed = addValue.trim();
if (!trimmed || addLoading) return;
setAddLoading(true);
setAddError(null);
const isLocalPath =
trimmed.startsWith('/') || trimmed.startsWith('~') || /^[A-Za-z]:[/\\]/.test(trimmed);
const input = isLocalPath ? { path: trimmed } : { url: trimmed };
- void addCodebase(input)
- .then(codebase => {
- void queryClient.invalidateQueries({ queryKey: ['codebases'] });
- setSelectedProjectId(codebase.id);
- setShowAddInput(false);
- setAddValue('');
- setAddError(null);
- })
- .catch((err: Error) => {
- setAddError(err.message);
- })
- .finally(() => {
- setAddLoading(false);
- });
+ try {
+ const codebase = await addCodebase(input);
+ await queryClient.invalidateQueries({ queryKey: ['codebases'] });
+ setSelectedProjectId(codebase.id);
+ setShowAddInput(false);
+ setAddValue('');
+ setAddError(null);
+ } catch (err: unknown) {
+ setAddError(err instanceof Error ? err.message : 'Failed to add project');
+ } finally {
+ setAddLoading(false);
+ }
}, [addValue, addLoading, queryClient, setSelectedProjectId]);
const handleAddKeyDown = useCallback(
(e: React.KeyboardEvent): void => {
if (e.key === 'Enter') {
- handleAddSubmit();
+ void handleAddSubmit();
} else if (e.key === 'Escape') {
setShowAddInput(false);📝 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.
Suggested change
| const handleAddSubmit = useCallback((): void => { | |
| const trimmed = addValue.trim(); | |
| if (!trimmed || addLoading) return; | |
| setAddLoading(true); | |
| setAddError(null); | |
| const isLocalPath = | |
| trimmed.startsWith('/') || trimmed.startsWith('~') || /^[A-Za-z]:[/\\]/.test(trimmed); | |
| const input = isLocalPath ? { path: trimmed } : { url: trimmed }; | |
| void addCodebase(input) | |
| .then(codebase => { | |
| void queryClient.invalidateQueries({ queryKey: ['codebases'] }); | |
| setSelectedProjectId(codebase.id); | |
| setShowAddInput(false); | |
| setAddValue(''); | |
| setAddError(null); | |
| }) | |
| .catch((err: Error) => { | |
| setAddError(err.message); | |
| }) | |
| .finally(() => { | |
| setAddLoading(false); | |
| }); | |
| }, [addValue, addLoading, queryClient, setSelectedProjectId]); | |
| const handleAddSubmit = useCallback(async (): Promise<void> => { | |
| const trimmed = addValue.trim(); | |
| if (!trimmed || addLoading) return; | |
| setAddLoading(true); | |
| setAddError(null); | |
| const isLocalPath = | |
| trimmed.startsWith('/') || trimmed.startsWith('~') || /^[A-Za-z]:[/\\]/.test(trimmed); | |
| const input = isLocalPath ? { path: trimmed } : { url: trimmed }; | |
| try { | |
| const codebase = await addCodebase(input); | |
| await queryClient.invalidateQueries({ queryKey: ['codebases'] }); | |
| setSelectedProjectId(codebase.id); | |
| setShowAddInput(false); | |
| setAddValue(''); | |
| setAddError(null); | |
| } catch (err: unknown) { | |
| setAddError(err instanceof Error ? err.message : 'Failed to add project'); | |
| } finally { | |
| setAddLoading(false); | |
| } | |
| }, [addValue, addLoading, queryClient, setSelectedProjectId]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/routes/ChatPage.tsx` around lines 150 - 175, The
handleAddSubmit function currently uses .then/.catch/.finally promise chains;
refactor it to be an async function using try/catch/finally to follow
async/await style: mark handleAddSubmit as async, await addCodebase(input)
inside a try block, call queryClient.invalidateQueries({ queryKey: ['codebases']
}), setSelectedProjectId(codebase.id), setShowAddInput(false), setAddValue('')
and clear addError on success, setAddError(err.message) in the catch, and reset
setAddLoading(false) in finally; keep existing logic for determining isLocalPath
and input and preserve dependencies (addValue, addLoading, queryClient,
setSelectedProjectId) in the hook declaration.
Comment on lines
+206
to
+216
| className={cn( | ||
| 'flex flex-col border-r border-border bg-surface overflow-hidden', | ||
| // Mobile: fixed full-height overlay | ||
| 'fixed inset-y-0 left-0 z-50', | ||
| // Desktop: back to normal inline flow | ||
| 'md:relative md:inset-auto md:z-auto md:h-full', | ||
| // Slide animation — mobile toggles, desktop always shown | ||
| 'transition-transform duration-300 ease-in-out', | ||
| mobileConvOpen ? 'translate-x-0' : '-translate-x-full md:translate-x-0' | ||
| )} | ||
| style={{ width: `${String(width)}px`, flexShrink: 0 }} |
There was a problem hiding this comment.
Cap mobile drawer width to viewport to prevent clipped controls.
Line 216 applies persisted desktop width directly on mobile. If width is saved near PANEL_MAX, the fixed drawer can render wider than the viewport and hide actionable UI on small screens.
📱 Proposed fix
- style={{ width: `${String(width)}px`, flexShrink: 0 }}
+ style={{ width: `min(100vw, ${String(width)}px)`, flexShrink: 0 }}📝 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.
Suggested change
| className={cn( | |
| 'flex flex-col border-r border-border bg-surface overflow-hidden', | |
| // Mobile: fixed full-height overlay | |
| 'fixed inset-y-0 left-0 z-50', | |
| // Desktop: back to normal inline flow | |
| 'md:relative md:inset-auto md:z-auto md:h-full', | |
| // Slide animation — mobile toggles, desktop always shown | |
| 'transition-transform duration-300 ease-in-out', | |
| mobileConvOpen ? 'translate-x-0' : '-translate-x-full md:translate-x-0' | |
| )} | |
| style={{ width: `${String(width)}px`, flexShrink: 0 }} | |
| className={cn( | |
| 'flex flex-col border-r border-border bg-surface overflow-hidden', | |
| // Mobile: fixed full-height overlay | |
| 'fixed inset-y-0 left-0 z-50', | |
| // Desktop: back to normal inline flow | |
| 'md:relative md:inset-auto md:z-auto md:h-full', | |
| // Slide animation — mobile toggles, desktop always shown | |
| 'transition-transform duration-300 ease-in-out', | |
| mobileConvOpen ? 'translate-x-0' : '-translate-x-full md:translate-x-0' | |
| )} | |
| style={{ width: `min(100vw, ${String(width)}px)`, flexShrink: 0 }} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/routes/ChatPage.tsx` around lines 206 - 216, The drawer
width uses the persisted desktop width directly (style on the container using
width and flexShrink) which can exceed the viewport on mobile; modify
ChatPage.tsx to cap the applied width on small screens by taking the min of the
saved width and the current viewport width (or apply a CSS maxWidth: '100vw')
when mobileConvOpen is used, ensuring the drawer never exceeds window.innerWidth
and still respects PANEL_MAX for desktop; update the width calculation (the
expression that builds style={{ width: `${String(width)}px`, flexShrink: 0 }})
to use Math.min(width, window.innerWidth) or a responsive maxWidth fallback so
controls aren’t clipped on mobile.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/web/src/components/layout/Layout.tsx`:
- Around line 15-18: The mobile nav state (open from useState in Layout and
provided via MobileNavContext.Provider) isn't reset when switching to desktop,
causing the drawer to reopen when returning to mobile; add a useEffect inside
the Layout component that listens for viewport changes (e.g., window.matchMedia
for the desktop breakpoint or window resize) and calls setOpen(false) when the
viewport crosses into desktop size, and ensure the listener is cleaned up on
unmount.
In `@packages/web/src/routes/ChatPage.tsx`:
- Around line 194-199: The mobile overlay (rendered when mobileConvOpen) is
being rendered with z-40/z-50 but the sticky mobile topbar (the sticky topbar
element with z-50) is rendered later and remains above the backdrop; update the
stacking so the overlay sits above the topbar by changing the overlay or topbar
z-index consistently (e.g., make the overlay z higher than the sticky topbar or
reduce the topbar z), ensuring the element rendered for the backdrop
(mobileConvOpen handler and its div) uses a z-class greater than the sticky
topbar’s z-class so the backdrop truly blocks and prevents clicks on the sticky
topbar.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 88711c7b-b41e-4c56-b64f-6b2487966b57
📒 Files selected for processing (3)
packages/web/src/components/chat/MessageInput.tsxpackages/web/src/components/layout/Layout.tsxpackages/web/src/routes/ChatPage.tsx
✅ Files skipped from review due to trivial changes (1)
- packages/web/src/components/chat/MessageInput.tsx
Comment on lines
+15
to
+18
| const [open, setOpen] = useState(false); | ||
|
|
||
| return ( | ||
| <MobileNavContext.Provider value={{ open, setOpen }}> |
There was a problem hiding this comment.
Reset the mobile nav state on desktop resize.
Line 15 keeps open alive, and Lines 25-42 only hide the drawer with responsive classes. If the menu is open during a rotate/resize, switching back to mobile reopens it unexpectedly.
🔧 Proposed fix
-import { useState } from 'react';
+import { useEffect, useState } from 'react';
@@
export function Layout(): React.ReactElement {
const [open, setOpen] = useState(false);
+
+ useEffect((): (() => void) => {
+ const onResize = (): void => {
+ if (window.innerWidth >= 768) {
+ setOpen(false);
+ }
+ };
+
+ window.addEventListener('resize', onResize);
+ return (): void => {
+ window.removeEventListener('resize', onResize);
+ };
+ }, []);
return (Also applies to: 25-42
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/components/layout/Layout.tsx` around lines 15 - 18, The
mobile nav state (open from useState in Layout and provided via
MobileNavContext.Provider) isn't reset when switching to desktop, causing the
drawer to reopen when returning to mobile; add a useEffect inside the Layout
component that listens for viewport changes (e.g., window.matchMedia for the
desktop breakpoint or window resize) and calls setOpen(false) when the viewport
crosses into desktop size, and ensure the listener is cleaned up on unmount.
Change drawer from 'fixed inset-y-0 z-50' to 'fixed top-12 bottom-0 z-40' so it opens below the 48px topbar instead of overlapping it.
Prevents the mobile hamburger drawer and its backdrop from overlapping the sticky topbar (z-50). Pattern matches ChatPage.tsx fix.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
packages/web/src/routes/ChatPage.tsx (3)
231-235:⚠️ Potential issue | 🟡 MinorKeep the
aria-labels in English to match the rest of the screen.These labels are still French while the visible UI copy is English. Please update both strings so assistive text stays consistent with the page locale.
Also applies to: 372-376
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/routes/ChatPage.tsx` around lines 231 - 235, The aria-labels in ChatPage.tsx are in French while the UI is English; update the two button elements (the one that calls setMobileConvOpen(false) and the other at the second occurrence that toggles mobile conversation panel) to use English aria-labels (e.g., "Close conversations panel" and "Open conversations panel" or similar) so the assistive text matches the page locale; locate the button elements in the ChatPage component that use setMobileConvOpen and replace the French strings with the appropriate English aria-labels.
205-216:⚠️ Potential issue | 🟠 MajorCap the mobile drawer width to the viewport.
This still applies the persisted desktop width directly on mobile, so a saved value near
PANEL_MAXcan overflow smaller screens and clip the drawer UI.Suggested fix
- style={{ width: `${String(width)}px`, flexShrink: 0 }} + style={{ width: `${String(width)}px`, maxWidth: '100vw', flexShrink: 0 }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/routes/ChatPage.tsx` around lines 205 - 216, The mobile drawer currently applies the persisted desktop "width" directly, which can overflow small viewports; update the ChatPage drawer element (the div using mobileConvOpen and the width variable) to cap the applied width on mobile by either adding a CSS constraint (e.g., max-width: 100vw / maxWidth: '100%') or computing a cappedWidth = Math.min(width, window.innerWidth || width) and using that for the style instead of raw width; ensure the style keeps flexShrink: 0 and only constrains the width value used for rendering on narrow screens.
150-175:⚠️ Potential issue | 🟠 MajorConvert
handleAddSubmittoasync/await.This still uses a
.then()/.catch()/.finally()chain, which keeps the error path and loading cleanup harder to follow than necessary. Please refactor this callback toasync/await, and once you do, call it asvoid handleAddSubmit()at Line 180. Based on learnings: Applies to**/*.{ts,tsx}: Always use async/await for promises instead of.then()chains or other promise handling patterns.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/routes/ChatPage.tsx` around lines 150 - 175, Convert the handleAddSubmit callback to an async function that uses try/catch/finally: mark function handleAddSubmit as async, compute trimmed/isLocalPath and input the same way, then await addCodebase(input) inside try, on success call queryClient.invalidateQueries({ queryKey: ['codebases'] }), setSelectedProjectId(codebase.id), setShowAddInput(false), setAddValue(''), and setAddError(null); in catch setAddError(err.message); in finally setAddLoading(false). Ensure you still guard early for empty trimmed or addLoading and setAddLoading(true)/setAddError(null) before the try block, and update the call site to invoke it as void handleAddSubmit().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/web/src/routes/ChatPage.tsx`:
- Around line 205-216: The panel currently only translates off-screen when
mobileConvOpen is false, leaving interactive elements reachable; update the
container (the div using mobileConvOpen in its className and style) so that when
mobileConvOpen is false on small screens it is non-interactive and hidden from
assistive tech—either conditionally don't render the panel on mobile or add
attributes/classes to disable interaction (e.g., aria-hidden="true", inert,
pointer-events-none, and remove focusability by ensuring child inputs/buttons
get tabIndex={-1} or are unmounted) while preserving the desktop behavior (md:
classes); use the existing mobileConvOpen flag to toggle this behavior.
---
Duplicate comments:
In `@packages/web/src/routes/ChatPage.tsx`:
- Around line 231-235: The aria-labels in ChatPage.tsx are in French while the
UI is English; update the two button elements (the one that calls
setMobileConvOpen(false) and the other at the second occurrence that toggles
mobile conversation panel) to use English aria-labels (e.g., "Close
conversations panel" and "Open conversations panel" or similar) so the assistive
text matches the page locale; locate the button elements in the ChatPage
component that use setMobileConvOpen and replace the French strings with the
appropriate English aria-labels.
- Around line 205-216: The mobile drawer currently applies the persisted desktop
"width" directly, which can overflow small viewports; update the ChatPage drawer
element (the div using mobileConvOpen and the width variable) to cap the applied
width on mobile by either adding a CSS constraint (e.g., max-width: 100vw /
maxWidth: '100%') or computing a cappedWidth = Math.min(width, window.innerWidth
|| width) and using that for the style instead of raw width; ensure the style
keeps flexShrink: 0 and only constrains the width value used for rendering on
narrow screens.
- Around line 150-175: Convert the handleAddSubmit callback to an async function
that uses try/catch/finally: mark function handleAddSubmit as async, compute
trimmed/isLocalPath and input the same way, then await addCodebase(input) inside
try, on success call queryClient.invalidateQueries({ queryKey: ['codebases'] }),
setSelectedProjectId(codebase.id), setShowAddInput(false), setAddValue(''), and
setAddError(null); in catch setAddError(err.message); in finally
setAddLoading(false). Ensure you still guard early for empty trimmed or
addLoading and setAddLoading(true)/setAddError(null) before the try block, and
update the call site to invoke it as void handleAddSubmit().
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8b3065bd-6df5-4485-b55e-12d82342ba14
📒 Files selected for processing (1)
packages/web/src/routes/ChatPage.tsx
Comment on lines
+205
to
+216
| <div | ||
| className={cn( | ||
| 'flex flex-col border-r border-border bg-surface overflow-hidden', | ||
| // Mobile: fixed overlay starting below the sticky topbar (~48 px) | ||
| 'fixed top-12 bottom-0 left-0 z-40', | ||
| // Desktop: back to normal inline flow | ||
| 'md:relative md:inset-auto md:z-auto md:h-full', | ||
| // Slide animation — mobile toggles, desktop always shown | ||
| 'transition-transform duration-300 ease-in-out', | ||
| mobileConvOpen ? 'translate-x-0' : '-translate-x-full md:translate-x-0' | ||
| )} | ||
| style={{ width: `${String(width)}px`, flexShrink: 0 }} |
There was a problem hiding this comment.
Hide the closed mobile drawer from focus and assistive tech.
When mobileConvOpen is false, this panel is only moved off-screen. Its buttons, inputs, and select remain mounted and can still be reached by keyboard/screen-reader users on mobile. Please also disable interaction when closed here—e.g. conditionally render it on mobile, or mark the closed state inert/non-focusable instead of relying on translate-x-full alone.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/routes/ChatPage.tsx` around lines 205 - 216, The panel
currently only translates off-screen when mobileConvOpen is false, leaving
interactive elements reachable; update the container (the div using
mobileConvOpen in its className and style) so that when mobileConvOpen is false
on small screens it is non-interactive and hidden from assistive tech—either
conditionally don't render the panel on mobile or add attributes/classes to
disable interaction (e.g., aria-hidden="true", inert, pointer-events-none, and
remove focusability by ensuring child inputs/buttons get tabIndex={-1} or are
unmounted) while preserving the desktop behavior (md: classes); use the existing
mobileConvOpen flag to toggle this behavior.
- Ajout du hook useVisualViewport qui suit window.visualViewport.height
pour que le conteneur principal se redimensionne quand le clavier mobile apparaît
- Layout.tsx : remplace h-dvh par le hook (style inline height) + overflow-hidden
- index.html : ajout interactive-widget=resizes-content dans la meta viewport
(Chrome Android redimensionne le viewport au lieu de chevaucher le clavier)
- index.css : html/body/#root overflow:hidden pour supprimer les scrollbars
parasites ; scrollbar-width:none + ::-webkit-scrollbar{display:none} sur mobile ;
scrollbars desktop conservées via media query min-width:769px
On iOS Safari the virtual keyboard overlays the page instead of resizing the viewport. Without position:fixed on the root container, iOS scrolls the layout viewport down when the keyboard opens, causing the chat input to remain hidden even though visualViewport.height correctly decreases. Setting position:fixed + top/left/right:0 + height:vpHeight pins the container to the visible area above the keyboard, so the MessageInput is always reachable.
…add dark/light theme toggle
Author
Author
This was referenced Apr 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Desktop behaviour is unchanged.
Changes
SSE Fix (Cloudflare Tunnel)
The previous SSE URL used window.location.hostname to build a direct connection to the backend port (http://:3090). This broke when accessed via Cloudflare Quick Tunnel because the tunnel only exposes the Vite dev server (port 3000), not the backend (port 3090).
Fix: Only use direct backend URL when on localhost. For any other hostname (tunnel, remote network), fall back to a relative URL so requests go through the Vite proxy, which correctly forwards them to the backend.
Test plan
Generated with Claude Sonnet 4.6
Summary by CodeRabbit
New Features
Enhancements
API