Enhancement [DEV-12682] Icon feedback and more#2690
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refines the new icon system by moving trend arrow variants to dedicated SVG assets (instead of transform-based variants), removing the per-variable SVG scale option, and adjusting inline SVG sizing/alignment behavior. It also includes minor dashboard UI consistency tweaks and a waffle-chart rendering improvement for denominator=10.
Changes:
- Reworked SVG registry + markup-variable SVG rendering (wrapper-based inline SVG markup, shared default scale, updated accessibility behavior).
- Updated TrendArrow to use registry SVGs (down/no-change as separate assets) and removed transform-based CSS variants; added unit tests.
- Improved waffle-chart “visible denominator” layout for denominator=10 and added coverage; small CSS spacing/radius adjustments.
Reviewed changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/waffle-chart/src/test/CdcWaffleChart.test.jsx | Refactors test config setup; adds test for denominator=10 grid layout. |
| packages/waffle-chart/src/scss/waffle-chart.scss | Adjusts callout spacing (gap). |
| packages/waffle-chart/src/CdcWaffleChart.tsx | Adds dynamic grid helper; special-cases denominator=10 to 5x2 grid. |
| packages/markup-include/src/_stories/MarkupInclude.stories.tsx | Adds Storybook interaction test for new SVG sizing behavior. |
| packages/core/types/MarkupVariable.ts | Removes svgScale from the public MarkupVariable type. |
| packages/core/styles/_global.scss | Adds CSS var overrides to allow “parent-height” icon sizing via data-cove-svg-size. |
| packages/core/helpers/tests/markupProcessor.test.ts | Updates/extends tests for new icon markup, accessibility flags, and transform removal. |
| packages/core/helpers/svgRegistry.ts | Adds new arrow assets, removes transform-based variants, and changes inline SVG markup generation. |
| packages/core/helpers/markupProcessor.ts | Removes svgScale usage/validation; uses shared inline SVG generation. |
| packages/core/components/ui/TrendArrow.tsx | Switches to SVG registry assets and parses raw SVG into React. |
| packages/core/components/ui/TrendArrow.test.tsx | Adds unit tests for TrendArrow’s new SVG asset behavior and accessibility behavior. |
| packages/core/components/ui/trend-arrow.css | Removes transform rules for down/no-change arrows (now separate assets). |
| packages/core/components/NestedDropdown/nesteddropdown.styles.css | Aligns border-radius with other controls (0.333rem). |
| packages/core/components/MultiSelect/multiselect.styles.css | Aligns border-radius with other controls (0.333rem). |
| packages/core/components/EditorPanel/components/MarkupVariablesEditor.tsx | Removes Icon Scale field and related validation. |
| packages/core/components/EditorPanel/components/MarkupVariablesEditor.test.tsx | Updates expectations to reflect removal of Icon Scale field. |
| packages/core/assets/user-icons/arrow-right.svg | Adds right arrow SVG for “no change”. |
| packages/core/assets/user-icons/arrow-down.svg | Adds down arrow SVG for “down”. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 22 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 25 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Addresses some feedback around new icon system:
Also has a few minor dashboard consistency improvements for the OPDC dashboard.
Testing Steps
Test the new Icon mode for markup variables, make sure they still work as expected.