Derive and inject scale-based tokens for radius / border / motion in preview and saved overrides - #109
Conversation
|
Warning Review limit reached
Next review available in: 53 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 (3)
📝 WalkthroughWalkthroughAdds derived override computation in TypeScript and PHP for radius, border width, and motion scales, then uses those derived values in preview CSS and saved CSS output. It also updates several admin-app components for non-reactive snapshots, input focus, draft state, and dialog/button attributes. ChangesDerived Override Token Expansion
Snapshot, focus, and dialog attributes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
PR Summary by QodoDerive and inject scale-based tokens for radius/border/motion in preview + saved CSS
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
SLASHED-for-WP/admin-app/src/components/shell/PreviewPanel.svelte (1)
26-29: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
withDerivedOverridesduplicates the merge already ininjectLivePreview.This helper repeats the exact merge logic from
injectLivePreview(persistence.tsLine 220). Consider exporting a singlewithDerivedOverridesfrompersistence.tsand importing it here so the merge precedence stays defined in one place.🤖 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 `@SLASHED-for-WP/admin-app/src/components/shell/PreviewPanel.svelte` around lines 26 - 29, The merge logic in withDerivedOverrides duplicates the same override precedence already implemented in injectLivePreview, so keep the behavior in one place. Move or export a single withDerivedOverrides helper from persistence.ts and reuse it in PreviewPanel.svelte, ensuring injectLivePreview and PreviewPanel both call the shared symbol so derived values continue to be overridden by explicit values consistently.SLASHED-for-WP/admin-app/src/lib/persistence.ts (1)
92-162: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the TS and PHP derived-token tables in sync.
computeDerivedOverridesandSlashed_CSS_Generator::compute_derived_overridesduplicate the same radius/border/motion tables and special-case tokens; add a small parity check and cross-links so preview CSS and persisted CSS can’t drift.🤖 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 `@SLASHED-for-WP/admin-app/src/lib/persistence.ts` around lines 92 - 162, Keep the derived-token logic in `computeDerivedOverrides` aligned with `Slashed_CSS_Generator::compute_derived_overrides` by adding a small parity check over the shared radius, border, and motion token tables plus the special-case tokens like `--sf-radius-none`, `--sf-radius-full`, `--sf-radius-pill`, `--sf-radius-outer`, and the motion duration/delay entries. Wire the check into the existing TS persistence path so any table drift is detected early, and add cross-links or shared constants where possible so both preview CSS and persisted CSS are generated from the same source of truth. Use the unique symbols in this module and the PHP generator to locate the duplicated mappings and keep them synchronized.
🤖 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.
Nitpick comments:
In `@SLASHED-for-WP/admin-app/src/components/shell/PreviewPanel.svelte`:
- Around line 26-29: The merge logic in withDerivedOverrides duplicates the same
override precedence already implemented in injectLivePreview, so keep the
behavior in one place. Move or export a single withDerivedOverrides helper from
persistence.ts and reuse it in PreviewPanel.svelte, ensuring injectLivePreview
and PreviewPanel both call the shared symbol so derived values continue to be
overridden by explicit values consistently.
In `@SLASHED-for-WP/admin-app/src/lib/persistence.ts`:
- Around line 92-162: Keep the derived-token logic in `computeDerivedOverrides`
aligned with `Slashed_CSS_Generator::compute_derived_overrides` by adding a
small parity check over the shared radius, border, and motion token tables plus
the special-case tokens like `--sf-radius-none`, `--sf-radius-full`,
`--sf-radius-pill`, `--sf-radius-outer`, and the motion duration/delay entries.
Wire the check into the existing TS persistence path so any table drift is
detected early, and add cross-links or shared constants where possible so both
preview CSS and persisted CSS are generated from the same source of truth. Use
the unique symbols in this module and the PHP generator to locate the duplicated
mappings and keep them synchronized.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 43b2bae8-9e1d-4864-b985-3553abf784a4
📒 Files selected for processing (4)
SLASHED-for-WP/admin-app/src/components/shell/PreviewPanel.svelteSLASHED-for-WP/admin-app/src/lib/persistence.tsSLASHED-for-WP/assets/admin-app/app.jsSLASHED-for-WP/includes/class-css-generator.php
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
…atting - --sf-radius-pill now emits var(--sf-radius-full) instead of 9999px, and --sf-radius-outer emits calc(var(--sf-radius-m) + var(--sf-component-pad)) so fine-tuning --sf-radius-full or --sf-radius-m propagates correctly - Skip derived-token emission in both TS and PHP when a scale value is non-numeric (e.g. var()/calc()), preventing silent override of framework runtime formulas; JS fmt() now guards against non-finite values - Expand inline PHP associative arrays to multi-line and fix assignment alignment to satisfy WordPress coding standards (phpcs) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MhTeTwHB6FWbkRuGmSc1yU
Merged origin/main (PR #110) into the branch. The only conflict was the generated app.js bundle; resolved by rebuilding from the merged source. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MhTeTwHB6FWbkRuGmSc1yU
- Delete num_or_default() helper that became unused after refactoring compute_derived_overrides() to use inline is_numeric() guards directly - Fix equals-sign alignment for $scale/$r_steps assignment block to satisfy WordPress phpcs alignment sniff Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MhTeTwHB6FWbkRuGmSc1yU
2 participants
Motivation
Radius scaleknob had no visible effect in the live preview or emitted CSS, indicating the high‑level scale knobs were not being expanded into concrete output tokens in all places where--sf-text-*and--sf-space-*were already handled.Description
computeDerivedOverrides(ov)toadmin-app/src/lib/persistence.tswhich precomputes derived tokens for--sf-radius-*,--sf-border-width-*, and--sf-duration-*(in addition to text/space derived tokens) and exported it for reuse in the preview code.computeDerivedOverridesininjectLivePreviewso:rootstyle injected assf-parent-overridescontains concrete output tokens that win over @layer declarations.admin-app/src/components/shell/PreviewPanel.svelteto applywithDerivedOverrides()(wrappingcomputeDerivedOverrides) when writingslashed-overridesinto preview iframes so single‑iframe and split light/dark previews reflect scale knobs immediately.SLASHED-for-WP/includes/class-css-generator.phpwithcompute_derived_overrides()and merged derived declarations before emitting saved override declarations so persisted frontend inline CSS contains concrete radius/border/motion tokens (while preserving user fine‑tune overrides precedence).assets/admin-app/app.js) so the SPA uses the new derived-token logic in the shipped preview.Testing
npm run checkinadmin-app(Svelte diagnostics) which completed with 0 errors and the existing warnings retained; the command succeeded.npm run build --ignore-scripts(Vite production build) and verified the bundle was produced; the build passed with existing warnings (chunk-size / a11y hints) and the generatedassets/admin-app/app.jswas updated.php -l includes/class-css-generator.phpwhich reported no syntax errors.npm run build(including theprebuildfetch sync) failed in this environment becausescripts/sync-core.mjs’s network fetch failed; the production build was validated by running Vite with--ignore-scriptsas above.Codex Task
Summary by CodeRabbit