feat: improve UI and fix responsive layout - #412
Conversation
PR Context Summary
Suggested issue links
Use |
|
Warning Review limit reached
Next review available in: 31 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe settings interface now has responsive spacing, larger touch targets, clearer interactive states, and additional accessibility attributes. The About section also provides loading feedback while copying the support summary. ChangesSettings UI refinements
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
ApprovabilityVerdict: Needs human review Unable to check for correctness in 57ef45a. UI and accessibility improvements with low runtime risk, but the author does not own any of the 7 modified files (all owned by Abhash-Chakraborty). The designated code owner should review these changes. You can customize Macroscope's approvability policy. Learn more. |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/app/settings/page.tsx`:
- Line 153: Update the retry button that calls settingsQuery.refetch() to set
disabled and aria-busy from settingsQuery.isFetching, preserving the existing
styling while preventing repeated requests and exposing the fetch state.
In `@frontend/src/components/trash-retention-settings.tsx`:
- Around line 38-42: Update the hint around the button in the trash-retention
settings component so it is not a focusable no-op: either render the explanation
as accessible passive text without a button, or implement an actual disclosure
with activation behavior plus matching aria-expanded and aria-controls
attributes. Preserve the existing hint content and styling where applicable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3199abea-05ad-47be-9bc3-e988e32d5f87
📒 Files selected for processing (7)
frontend/src/app/settings/page.tsxfrontend/src/components/about-settings.tsxfrontend/src/components/ai-runtime-settings.tsxfrontend/src/components/appearance-settings.tsxfrontend/src/components/hardware-accel-settings.tsxfrontend/src/components/map-privacy-settings.tsxfrontend/src/components/trash-retention-settings.tsx
|
Thanks for picking this up — the a11y bits are genuinely good (focus-visible rings, aria-busy on the copy button, live region on hardware detection). Two things block a merge for me though: the toggles in Also the map-privacy toggle switched its checked colour from |
|
Thanks for the detailed review. I'll revert the visual toggle changes (height, thumb color, and checked color), keep the larger touch target by increasing the hit area instead of the switch itself, and either implement a proper disclosure for the info button or replace it with non-interactive text. I'll also attach before/after screenshots for mobile and desktop in both light and dark themes in the updated PR. |
|
Thanks for the review! I've addressed the requested changes: Restored the toggle visuals to h-7 w-12 while keeping a larger touch target via padding. I'd appreciate another review when you have a chance. |
The PR set out to fix responsive layout, and the toggle and a11y work is right, but the page still scrolled sideways at phone widths - about 150px of overflow at 390px, clipping text on every card. Cause is the two-column grid. Columns are only declared at lg:, so at mobile the single implicit track is auto-sized, and an auto track sizes to max-content rather than to the container. One non-wrapping value in the About card was enough to stretch that track to ~650px inside a 358px container and push the whole document into horizontal scroll. Declaring the base track as minmax(0,1fr) constrains it; documentElement.scrollWidth at a 390px viewport goes from 543 to 390. The About rows contributed the max-content in the first place: the truncate utility implies white-space: nowrap, and it was applied at every breakpoint even though the row stacks below sm:. Now the value wraps on phones, where it has a full line to itself, and only truncates from sm: up where it shares a line with the label. This overflow predates the PR - canary has it too - but fixing responsive layout is what the PR is for, so it belongs here. Verified at 390px and 1280px: biome clean, 300 unit tests, production build, 14 Playwright e2e. Also probed the new toggles - 48x28 visual with a 64x44 hit area from the ::before expansion, and no overlap with any neighbouring interactive element - and the trash-retention disclosure, which correctly flips aria-expanded and reveals its panel.
Abhash-Chakraborty
added
frontend
Abhash-Chakraborty
left a comment
Abhash-Chakraborty
left a comment
There was a problem hiding this comment.
Thanks for turning this around properly — the toggles are back to a proper pill with a white thumb, the touch target comes from a ::before expansion instead of distorting the switch, the map colour is reverted, and the info button is a real disclosure now.
I verified rather than eyeballed: both toggles measure 48x28 visually with a 64x44 hit area and no overlap with any neighbouring interactive element, and the disclosure flips aria-expanded and reveals its panel.
One thing was still outstanding, so I pushed a fix: the page still scrolled sideways at phone widths (~150px of overflow at 390px, clipping text on every card). The two-column grid only declares columns at lg:, so on mobile the implicit track is auto — which sizes to max-content, not to the container. One non-wrapping value in the About card stretched it to ~650px inside a 358px container. Declaring the base track as minmax(0,1fr) fixes it; scrollWidth at 390px goes 543 → 390. Also made the About value wrap on phones instead of truncating, since truncate implies nowrap and was what produced the max-content in the first place.
That overflow predates your PR — canary has it too — but fixing responsive layout is what this PR is for, so it belongs here. Approving.
|
@macroscope-app review Please review this PR against its linked issue, local-first privacy rules, and the current Find repo instructions. |
40fcb16
into
Abhash-Chakraborty:canary
2 participants
Summary
Improved the Settings page UI/UX by enhancing layout consistency, responsiveness, and accessibility while preserving all existing functionality.
Fixes #358
Type of change
Release impact
What changed
How to test
npm installnpm run devbiome check tsc --noEmit npm test npm run buildChecklist
canaryunless it is the maintainer promotion PRGSSoC'26 checklist
Summary by CodeRabbit
Style
Accessibility
Bug Fixes