ECHOES-1446 [NO-JIRA] Apply Option 2 color migration - #745
ECHOES-1446 [NO-JIRA] Apply Option 2 color migration#745marciopmoreira6 wants to merge 4 commits into
Conversation
✅ Deploy Preview for echoes-react ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
| '--table-header-background-color': cssVar('table-colors-background-header-default'), | ||
| border: `${cssVar('border-width-default')} solid ${cssVar('color-border-bold')}`, | ||
| 'border-collapse': 'separate', | ||
| border: `${cssVar('border-width-default')} solid ${cssVar('issue-row-colors-border')}`, |
There was a problem hiding this comment.
💡 Quality: Generic Table styling bound to issue-row-specific tokens
In TableStyles.tsx, generic table primitives (StyledTable border, cellBaseStyle border-top, and StyledTableCell backgrounds) were migrated to issue-row-colors-* tokens (border, background-default/hover/selected/selected-hover). The Table component is generic, so coupling it to an issue-row semantic concept is a token-semantics mismatch: any non-issue table now inherits issue-row coloring and cannot diverge. Consider a generic component token (e.g. a table-colors-* / layer-3 table token) that maps to the same value, so the semantic name matches the component. Flagging as a designer+developer checkpoint rather than a blocker.
Was this helpful? React with 👍 / 👎
| background-color: ${cssVar('color-icon-on-color')}; | ||
| border: 0.25rem solid ${cssVar('color-border-accent-default')}; | ||
| background-color: ${cssVar('select-colors-indicator-foreground-selected')}; | ||
| border: 0.25rem solid ${cssVar('select-colors-indicator-border-selected')}; |
There was a problem hiding this comment.
💡 Quality: Hardcoded 0.25rem border width not routed through cssVar
The radio indicator inner border uses a raw 0.25rem value on a line modified in this diff, while the color was migrated to a token. Per the design-token guidance all dimension values should come from a token via cssVar. This pre-existed but is now on a touched line; route the width through the appropriate dimension token (with designer alignment on the exact token).
Was this helpful? React with 👍 / 👎
|
|
||
| <Table | ||
| ariaLabel="Issues on new code" | ||
| gridTemplate="max-content minmax(280px, 1fr) minmax(180px, 0.6fr) max-content max-content" |
There was a problem hiding this comment.
💡 Quality: Hardcoded pixel values in Issues story gridTemplate
The Issues playground story passes raw pixel values (minmax(280px, 1fr), minmax(180px, 0.6fr)) in the gridTemplate prop instead of design tokens. Story files are lower-stakes, but per the styling guidance layout dimensions should come from cssVar dimension tokens. Consider composing the template from dimension tokens for consistency with the rest of the pilot stories.
Was this helpful? React with 👍 / 👎
| "action": { | ||
| "neutral": { | ||
| "default": { | ||
| "$type": "color", | ||
| "$value": "{echoes.color.palette.ui.gray.12}", | ||
| "$description": "Highest-contrast neutral fill for primary and routine actions." | ||
| }, | ||
| "hover": { | ||
| "$type": "color", | ||
| "$value": "{echoes.color.palette.ui.gray.11}", | ||
| "$description": "Hover fill for primary and routine neutral actions." | ||
| }, | ||
| "focus": { | ||
| "$type": "color", | ||
| "$value": "{echoes.color.palette.ui.gray.12}", |
There was a problem hiding this comment.
💡 Quality: Many new semantic/component tokens need designer+dev checkpoint
This PR introduces a large set of new semantic and component tokens (e.g. background.action.neutral.*, button-colors-*, select-colors-*, input-colors-*, navigation-item-colors-*, issue-row-colors-*, link-colors-*) and repurposes deprecated accent aliases as neutral mappings. All referenced tokens were verified present in both generated light and dark CSS, so this is not a breakage — but per the token-review policy these new contracts require the normal designer + developer checkpoint before rollout. Confirm token names, semantics, and WCAG contrast are signed off.
Was this helpful? React with 👍 / 👎
Code Review 👍 Approved with suggestions 0 resolved / 4 findingsApplies the Option 2 color migration by reorganizing palettes into brand token sets and updating component styling. Consider addressing minor findings such as the generic Table styling bound to issue-row-specific tokens, hardcoded border width and pixel values, and scheduling the required designer/developer token checkpoint. 💡 Quality: Generic Table styling bound to issue-row-specific tokens📄 src/components/table/TableStyles.tsx:30 📄 src/components/table/TableStyles.tsx:90 📄 src/components/table/TableStyles.tsx:114 📄 src/components/table/TableStyles.tsx:117 📄 src/components/table/TableStyles.tsx:121 📄 src/components/table/TableStyles.tsx:125 In TableStyles.tsx, generic table primitives (StyledTable border, cellBaseStyle border-top, and StyledTableCell backgrounds) were migrated to 💡 Quality: Hardcoded 0.25rem border width not routed through cssVar📄 src/components/select/SelectItemCommons.tsx:185 The radio indicator inner border uses a raw 💡 Quality: Hardcoded pixel values in Issues story gridTemplate📄 stories/playground/Issues-stories.tsx:101 The Issues playground story passes raw pixel values ( 💡 Quality: Many new semantic/component tokens need designer+dev checkpoint📄 design-tokens/tokens/modes/light.json:36-50 📄 design-tokens/tokens/modes/dark.json:40-54 📄 src/components/buttons/ButtonStyles.tsx:113-127 This PR introduces a large set of new semantic and component tokens (e.g. 🤖 Prompt for agentsOptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |




Part of
Summary
Architecture
Preserves the intended token direction:
Brand primitives → light/dark semantics → component tokens → Echoes componentsNo
echoes.color.roleslayer is introduced, and migrated components do not consume primitive palette tokens directly.Validation
Review notes
DESIGN.mddiffers: dark Blue 9 is#73A7EC, light inverse neutral is#FAFBFC, and dark Gray 12 is#F4F6F8.Full implementation report:
docs/new-design-language-color-pilot-summary.md