feat(configurator): add Grid gap control + document responsive gap pattern - #629
Conversation
…ttern Add a "Grid gap" control to the Auto grid section of the Layout panel, wired to --sf-grid-gap. It offers the space-scale steps as a dropdown, a rem slider, and a raw-value input — matching the existing gap controls in the Spacing panel. Previously the section only exposed --sf-grid-min, so the grid gap had no dedicated UI. Document the responsive-gap pattern in docs/layout.md: gaps are fluid by default (via the shared space scale), and per-primitive mobile/desktop divergence is done with a @container override of the scoped gap token — no new tokens, consistent with the framework's container-driven model. No core/API changes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LeR8DfbqZqKtiFS72AJozP
|
Warning Review limit reached
Next review available in: 58 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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 (1)
📝 WalkthroughWalkthroughThe layout configurator now exposes ChangesGrid gap configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: 🚥 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 SummaryThis PR wires a new "Grid gap"
Confidence Score: 4/5Safe to merge — adds a UI-only control with no framework CSS changes and documentation that follows existing patterns. The configurator change is additive and correctly wired to --sf-grid-gap following the established SliderRow pattern. Two minor rough edges: the step granularity differs from all peer gap controls, and the existing Auto Grid preview ignores the new value (hardcoded gap-1 class), so users get no live feedback from the new slider in the preview area. The preview block in the Auto Grid section of LayoutPanel.svelte (lines 255-265) could benefit from wiring gridGap into the inline style. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["User moves Grid gap slider"] --> B["onChange fires"]
B --> C["onSet('--sf-grid-gap', value + 'rem')"]
C --> D["overrides record updated"]
D --> E["gridGap re-derived via parseRem"]
E --> F["SliderRow re-renders with new value"]
A2["User selects SPACE_SCALE option"] --> G["onRawSet fires"]
G --> C
A3["User clicks Reset"] --> H["onReset('--sf-grid-gap')"]
H --> I["override removed"]
I --> J["gridGap falls back to default 1rem display"]
D --> L["CSS var --sf-grid-gap applied to page"]
L --> M[".sf-grid / .sf-grid-flex gap updated live"]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A["User moves Grid gap slider"] --> B["onChange fires"]
B --> C["onSet('--sf-grid-gap', value + 'rem')"]
C --> D["overrides record updated"]
D --> E["gridGap re-derived via parseRem"]
E --> F["SliderRow re-renders with new value"]
A2["User selects SPACE_SCALE option"] --> G["onRawSet fires"]
G --> C
A3["User clicks Reset"] --> H["onReset('--sf-grid-gap')"]
H --> I["override removed"]
I --> J["gridGap falls back to default 1rem display"]
D --> L["CSS var --sf-grid-gap applied to page"]
L --> M[".sf-grid / .sf-grid-flex gap updated live"]
|
| </button> | ||
| {#if showAutoGrid} | ||
| <SliderRow | ||
| label="Grid gap" value={gridGap} min={0} max={4} step={0.0625} unit="rem" |
There was a problem hiding this comment.
Step granularity inconsistent with peer gap controls
The Grid gap slider uses step={0.0625} (1/16 rem ≈ 1 px/step), while every other gap-style control in this file — center gutter (step={0.125}), imposter margin (step={0.125}), alternate inner gap (step={0.125}) — uses step={0.125} (2 px/step). If this finer resolution is intentional it should be called out in a comment; otherwise aligning it to 0.125 keeps the UX consistent.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
The finer step is intentional, and I've added a code comment explaining it (e799111). --sf-grid-gap is a gap token that defaults to --sf-gap, so I mirrored the gap-token sliders in SpacingPanel (--sf-gap / --sf-content-gap / --sf-gutter), which all use step={0.0625}. The peer controls you cite (center gutter, imposter margin, alternate inner gap) aren't the closest analog here — tuning the grid gap more coarsely than the very token it inherits from would be the odd result. Keeping 0.0625 for consistency with the gap family rather than aligning to the panel's non-gap size controls.
Generated by Claude Code
Address review feedback: document why the Grid gap slider uses a finer step (0.0625rem) than the other size controls in LayoutPanel. It matches the gap-token sliders in SpacingPanel (--sf-gap / --sf-content-gap / --sf-gutter), which --sf-grid-gap defaults to, so the grid gap isn't tuned more coarsely than the token it inherits from. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LeR8DfbqZqKtiFS72AJozP
Address review feedback: the Auto grid preview used a hardcoded gap-1 class, so the new Grid gap slider produced no visible change. Drive the preview's gap from gridGap, scaled the same way as the column widths, so the control is self-demonstrating and consistent with the live gridMin preview. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LeR8DfbqZqKtiFS72AJozP
Add a "Grid gap" control to the Auto grid section of the Layout panel,
wired to --sf-grid-gap. It offers the space-scale steps as a dropdown,
a rem slider, and a raw-value input — matching the existing gap controls
in the Spacing panel. Previously the section only exposed --sf-grid-min,
so the grid gap had no dedicated UI.
Document the responsive-gap pattern in docs/layout.md: gaps are fluid by
default (via the shared space scale), and per-primitive mobile/desktop
divergence is done with a @container override of the scoped gap token —
no new tokens, consistent with the framework's container-driven model.
No core/API changes.
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01LeR8DfbqZqKtiFS72AJozP
Summary by CodeRabbit
New Features
Documentation