Repository navigation
Let users dim or hide the background dots - #245
Conversation
A "Background dots" slider under Map layers sets how strongly the dot grid draws behind the map, the library, and exported screenshots and videos. 100% is today's look; 0% hides the grid. The value is a new AppPreferences field (index 19) that defaults to 1.0, so preferences saved by older builds load unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 39 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (18)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughApp preferences now include a persisted background dot opacity setting. Map and screenshot dot grids use the setting, video exports pass it to screenshots, and library backups serialize and restore it. ChangesBackground Dot Opacity
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant ExportVideoDialog
participant AppPreferencesProvider
participant VideoExporter
participant ScreenshotView
participant DotGrid
ExportVideoDialog->>AppPreferencesProvider: Read backgroundDotOpacity
ExportVideoDialog->>VideoExporter: Pass backgroundDotOpacity
VideoExporter->>ScreenshotView: Pass backgroundDotOpacity
ScreenshotView->>DotGrid: Pass opacity
Merge Risk: ⚪ Minimal · up to No merge-blocking issue is identified; the change is ready to merge after normal checks. Security Architecture ReviewSecurity architecture risk: ⚪ Minimal · up to The new setting controls cosmetic background dots. The reviewed changes preserve older saved preferences, validate imported values and do not introduce new access permissions or sensitive-data exposure. No material security risk was identified in this change. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 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-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@greptileai review |
| void _handleGlobalPointer(PointerEvent event) { | ||
| if (event.kind != PointerDeviceKind.mouse) return; | ||
| // A hidden grid has no glow to animate. | ||
| if (ref.read(appPreferencesProvider).backgroundDotOpacity <= 0) return; |
There was a problem hiding this comment.
If the pointer leaves the library grid while Background dots is at 0%, this return skips the exit handling. The grid stays mounted, so restoring the dots shows a glow at the pointer’s old position until another pointer event updates it. Clear the hover when hiding the grid, or continue handling exits while it is hidden.
Artifacts
Authored Flutter widget reproduction test
- The test mounts each grid implementation, changes opacity around a real mouse exit, and inspects the restored painter’s hover and position.
Parent-commit grid implementation
- This source was extracted from `HEAD^` and supplies the pre-change widget for the comparison run.
Baseline widget-test output without the guard
- The Flutter test ran the parent-commit widget and observed restored hover 0.0 after the mouse exited while hidden.
Current widget-test output with the guard
- The Flutter test ran the PR widget and observed restored hover 1.0 at the stale (80,80) position after the mouse exited while hidden.
Combined Flutter test execution record
- The captured command, working directory, exit code, and output show both comparison tests passing with different restored hover values.
There was a problem hiding this comment.
Fixed in 01f8056. While the opacity is 0, pointer events now reset the glow and its target to 0 instead of returning early, so turning the dots back on starts with no glow. test/hover_dot_grid_glow_test.dart covers it: hover until the glow is up, hide the dots, leave the grid, show them again, and expect a glow of 0. Without the fix it reads 1.0. (Correction to my first reply: the shader does load in flutter_tester.)
Main restructured screenshot and video capture (page_screenshot.dart) and brought library-backup globals. The dot opacity now reaches the new capture paths explicitly, and library backups carry it next to the other map-layer toggles; backups without the key keep the current value. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
An additive field whose default reproduces the old behavior loads from old records without one, as AppPreferences fields 17-19 do. The rule now asks for a migration only when old data would not load correctly, and for a test that proves the additive case. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Moving the mouse off a hidden grid used to leave the glow target set, so turning the dots back on showed a glow at the old pointer spot until the next move. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@greptileai review |
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
| for (var i = 0; i < 10; i++) { | ||
| await tester.runAsync( | ||
| () => Future<void>.delayed(const Duration(milliseconds: 50))); | ||
| await tester.pump(); | ||
| } |
There was a problem hiding this comment.
If the shader takes more than 500 ms to load on a slower runner, these ten delays finish before the grid creates its CustomPaint. The subsequent glow() lookup throws, failing the regression test even when the hover behavior is correct. Wait for the painted grid to appear before checking its glow. This is a non-blocking test reliability issue.
Artifacts
Shader-wait reproduction command
- This executed script temporarily delays shader loading, runs the widget test in each condition, and restores the tracked sources.
- The original widget test ran without an injected delay and exited successfully.
Fixed wait fails with a delayed shader
- The original widget test ran with a 750 ms shader delay and exited with `Bad state: No element` at `glow()`.
▶ Chromium capture of the fixed-wait test failure
- Chromium displayed the captured failing widget-test output, emphasizing the missing `CustomPaint` lookup.
Poster showing the fixed-wait failure
- A frame from the Chromium output capture shows the original test's failure under delayed shader loading.
Readiness-wait test passes with the same delay
- The widget test ran with the same 750 ms shader delay and a bounded `CustomPaint` readiness wait, then exited successfully.
▶ Chromium capture of the readiness-wait test passing
- Chromium displayed the captured passing widget-test output under the same delayed-load condition.
Poster showing the readiness-wait pass
- A frame from the Chromium output capture shows the test passing after waiting for `CustomPaint`.
Chromium evidence-capture script
- This executed Playwright script rendered the actual test logs in Chromium and recorded the paired videos and posters.
1 participant
A user asked for a way to turn off the dot grid behind the map (THINKII's feedback post, Sept 28). Settings → Map layers now has a Background dots slider. 100% is today's look and 0% hides the grid.
The dots are faint even at 100%, so the crop above is zoomed 3×. At normal size the difference is subtle: the canvas goes from lightly textured to flat.
What follows the setting
DotGrid).HoverDotGrid). The cursor glow scales too, so 0% leaves no dots under the mouse either.page_screenshot.dart,VideoExporter). Their offscreen container doesn't read Hive, so the value is passed in explicitly, the same way spawn barriers and region names already are.Reproduction
On main, Settings has no control for the dots. They always draw at a fixed alpha of 0.7 on the border color.
Data
AppPreferencesgetsbackgroundDotOpacityat Hive index 19 (generated,nextIndex20), defaulting to 1.0. Preferences written by older builds have no field 19, so they read as 1.0 and the app looks unchanged. A test writes a record the way a pre-change build did and reads it back.Library backups (from #234) already carry the other Map-layers toggles, so they now carry
backgroundDotOpacitytoo, as an optional key inglobals.appPreferences. A backup without the key, meaning every backup made before this PR, keeps the current value on import. The library-backup round-trip test checks both cases. .ica files don't hold preferences, so they don't change.Verification
flutter analyze lib test: no new issues.flutter testafter merging main: 1581 passed. New tests cover old preferences loading at full opacity, persistence and clamping, the grid painting nothing at 0%, a hidden library grid not animating on mouse movement, and screenshots honoring the value they're given.No migration, on purpose. This field is additive and the adapter default (1.0) is the old look, so no stored value needs rewriting, the same as fields 17 and 18. AGENTS.md now says this directly: a migration is needed when old data would not load correctly, and an additive field with an old-behavior default needs a test instead.
🤖 Generated with Claude Code
Summary by CodeRabbit
The confirmed test reliability issue is non-blocking; no outstanding finding makes the product change unsafe to merge.
Findings
Summary
The PR adds a persisted Background dots setting across the editor, library, screenshots, video exports, and backups. The new glow regression test can fail on slower runners when its fixed wait ends before the shader loads.
Reviews (7) · Last reviewed commit: "Test that hidden dots come back without ..."