Redesign settings and unify map theming - #123
Conversation
Settings dialog: responsive sizing, scale-based type hierarchy (no bold/uppercase emphasis), semantic icon accents, InkWell-backed nav with reliable hover and click cursors, and consistent vertical rhythm. Map theming now lives in one place: a single profile list under Current strategy with inline management, plus a live-preview editor dialog (real map + dot grid re-rendered as colors change). Strategy overrides are labeled "This strategy only", promotable to profiles, and guarded by a confirm before being discarded. Built-in profiles are tagged. The duplicated SVG color mapper is extracted and shared by the canvas, screenshot exporter, and preview.
|
@greptileai review |
|
Warning Review limit reached
Next review available in: 32 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 (4)
📝 WalkthroughWalkthroughThe change adds shared map palette mapping and a profile editor. It integrates profile management into the settings UI, removes embedded editors, and revises settings layout, navigation, colors, controls, and card descriptions. ChangesMap theme and settings experience
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SettingsTab
participant MapThemeSettingsSection
participant MapThemeEditorDialog
participant MapThemeProvider
participant StrategyProvider
SettingsTab->>MapThemeSettingsSection: render map theme settings
MapThemeSettingsSection->>MapThemeEditorDialog: open with selected mode and palette
MapThemeEditorDialog->>MapThemeProvider: create or update profile
MapThemeEditorDialog->>StrategyProvider: apply strategy palette override
MapThemeEditorDialog-->>MapThemeSettingsSection: close and report outcome
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Greptile SummaryThe settings redesign centralizes map-theme profile management, strategy-specific palette customization, and shared SVG palette rendering. Focused persistence coverage confirmed that profile selections and per-strategy overrides restore correctly, switching back to a profile clears the override, and no strategy record is created when no strategy is open. Confidence Score: 5/5No blocking failure remains. No accepted blocking findings remain after exercising the map-theme persistence and restoration paths.
 T-Rex LogsWhat T-Rex did
Reviews (3): Last reviewed commit: "Make profile write results honest" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
lib/widgets/map_svg_color_mapper.dart (1)
13-41: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAdd value equality to
MapSvgColorMapper.
flutter_svgincludes theColorMapperin its cache key, so eachbuild()-time instance misses cached SVGs even whenreplacementsis the same. Implement==andhashCodeoverreplacementsso identical palette mappers reuse cached parsed SVGs, especially during preview color-picker rebuilds.🤖 Prompt for 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. In `@lib/widgets/map_svg_color_mapper.dart` around lines 13 - 41, Implement value-based == and hashCode on MapSvgColorMapper using its replacements map, so instances with identical mappings compare equal and produce the same hash. Preserve the existing substitute behavior and constructor APIs.
🤖 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 `@lib/widgets/dialogs/map_theme_editor_dialog.dart`:
- Around line 397-417: The _saveProfileEdits flow currently reports success even
when renameProfile or updateProfilePalette rejects invalid input. Update these
provider methods to return an explicit success/failure result or throw for
invalid names, then have _saveProfileEdits detect a rejected name and avoid
popping or showing “Profile updated.”; retain the success flow when the palette
update succeeds and no name change is requested.
In `@lib/widgets/map_theme_settings_section.dart`:
- Around line 63-69: Update the override “Custom” row in the map theme settings
section to pass onTap: null instead of a no-op callback. In
_ProfileListRow.build, reserve and render the check column based on isSelected
rather than onTap, while keeping the selected check mark and row alignment
unchanged.
In `@lib/widgets/settings_tab.dart`:
- Around line 69-87: Clamp dialogWidth and dialogHeight in SettingsTab to
positive minimums exceeding the 208 px navigation rail plus 48 px padding,
preserving the existing maximum constraints. Apply the same positive-floor
clamping in lib/widgets/dialogs/map_theme_editor_dialog.dart at lines 96-116,
using a minimum above the 340 px control pane or making that pane flexible.
---
Nitpick comments:
In `@lib/widgets/map_svg_color_mapper.dart`:
- Around line 13-41: Implement value-based == and hashCode on MapSvgColorMapper
using its replacements map, so instances with identical mappings compare equal
and produce the same hash. Preserve the existing substitute behavior and
constructor APIs.
🪄 Autofix
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: 706f4d36-aa57-4738-92a1-4ee06ef3a507
📒 Files selected for processing (8)
lib/const/settings.dartlib/interactive_map.dartlib/screenshot/screenshot_view.dartlib/widgets/dialogs/map_theme_editor_dialog.dartlib/widgets/map_svg_color_mapper.dartlib/widgets/map_theme_settings_section.dartlib/widgets/settings_scope_card.dartlib/widgets/settings_tab.dart
…lidation - Clamp both dialogs' responsive dimensions to positive floors so tiny windows can't produce negative pane widths - The override "Custom" row is no longer fake-tappable; the check column now follows selection instead of tappability - Editing a profile with an empty name shows an error instead of a false "Profile updated" toast
|
@greptileai review — pushed fixes for the size-clamping, override-row semantics, and rename validation; replied on the gradient thread with the canvas-fidelity rationale. |
renameProfile and updateProfilePalette now return whether anything was written instead of silently no-oping, and both callers surface the failed outcome (including the partial colors-saved-name-rejected case) instead of toasting a false success.
|
@greptileai review |
1 participant
What
A ground-up pass on the settings dialog plus a consolidation of all map-theme workflows into one place with a live-preview editor.
Settings dialog
Settings.settings*Accent) — the violet action hue stays reserved for commands/selectionInkWell(transparencyMaterial) — reliable hover, pressed states, and pointer cursors; no more stuck highlights from hand-rolledMouseRegionstateMap theming, unified
ColorMapperfrominteractive_map.dartandscreenshot_view.dartintomap_svg_color_mapper.dart, shared by canvas, exporter, and previewData safety
No Hive schema, migration, or serialization changes. Profiles/overrides persist exactly as before;
.icaround-trip untouched.Testing
flutter analyzeclean (one pre-existing deprecation inpages_bar.dart)cursor_glyph_debug,update_checker,windows_desktop_update_controller), none relatedSummary by CodeRabbit
New Features
Improvements