fix(web): improve mobile layout for file browser, toolbar, and footer#154
Merged
Conversation
Mobile web layout had three main issues: the toolbar with breadcrumbs and action buttons overflowed on narrow screens, there were no visible action buttons for files (only right-click context menu worked on desktop), and the footer looked sparse with hidden center links. Changes: - Stack toolbar vertically on mobile (breadcrumbs above, actions below) - Add per-row "..." action button visible only on mobile that opens the context menu, replacing the need for long-press discovery - Hide size/date columns on mobile to make room for action button - Center footer content on mobile instead of spreading it edge-to-edge - Remove duplicate mobile overrides from layout.css (consolidated in responsive.css) https://claude.ai/code/session_01HcPczWB3PzHSv2EwbSzXwo
|
No actionable comments were generated in the recent review. 🎉 WalkthroughAdds a mobile action column and button to the file list UI. FileList header gains an empty actions cell; FileListItem adds a mobile action button that synthesizes a MouseEvent to open the context menu. CSS and responsive rules add mobile-only action styling and adjust file-list/grid layouts. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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)
apps/web/src/styles/responsive.css (1)
91-94:.file-list-header-namehas no explicitgrid-area: name— relies on auto-placement
.file-list-item-row-topcorrectly hasgrid-area: name(infile-browser.css), so item rows are robust. However,.file-list-header-namehas nogrid-areadefined anywhere; it lands in the first column only becausefile-list-header-sizeandfile-list-header-datearedisplay: none. Adding any visible element to the header row without an explicit area would silently shift the name column out of the"name"area on mobile.♻️ Suggested fix — add to file-browser.css
.file-list-header-name { display: flex; align-items: center; + grid-area: name; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/styles/responsive.css` around lines 91 - 94, The header's name cell (.file-list-header-name) lacks an explicit grid-area so it relies on auto-placement and can shift if other header elements become visible; add a rule in file-browser.css that assigns grid-area: name to .file-list-header-name to match .file-list-item-row-top and ensure the header always occupies the "name" area alongside the existing .file-list-header grid-template-columns and hidden .file-list-header-size/.file-list-header-date.apps/web/src/styles/file-browser.css (1)
700-709: Separate:focusfrom:focus-visiblefor color/border changeThe combined
:hover, :focusselector appliescolorandborder-colorchanges on all focus events (including mouse-click focus), not just keyboard focus. The:focus-visiblerule below only adds theoutline, so mouse-click focus will silently change the button's color/border with no:focus-visibleindication—which is the inverse of the expected pattern.♻️ Suggested fix
-.file-list-item-action-btn:hover, -.file-list-item-action-btn:focus { +.file-list-item-action-btn:hover, +.file-list-item-action-btn:focus-visible { color: var(--color-text-primary); border-color: var(--color-green-primary); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/styles/file-browser.css` around lines 700 - 709, The current combined selector ".file-list-item-action-btn:hover, .file-list-item-action-btn:focus" causes color/border changes on all focus events (including mouse focus); update selectors so color and border changes apply only on hover and keyboard focus-visible: replace the use of ":focus" with ":focus-visible" for the color/border rule (i.e., use ".file-list-item-action-btn:hover, .file-list-item-action-btn:focus-visible"), keep the existing ".file-list-item-action-btn:focus-visible" rule for the outline, and do not apply color/border styles to the plain ".file-list-item-action-btn:focus" state so mouse-click focus won't change color/border unexpectedly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/components/file-browser/FileList.tsx`:
- Around line 122-124: The empty header element in the FileList component (the
element with className "file-list-header-actions") currently has
aria-hidden="true" which removes it from the accessibility tree and prevents the
role="columnheader" from ever being exposed; fix by removing the unconditional
aria-hidden and either drop the role (if it should be purely visual) or instead
keep the role="columnheader" and apply CSS-only hiding for desktop (e.g., hide
via media-query display/visibility classes) or use a scoped hiding utility
(visually-hidden or aria-hidden only when actually hidden) so the element is
available to assistive tech on mobile where it is visible.
In `@apps/web/src/components/file-browser/FileListItem.tsx`:
- Around line 144-160: The synthetic MouseEvent objects created in
handleActionButtonClick and handleTouchStart are missing stopPropagation,
causing a TypeError when useContextMenu.show calls event.stopPropagation();
update both synthetic event objects (the ones currently providing preventDefault
and clientX/clientY) to also include a no-op stopPropagation: () => {} so they
match the interface expected by useContextMenu.show.
---
Nitpick comments:
In `@apps/web/src/styles/file-browser.css`:
- Around line 700-709: The current combined selector
".file-list-item-action-btn:hover, .file-list-item-action-btn:focus" causes
color/border changes on all focus events (including mouse focus); update
selectors so color and border changes apply only on hover and keyboard
focus-visible: replace the use of ":focus" with ":focus-visible" for the
color/border rule (i.e., use ".file-list-item-action-btn:hover,
.file-list-item-action-btn:focus-visible"), keep the existing
".file-list-item-action-btn:focus-visible" rule for the outline, and do not
apply color/border styles to the plain ".file-list-item-action-btn:focus" state
so mouse-click focus won't change color/border unexpectedly.
In `@apps/web/src/styles/responsive.css`:
- Around line 91-94: The header's name cell (.file-list-header-name) lacks an
explicit grid-area so it relies on auto-placement and can shift if other header
elements become visible; add a rule in file-browser.css that assigns grid-area:
name to .file-list-header-name to match .file-list-item-row-top and ensure the
header always occupies the "name" area alongside the existing .file-list-header
grid-template-columns and hidden .file-list-header-size/.file-list-header-date.
Comment on lines
+122
to
124
| {/* Empty header cell for mobile action column - hidden on desktop via CSS */} | ||
| <div className="file-list-header-actions" role="columnheader" aria-hidden="true" /> | ||
| </div> |
There was a problem hiding this comment.
role="columnheader" is unreachable due to aria-hidden="true"
aria-hidden="true" unconditionally removes the element from the accessibility tree — including on mobile where it becomes visible via CSS. The role="columnheader" annotation is therefore never exposed to screen readers. Drop the role or replace with a scoped hide.
♻️ Suggested fix
-<div className="file-list-header-actions" role="columnheader" aria-hidden="true" />
+<div className="file-list-header-actions" aria-hidden="true" />📝 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
| {/* Empty header cell for mobile action column - hidden on desktop via CSS */} | |
| <div className="file-list-header-actions" role="columnheader" aria-hidden="true" /> | |
| </div> | |
| {/* Empty header cell for mobile action column - hidden on desktop via CSS */} | |
| <div className="file-list-header-actions" aria-hidden="true" /> | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/file-browser/FileList.tsx` around lines 122 - 124,
The empty header element in the FileList component (the element with className
"file-list-header-actions") currently has aria-hidden="true" which removes it
from the accessibility tree and prevents the role="columnheader" from ever being
exposed; fix by removing the unconditional aria-hidden and either drop the role
(if it should be purely visual) or instead keep the role="columnheader" and
apply CSS-only hiding for desktop (e.g., hide via media-query display/visibility
classes) or use a scoped hiding utility (visually-hidden or aria-hidden only
when actually hidden) so the element is available to assistive tech on mobile
where it is visible.
- Remove unreachable role="columnheader" from aria-hidden element - Add missing stopPropagation() to synthetic MouseEvent objects in handleActionButtonClick and handleTouchStart (prevents TypeError when useContextMenu.show() calls event.stopPropagation()) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
FSM1
temporarily deployed
to
claude/fix-mobile-web-layout-Lz4pl - cipher-box PR #154
Render
Destroyed
Closed
Merged
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
2 participants
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.
Mobile web layout had three main issues: the toolbar with breadcrumbs
and action buttons overflowed on narrow screens, there were no visible
action buttons for files (only right-click context menu worked on
desktop), and the footer looked sparse with hidden center links.
Changes:
the context menu, replacing the need for long-press discovery
responsive.css)
https://claude.ai/code/session_01HcPczWB3PzHSv2EwbSzXwo
Summary by CodeRabbit
New Features
Style