feat: batch 10 — shadow safety + coverage GAPs + kpi-tile variant UX#21
Merged
simonsangla merged 1 commit intomainfrom Apr 16, 2026
Merged
feat: batch 10 — shadow safety + coverage GAPs + kpi-tile variant UX#21simonsangla merged 1 commit intomainfrom
simonsangla merged 1 commit intomainfrom
Conversation
Closes the inspector findings from Batch 9: - 1 P1 (CSS injection via unguarded shadow strings) - GAP #1 (T-129 integration tests) - GAP #2 (export emission tests for new tokens) - 1 P2 (kpi-tile metric variant lacks user-facing affordance) T-130 — cssShadow validator tightened. Rejects `;` `}` `{` `/*` `*/` newlines `<` `>`. These are the characters sufficient to break out of a `:root { --shadow-x: VALUE; }` declaration in CSS exports or `$shadow-x: VALUE;` in SCSS. Multi-layer rgba/hsla still works because parens and commas are NOT in the blocklist. Cavekit-schema R8 revised to mandate this contract explicitly. T-131 — tests/export/newTokens.test.ts asserts every export format (toCSSVars/Variant, toSCSSVars/Variant, toTailwindConfig/Variant, toStyleDictionary/Variant, toJSON/Variant, toTSObject/Variant) emits the 5 new color slots, 4 shadow slots, and 5 radius slots with the right shape (kebab-case CSS, $-prefixed SCSS, theme.extend.boxShadow + .borderRadius for Tailwind, type:'boxShadow'/'dimension' for Style Dictionary). T-132 — tests/persistence/integration.test.tsx covers: - save → load → toJSON round-trip preserves shadows + radii (custom values + every preset) - themeToStyleVars output is a complete superset of every CSS var that WidgetPreview reads - A wrapping div with the preset's CSS vars actually carries those values to a child WidgetPreview (proves the inheritance chain end-to-end) T-133 — same file, 8 malicious shadow payloads (semicolon escape, brace open/close, comment open/close, newline, angle brackets) all rejected as corrupt by loadTheme. Benign multi-layer rgba shadow still loads as ok. T-134 — kpi-tile gets an in-card Tile/Metric pill toggle. Cavekit-widgets R4 revised to specify: (a) only kpi-tile exposes per-widget config UI, (b) variant choice is transient per-session (not persisted, not in any export), (c) toggle uses aria-pressed for both states. Implementation keeps the variant in WidgetSelector local state and stops event propagation so toggle clicks don't flip the parent switch. T-135 — 5 new behavior tests on WidgetSelector covering toggle rendering, default state, click-flips-aria-pressed, switch-isolation, and single-widget-only scope (exactly one variant-toggle row in the entire selector — no other widget gains config UI). Validation: lint 0 errors, typecheck pass, test 261/261 (220 prior + 41 new), build pass (vite ~107ms). Cavekit revisions: - cavekit-schema R8: explicit character blocklist for shadow values - cavekit-widgets R4: kpi-tile metric variant scope clarified Build site: context/plans/build-site-batch10.md (6 tasks, 3 tiers, 13/13 criteria covered).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements Batch 10 changes, focusing on security hardening for shadow tokens and enhancing the kpi-tile widget. Key updates include a character blocklist in the cssShadow validator to prevent CSS injection, comprehensive export emission tests for new tokens, and a transient variant toggle for the kpi-tile preview. Feedback indicates that the use of e.stopPropagation() in the WidgetSelector component is redundant because the variant buttons are siblings to the main toggle rather than nested within it.
Comment on lines
+111
to
+115
| onClick={(e) => { | ||
| // Stop propagation so this doesn't toggle the parent switch | ||
| e.stopPropagation() | ||
| setKpiVariant(v) | ||
| }} |
There was a problem hiding this comment.
The use of e.stopPropagation() here is redundant and potentially misleading. Since the variant buttons are siblings of the main cardBtn (the widget selection toggle) and not nested within it, clicking a variant button will not trigger the cardBtn's onClick handler. The DOM structure already provides the event isolation mentioned in the implementation notes.
Suggested change
| onClick={(e) => { | |
| // Stop propagation so this doesn't toggle the parent switch | |
| e.stopPropagation() | |
| setKpiVariant(v) | |
| }} | |
| onClick={() => { | |
| setKpiVariant(v) | |
| }} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the inspector findings from Batch 9 (PR #20): 1 P1 (CSS injection via unguarded shadow strings), 2 GAPs (export emission tests + integration tests), and 1 P2 (kpi-tile metric variant lacked user-facing affordance). 6 tasks T-130..T-135 in 3 tiers, 13/13 batch-10 criteria covered.
Tasks
cssShadowvalidator now rejects;}{/**/newlines<>. Multi-layer rgba/hsla still works (parens + commas allowed). Cavekit-schema R8 revised to mandate this contract.tests/export/newTokens.test.tsasserts every export format emits the 5 new color slots + 4 shadows + 5 radii with the right shape (kebab-case CSS,$-prefixed SCSS,theme.extend.boxShadow/borderRadiusfor Tailwind,type:'boxShadow'/'dimension'for Style Dictionary).tests/persistence/integration.test.tsxcoverstoJSONround-trip preserving shadows + radii, every preset preserved through save/load,themeToStyleVarssuperset of preview-consumed vars, and the inheritance chain end-to-end.loadTheme. Benign multi-layer rgba shadow still loads.aria-pressed. Event propagation stopped so toggle clicks don't flip the parent switch.Cavekit revisions
cavekit-schema.mdR8 — explicit character blocklist for shadow valuescavekit-widgets.mdR4 — kpi-tile metric variant scope clarifiedValidation
Browser verification
Test plan
npm run lint— cleannpm run typecheck— cleannpm run test— 261/261npm run build— succeeds🤖 Generated with Claude Code