Skip to content

fix(settings): contain mobile dialog content - #1613

Merged
giswqs merged 1 commit into
mainfrom
fix/issue-1605-settings-mobile-overflow
Aug 1, 2026
Merged

fix(settings): contain mobile dialog content#1613
giswqs merged 1 commit into
mainfrom
fix/issue-1605-settings-mobile-overflow

Conversation

@giswqs

@giswqs giswqs commented Aug 1, 2026

Copy link
Copy Markdown
Member

Summary

  • constrain the Settings content grid to the dialog width on narrow screens
  • make the section navigation horizontally scrollable on mobile
  • keep Settings section content from expanding beyond the viewport

Verification

  • reproduced and verified at a 393 x 852 viewport
  • verified AI Providers in English and Chinese
  • verified light and dark themes
  • npm run build
  • pre-commit run --files apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx

Fixes #1605

Summary by CodeRabbit

  • Style
    • Improved the Settings dialog layout across different screen sizes.
    • Preserved horizontal scrolling for mobile navigation and vertical scrolling for desktop navigation.

Copilot AI review requested due to automatic review settings August 1, 2026 14:01
@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 89bfed3f-3fa3-49ea-9c71-9a21f4a4edd7

📥 Commits

Reviewing files that changed from the base of the PR and between 1247916 and 3d80469.

📒 Files selected for processing (1)
  • apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx

📝 Walkthrough

Walkthrough

The Settings dialog now applies minimum-width constraints to its layout containers. Mobile navigation supports horizontal scrolling. Desktop navigation retains vertical scrolling.

Changes

Settings dialog layout

Layer / File(s) Summary
Responsive containers and navigation
apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx
The dialog containers use min-w-0 so grid children can shrink. Navigation scrolls horizontally on small screens and vertically on desktop.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Suggested reviewers: copilot, craun718, rahuldshetty

Poem

A rabbit hops through settings bright,
With tidy widths arranged just right.
Tabs glide left on screens so small,
Desktop scrolls up the hall.
“Responsive hops!” the rabbit sings.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the fix for mobile Settings dialog content containment.
Linked Issues check ✅ Passed The changes address issue #1605 by constraining mobile Settings content and enabling horizontal navigation scrolling.
Out of Scope Changes check ✅ Passed All reported changes relate to responsive Settings dialog layout and the linked mobile UI issue.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/issue-1605-settings-mobile-overflow

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adjusts the Settings dialog layout to prevent content from overflowing the dialog/viewport on narrow (mobile) screens, addressing the UI overflow reported in #1605.

Changes:

  • Add min-w-0 to the Settings dialog grid and content pane to allow proper shrinking within the dialog on small screens.
  • Make the section navigation horizontally scrollable on mobile via overflow-x-auto, while keeping the existing desktop behavior at md+.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

🔍 Cloudflare PR preview

Item Value
Site https://54d5bdf6.geolibre-preview.pages.dev
Demo app https://54d5bdf6.geolibre-preview.pages.dev/demo/
Commit 3d80469

Comment thread apps/geolibre-desktop/src/components/layout/SettingsDialog.tsx
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Code review

Bugs: None found. The min-w-0 additions on the outer grid and the content pane correctly address the classic CSS grid-item overflow issue (grid items default to min-width: auto, sized to their intrinsic content, which can force the parent dialog wider than the viewport). The overflow-x-auto / md:overflow-x-visible pairing on the nav is sound: Button already applies whitespace-nowrap, so nav items won't wrap and will correctly trigger horizontal scrolling instead of clipping or forcing page-level overflow on mobile, while desktop keeps its original fixed-column, non-scrolling layout. (Confidence: high)

Security: No concerns — purely presentational Tailwind class changes, no new user input handling. (Confidence: high)

Performance: No concerns — no new re-renders, effects, or layout thrashing introduced. (Confidence: high)

Quality: Minor UX nit posted inline — the newly-scrollable mobile nav has no visual affordance (fade/arrow) indicating additional off-screen section buttons; existing focus-driven auto-scroll mitigates this for section switches but not for initial discovery. Non-blocking suggestion. (Confidence: low)

CLAUDE.md: Change correctly preserves the existing logical-property convention (border-e) for RTL support and doesn't introduce any physical-direction utilities; no other applicable guidelines (i18n strings, plugin catalogs, lockfiles) are touched by this change. (Confidence: high)

Overall: a small, well-scoped, correct CSS fix for the reported mobile overflow issue.

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

🔍 GitHub Pages PR preview

Item Value
Site https://opengeos.org/pages-preview/GeoLibre/pr-1613/
Demo app https://opengeos.org/pages-preview/GeoLibre/pr-1613/demo/
Commit 3d80469

@giswqs
giswqs merged commit 74acd61 into main Aug 1, 2026
23 checks passed
@giswqs
giswqs deleted the fix/issue-1605-settings-mobile-overflow branch August 1, 2026 14:10
@giswqs giswqs mentioned this pull request Aug 1, 2026
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: setting ui err

2 participants