Conversation
add toolbar to mobile
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughImplements a collapsible left toolbar with a burger toggle, removes active-state styling for toolbar buttons, adds mobile-responsive CSS for chat and layout, and injects JS to manage open/close state with media-query awareness and a minimal window.__leftToolbar API. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant B as Burger Button
participant LT as Left Toolbar (nav#left-toolbar)
participant BD as Body (document.body)
participant MQ as MediaQuery (min-width:768px)
participant API as window.__leftToolbar
note over MQ,LT: Initialize on load
MQ->>API: Determine default state (desktop=open, mobile=collapsed)
API->>LT: Add/Remove .collapsed
API->>BD: Add/Remove .left-toolbar-open
API->>B: Set aria-pressed/expanded, title
U->>B: pointerdown (toggle)
B->>API: toggle()
API->>LT: Toggle .collapsed
API->>BD: Toggle .left-toolbar-open
API->>B: Update ARIA/title
MQ-->>API: change (viewport resized)
API->>LT: Sync state for breakpoint
API->>BD: Sync body class
API->>B: Sync ARIA/title
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
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. ✨ 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 (
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/public/css/responsive.css (1)
58-63: Unify all remaining 768px media queries to 767px for consistency.There are multiple blocks still using
@media (max-width: 768px). Align them all to767pxto avoid the 768px overlap with desktop logic.Example (apply similarly to all blocks listed in this comment):
-@media (max-width: 768px) { +@media (max-width: 767px) {Also applies to: 65-79, 81-99, 101-177, 179-200, 203-208, 211-224, 225-256, 258-264, 267-287, 291-296, 299-305, 308-314
🧹 Nitpick comments (7)
app/public/css/chat-components.css (2)
36-47: Remove no-op transition or make it effective.
.chat-containertransitionsmargin-lefthere, but this selector never changesmargin-leftin this file. The shift is applied to#containerin responsive.css. Keep transitions only where properties actually change to avoid confusion and unnecessary paint costs.Apply:
.chat-container { @@ - /* Ensure content is accessible when toolbar is open */ - transition: margin-left 220ms ease; + /* No margin-left transition here: #container owns the shift */
31-48: Respect prefers-reduced-motion for smoother accessibility.Mobile introduces several transitions. Offer an opt-out for motion-sensitive users.
Append right after this block:
+@media (max-width: 767px) and (prefers-reduced-motion: reduce) { + .chat-container { transition: none !important; } +}app/public/css/responsive.css (2)
22-23: Add prefers-reduced-motion guard for layout shifts.The container shifts via
margin-leftanimation. Honor users who prefer reduced motion.Append:
+@media (max-width: 767px) and (prefers-reduced-motion: reduce) { + #container, + .chat-container { + transition: none !important; + } +}Also applies to: 225-231
133-149: Review responsive header sizing at narrow viewports (≤320px)The three header controls at app/public/css/responsive.css (lines 133–149) each have flex: 1 and min-width: 0 but are capped at 30%/35%/35% of the container—with a gap of 8 px between items—so at a 320 px viewport you end up with:
- Container content width: 320 px – (2×10 px padding) = 300 px
- Total max-width: 30%·300 px + 35%·300 px + 35%·300 px = 300 px
- Total gaps: 2×8 px = 16 px
→ 300 px + 16 px = 316 px > 300 px availableThis will force at least one control below its cap and risks visible overflow or clipping.
• Pinpoint locations:
– app/public/css/responsive.css:133–135 (#graph-selectmax-width: 30%)
– app/public/css/responsive.css:137–142 (#custom-file-uploadmax-width: 35%)
– app/public/css/responsive.css:146–149 (.dropdown-containermax-width: 35%)• Next steps:
- Manually test the chat header at exactly 320 px (e.g. using your browser’s device toolbar) and confirm there’s no horizontal scroll or clipped controls.
- If you see overflow, consider either:
- Reducing the caps (for example, 28%/32%/32%), or
- Allowing the controls to wrap (e.g. add
flex-wrap: wrapbelow 360 px), or- Introducing a tighter media query (
@media (max-width: 360px) { … }) with adjusted layoutapp/public/css/buttons.css (1)
362-375: Use a shared CSS variable for the 48px toolbar width and honor reduced motion.You hard-code 48px repeatedly. Switch to a custom property with a fallback and add a motion guard.
Apply representative changes (repeat similarly for all 48px instances in this file):
-#left-toolbar { +/* Use var with fallback in case :root isn't loaded yet */ #left-toolbar { position: fixed; top: 0px; bottom: 0px; - width: 48px; + width: var(--left-toolbar-width, 48px); @@ -#left-toolbar.collapsed #burger-toggle-btn { +#left-toolbar.collapsed #burger-toggle-btn { display: flex; - width: 48px; - height: 48px; + width: var(--left-toolbar-width, 48px); + height: var(--left-toolbar-width, 48px); @@ -#left-toolbar:not(.collapsed) { +#left-toolbar:not(.collapsed) { - width: 48px; + width: var(--left-toolbar-width, 48px); - padding: 6px 6px; + padding: 6px 6px;And append once:
+@media (prefers-reduced-motion: reduce) { + #left-toolbar { transition: none !important; } + #left-toolbar.collapsed { transition: none !important; } +}Also applies to: 393-438
app/templates/components/left_toolbar.j2 (2)
75-92: Optionally hide collapsed toolbar from the accessibility tree.When collapsed, most buttons are
display:none, but marking the nav as inert/hidden avoids accidental AT navigation into the container.Apply inside
setOpen:function setOpen(open) { if (open) { nav.classList.remove('collapsed'); document.body.classList.add('left-toolbar-open'); + nav.removeAttribute('inert'); + nav.removeAttribute('aria-hidden'); @@ } else { nav.classList.add('collapsed'); document.body.classList.remove('left-toolbar-open'); + nav.setAttribute('inert', ''); + nav.setAttribute('aria-hidden', 'true');Note:
inertis broadly supported now; degrades harmlessly if unsupported.
94-104: Persist user preference across reloads (optional).Mobile users expect the last open/closed state to stick.
Append minimal persistence:
- try { - setOpen(mq.matches); + try { + const saved = localStorage.getItem('leftToolbarOpen'); + setOpen(saved != null ? saved === '1' : mq.matches); } catch (e) { // fail-safe: keep open setOpen(true); } @@ function setOpen(open) { if (open) { @@ } else { @@ } + try { localStorage.setItem('leftToolbarOpen', open ? '1' : '0'); } catch {} }I can wire this and add a Cypress/Playwright check if you’d like.
📜 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 sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
app/public/css/buttons.css(1 hunks)app/public/css/chat-components.css(1 hunks)app/public/css/responsive.css(3 hunks)app/templates/components/left_toolbar.j2(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e-tests
- GitHub Check: e2e-tests
🔇 Additional comments (2)
app/public/css/buttons.css (1)
393-438: Approve: mobile collapsed/open behavior is thoughtfully handled.The collapsed state with pointer-events gating and explicit visibility of the burger works well on touch targets. Good job keeping “no-JS” mobile fallback coherent.
app/templates/components/left_toolbar.j2 (1)
94-111: Approve: robust mq listener (new/old APIs) and clean state sync.Nice use of
addEventListener('change')withaddListenerfallback and centralizedsetOpento update DOM + ARIA state.
| #left-toolbar.collapsed #burger-toggle-btn { | ||
| display: flex; | ||
| width: 48px; | ||
| height: 48px; | ||
| margin: 6px 0; | ||
| position: absolute; | ||
| top: 6px; | ||
| left: 6px; | ||
| z-index: 9999; /* keep on top so it's always clickable */ | ||
| pointer-events: auto; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Lower burger z-index to fit your stacking context (avoid covering modals).
Burger uses z-index: 9999, while modals/buttons are in the ~100–2000 range. This can put the burger above dialogs/tooltips unintentionally.
Apply:
#left-toolbar.collapsed #burger-toggle-btn {
display: flex;
width: 48px;
height: 48px;
margin: 6px 0;
position: absolute;
top: 6px;
left: 6px;
- z-index: 9999; /* keep on top so it's always clickable */
+ z-index: 2001; /* above header buttons (2000), below modals as needed */
pointer-events: auto;
}Scan z-index usage to confirm the scale:
🏁 Script executed:
#!/bin/bash
rg -nP 'z-index\s*:\s*\d+' app/public/css | sort -t: -k3,3nLength of output: 848
Lower burger toggle z-index to sit below modals and above header buttons
The #left-toolbar.collapsed #burger-toggle-btn is currently set to z-index: 9999, which will render it above all modals (whose highest z-index in app/public/css/modals.css is 3000) and risk covering dialogs or tooltips. We should reduce it to 2001 so it still appears above header buttons (z-index 2000) but below any modal layers.
Affected location:
- File: app/public/css/buttons.css
Lines: ~381–391
Proposed change:
#left-toolbar.collapsed #burger-toggle-btn {
display: flex;
width: 48px;
height: 48px;
margin: 6px 0;
position: absolute;
top: 6px;
left: 6px;
- z-index: 9999; /* keep on top so it's always clickable */
+ z-index: 2001; /* above header buttons (2000), below modals (>=3000) */
pointer-events: auto;
}📝 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.
| #left-toolbar.collapsed #burger-toggle-btn { | |
| display: flex; | |
| width: 48px; | |
| height: 48px; | |
| margin: 6px 0; | |
| position: absolute; | |
| top: 6px; | |
| left: 6px; | |
| z-index: 9999; /* keep on top so it's always clickable */ | |
| pointer-events: auto; | |
| } | |
| #left-toolbar.collapsed #burger-toggle-btn { | |
| display: flex; | |
| width: 48px; | |
| height: 48px; | |
| margin: 6px 0; | |
| position: absolute; | |
| top: 6px; | |
| left: 6px; | |
| z-index: 2001; /* above header buttons (2000), below modals (>=3000) */ | |
| pointer-events: auto; | |
| } |
🤖 Prompt for AI Agents
In app/public/css/buttons.css around lines 381 to 391 the
#left-toolbar.collapsed #burger-toggle-btn rule sets z-index: 9999 which forces
the burger toggle above modals; change that z-index to 2001 so the toggle
remains above header buttons (z-index 2000) but beneath modal layers (max
z-index ~3000) — update the z-index value in that rule to 2001 and keep the rest
of the rule unchanged.
| @media (max-width: 768px) { | ||
| .chat-messages { | ||
| padding: 2px; | ||
| } | ||
|
|
||
| .chat-container { | ||
| padding-right: 10px; | ||
| padding-left: 10px; | ||
| padding-top: 10px; | ||
| padding-bottom: 10px; | ||
| /* Ensure no horizontal overflow */ | ||
| max-width: 100vw; | ||
| overflow-x: hidden; | ||
| box-sizing: border-box; | ||
| /* Ensure content is accessible when toolbar is open */ | ||
| transition: margin-left 220ms ease; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix 768 vs 767 breakpoint mismatch (off-by-one at exactly 768px).
JS uses (min-width: 768px) for “desktop”, and buttons.css uses @media (max-width: 767px) for “mobile”. Here you use @media (max-width: 768px), which causes the mobile chat rules to apply at exactly 768px while the toolbar treats 768px as desktop. Unify to 767px to avoid layout jitter at 768px.
Apply:
-@media (max-width: 768px) {
+@media (max-width: 767px) {🤖 Prompt for AI Agents
In app/public/css/chat-components.css around lines 31 to 48, the media query
uses @media (max-width: 768px) which conflicts with other files that treat 768px
as desktop; change the breakpoint to @media (max-width: 767px) so mobile styles
apply up to 767px and 768px is consistently handled as desktop; update that
single media rule and verify related comments remain accurate.
| /* When left toolbar is open, push content to make room */ | ||
| body.left-toolbar-open .chat-container { | ||
| max-width: calc(100vw - 48px); | ||
| } | ||
|
|
||
| /* Ensure body doesn't overflow */ | ||
| body { | ||
| max-width: 100vw; | ||
| overflow-x: hidden; | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Standardize breakpoint and de-duplicate the “48px” constant via a CSS variable.
- Breakpoint: Align with JS/buttons (desktop ≥ 768, mobile ≤ 767). Use
max-width: 767pxhere. - DRY: The left toolbar width (48px) is hard-coded multiple times. Use a shared custom property so updates don’t require multi-file edits.
Apply:
-@media (max-width: 768px) {
+@media (max-width: 767px) {
- body.left-toolbar-open .chat-container {
- max-width: calc(100vw - 48px);
- }
+ body.left-toolbar-open .chat-container {
+ max-width: calc(100vw - var(--left-toolbar-width, 48px));
+ }
@@
- #container {
+ #container {
width: 100%;
max-width: 100vw;
overflow-x: hidden;
box-sizing: border-box;
transition: margin-left 220ms ease;
}
@@
- body.left-toolbar-open #container {
- margin-left: 48px;
- width: calc(100% - 48px);
- max-width: calc(100vw - 48px);
- }
+ body.left-toolbar-open #container {
+ margin-left: var(--left-toolbar-width, 48px);
+ width: calc(100% - var(--left-toolbar-width, 48px));
+ max-width: calc(100vw - var(--left-toolbar-width, 48px));
+ }And near the top of a globally loaded CSS (or here if that’s acceptable):
+:root {
+ /* Single source of truth for the mobile toolbar width */
+ --left-toolbar-width: 48px;
+}Also applies to: 16-24, 25-31, 37-41, 43-54
🤖 Prompt for AI Agents
In app/public/css/responsive.css around lines 5 to 15, the CSS uses a hard-coded
48px toolbar width and an inconsistent breakpoint; create a custom property
(e.g., --left-toolbar-width: 48px) near the top of a globally loaded CSS file
(or at the top of this file) and replace all literal 48px occurrences with
calc(var(--left-toolbar-width)); update the responsive breakpoint to use
max-width: 767px for mobile (and ensure any corresponding media queries follow
desktop ≥768px / mobile ≤767px logic), and apply the same replacement for the
other affected ranges (lines 16-24, 25-31, 37-41, 43-54).
| <button id="burger-toggle-btn" class="action-button toolbar-button" title="Open menu" aria-label="Open menu"> | ||
| <svg width="20" height="20" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg"> | ||
| <rect x="3" y="6" width="18" height="2" rx="1" fill="currentColor" /> | ||
| <rect x="3" y="11" width="18" height="2" rx="1" fill="currentColor" /> | ||
| <rect x="3" y="16" width="18" height="2" rx="1" fill="currentColor" /> | ||
| </svg> | ||
| </button> | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add aria-controls and initial aria-expanded for the burger button.
Improves AT context and aligns with the toggling behavior you already implement.
Apply:
-<button id="burger-toggle-btn" class="action-button toolbar-button" title="Open menu" aria-label="Open menu">
+<button id="burger-toggle-btn"
+ class="action-button toolbar-button"
+ title="Open menu"
+ aria-label="Open menu"
+ aria-controls="left-toolbar"
+ aria-expanded="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.
| <button id="burger-toggle-btn" class="action-button toolbar-button" title="Open menu" aria-label="Open menu"> | |
| <svg width="20" height="20" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg"> | |
| <rect x="3" y="6" width="18" height="2" rx="1" fill="currentColor" /> | |
| <rect x="3" y="11" width="18" height="2" rx="1" fill="currentColor" /> | |
| <rect x="3" y="16" width="18" height="2" rx="1" fill="currentColor" /> | |
| </svg> | |
| </button> | |
| <button id="burger-toggle-btn" | |
| class="action-button toolbar-button" | |
| title="Open menu" | |
| aria-label="Open menu" | |
| aria-controls="left-toolbar" | |
| aria-expanded="false"> | |
| <svg width="20" height="20" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg"> | |
| <rect x="3" y="6" width="18" height="2" rx="1" fill="currentColor" /> | |
| <rect x="3" y="11" width="18" height="2" rx="1" fill="currentColor" /> | |
| <rect x="3" y="16" width="18" height="2" rx="1" fill="currentColor" /> | |
| </svg> | |
| </button> |
🤖 Prompt for AI Agents
In app/templates/components/left_toolbar.j2 around lines 6 to 13, the burger
toggle button is missing ARIA attributes: add aria-controls="<id-of-side-menu>"
referencing the id of the element the button toggles and set an initial
aria-expanded="false" on the button; ensure the referenced side menu element has
the same id and update aria-expanded to "true"/"false" when the menu is
opened/closed by your existing toggle logic.
| <button id="theme-toggle-btn" class="action-button toolbar-button" title="Toggle Theme" | ||
| aria-label="Toggle theme" aria-pressed="false" tabindex="-1"> | ||
| <svg class="theme-icon" width="18" height="18" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg"> | ||
| <circle class="sun" cx="12" cy="12" r="5" stroke="currentColor" stroke-width="2"/> | ||
| <path class="sun" d="M12 1v2m0 18v2M4.22 4.22l1.42 1.42m12.72 12.72l1.42 1.42M1 12h2m18 0h2M4.22 19.78l1.42-1.42M18.36 5.64l1.42-1.42" stroke="currentColor" stroke-width="2"/> | ||
| <path class="moon" d="M21 12.79A9 9 0 1 1 11.21 3 7 7 0 0 0 21 12.79z" stroke="currentColor" stroke-width="2" fill="currentColor"/> | ||
| <svg class="theme-icon" width="18" height="18" viewBox="0 0 24 24" fill="none" | ||
| xmlns="http://www.w3.org/2000/svg"> | ||
| <circle class="sun" cx="12" cy="12" r="5" stroke="currentColor" stroke-width="2" /> |
There was a problem hiding this comment.
Keyboard accessibility: remove tabindex="-1" from toolbar items (or implement a roving tabindex).
With tabindex="-1", Theme and Schema buttons are unreachable via Tab. Either remove the attribute or implement full roving tabindex with Arrow key handling.
Apply minimal fix:
-<button id="theme-toggle-btn" class="action-button toolbar-button" title="Toggle Theme"
- aria-label="Toggle theme" aria-pressed="false" tabindex="-1">
+<button id="theme-toggle-btn" class="action-button toolbar-button" title="Toggle Theme"
+ aria-label="Toggle theme" aria-pressed="false">
@@
-<button class="action-button toolbar-button" id="schema-button" title="Schema" aria-label="Open schema"
- aria-pressed="false" tabindex="-1">
+<button class="action-button toolbar-button" id="schema-button" title="Schema" aria-label="Open schema"
+ aria-pressed="false">If you prefer roving tabindex, I can provide a compact handler.
Also applies to: 48-50
🤖 Prompt for AI Agents
In app/templates/components/left_toolbar.j2 around lines 15-19 (and also lines
48-50), toolbar buttons currently use tabindex="-1", which prevents keyboard Tab
focus; remove the tabindex="-1" attributes from these button elements so they
become tabbable by default (or, if you opt for a roving tabindex solution,
implement a single tabindex="0" for the active item plus Arrow key handlers to
move focus and update tabindex values accordingly). Ensure all toolbar buttons
are reachable via Tab after the change.
|
|
||
| // Media query: desktop when >= 768px (adjust if your breakpoints differ) | ||
| const mq = window.matchMedia('(min-width: 768px)'); | ||
|
|
||
| // Initialize based on media query: desktop -> open, mobile -> closed | ||
| try { | ||
| setOpen(mq.matches); | ||
| } catch (e) { | ||
| // fail-safe: keep open | ||
| setOpen(true); | ||
| } | ||
|
|
||
| // Listen for viewport changes and update default state accordingly | ||
| if (typeof mq.addEventListener === 'function') { | ||
| mq.addEventListener('change', (ev) => setOpen(ev.matches)); | ||
| } else if (typeof mq.addListener === 'function') { | ||
| mq.addListener((ev) => setOpen(ev.matches)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid preventing focus for mouse users; restrict pointerdown handling to touch/pen only.
e.preventDefault() on pointerdown suppresses focus for mouse clicks and may harm accessibility. Use it only for touch/pen to avoid double-toggles; let mouse clicks focus the button.
Apply:
- btn.addEventListener('pointerdown', function (e) {
- // Prevent unwanted focus/ghost clicks and handle touch/pen/mouse uniformly
- e.preventDefault();
- handleToggleEvent();
- // ignore the following 'click' event which may also fire
- ignoreNextClick = true;
- });
+ btn.addEventListener('pointerdown', function (e) {
+ if (e.pointerType === 'touch' || e.pointerType === 'pen') {
+ e.preventDefault(); // avoid 300ms delay / ghost click
+ handleToggleEvent();
+ ignoreNextClick = true; // consume the synthesized click
+ }
+ });Also applies to: 119-135
🤖 Prompt for AI Agents
In app/templates/components/left_toolbar.j2 around lines 94-111 (and also apply
same fix to 119-135), the current pointerdown handler calls e.preventDefault()
unconditionally which prevents mouse clicks from focusing the button; change the
handler to only call e.preventDefault() when the pointer is touch or pen (e.g.
if (e.pointerType === 'touch' || e.pointerType === 'pen') { e.preventDefault();
}) so mouse pointer events still allow normal focus and keyboard accessibility
while preventing double-toggle on touch/pen.
1 participant
Summary by CodeRabbit