feat(css-utilities, design-tokens): DLT-3330 DLT-3332 add off-scale pixel-indexed layout tokens and sizing utilities#1206
Conversation
|
Please add either the |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Cache: Disabled due to Reviews > Disable Cache setting Disabled knowledge base sources:
Adds five off-scale pixel-indexed layout tokens (1px, 2px, 8px, 20px, 24px), 35 hyphenated sizing utilities across seven sizing properties, docs updates, migration/lint rule extensions, and comprehensive tests; includes an unrelated dropdown checkmark removal. Overall Judgement: Rationale: Feature is well implemented and tested, but the unrelated UI change (control_selection.vue) should be split into a separate PR to keep scope focused. WalkthroughAdds five pixel-suffixed off-scale layout stops ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Francis Rupert (francisrupert)
added
the
no-visual-test
Francis Rupert (francisrupert)
requested review from
Brad Paugh (braddialpad),
Ignacio Ropolo (iropolo) and
Nina Repetto (ninarepetto)
as code owners
Francis Rupert (francisrupert)
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/dialtone-css/lib/build/js/dialtone_migration_helper/tests/size-to-layout-test-examples.vue (1)
126-129: Avoid duplicateblock-sizein the off-scale CSS sample.Line 129 overrides Line 126, so one sample is effectively shadowed. Consider using a distinct layout property for the 24px case to keep each mapping independently visible.
Proposed tweak
- block-size: var(--dt-size-550); /* → layout-24px (24px) */ + max-block-size: var(--dt-size-550); /* → layout-24px (24px) */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/dialtone-css/lib/build/js/dialtone_migration_helper/tests/size-to-layout-test-examples.vue` around lines 126 - 129, The CSS sample duplicates block-size (var(--dt-size-200) and var(--dt-size-550)), causing the earlier mapping to be shadowed; update the later occurrence (the var(--dt-size-550) 24px case) to a distinct layout property (for example inline-size, min-block-size, max-block-size, or max-inline-size) so each size-to-layout mapping remains independently visible—locate the two block-size entries in the test example and replace the second one with the chosen distinct property while keeping the comment mapping consistent.packages/dialtone-css/lib/build/js/dialtone_migration_helper/tests/utility-class-to-token-stops.test.mjs (1)
15-16: Add off-scale coverage ford-size*legacy selectors.Line 15 currently omits the
sizeprefix from the off-scale matrix. Since this PR introduces pixel-indexed size utilities too, addsizehere (or add an explicit non-support assertion) so regressions on that path are caught.Proposed test update
- const prefixes = ['h', 'w', 'hmn', 'hmx', 'wmn', 'wmx']; + const prefixes = ['h', 'w', 'size', 'hmn', 'hmx', 'wmn', 'wmx'];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/dialtone-css/lib/build/js/dialtone_migration_helper/tests/utility-class-to-token-stops.test.mjs` around lines 15 - 16, The test currently builds an off-scale matrix using the prefixes array but omits the legacy "size" prefix, so update the prefixes constant used in the test (prefixes = ['h','w','hmn','hmx','wmn','wmx']) to include "size" (i.e. add 'size' into the array) so the pixel-indexed size utilities are covered; alternatively, if you don't want to assert support for legacy d-size, add an explicit non-support assertion in the same test that documents and checks that "size" is intentionally unsupported.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@packages/dialtone-css/lib/build/js/dialtone_migration_helper/tests/size-to-layout-test-examples.vue`:
- Around line 126-129: The CSS sample duplicates block-size (var(--dt-size-200)
and var(--dt-size-550)), causing the earlier mapping to be shadowed; update the
later occurrence (the var(--dt-size-550) 24px case) to a distinct layout
property (for example inline-size, min-block-size, max-block-size, or
max-inline-size) so each size-to-layout mapping remains independently
visible—locate the two block-size entries in the test example and replace the
second one with the chosen distinct property while keeping the comment mapping
consistent.
In
`@packages/dialtone-css/lib/build/js/dialtone_migration_helper/tests/utility-class-to-token-stops.test.mjs`:
- Around line 15-16: The test currently builds an off-scale matrix using the
prefixes array but omits the legacy "size" prefix, so update the prefixes
constant used in the test (prefixes = ['h','w','hmn','hmx','wmn','wmx']) to
include "size" (i.e. add 'size' into the array) so the pixel-indexed size
utilities are covered; alternatively, if you don't want to assert support for
legacy d-size, add an explicit non-support assertion in the same test that
documents and checks that "size" is intentionally unsupported.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 90cac726-1e1b-4a99-a580-6e7d32d0e59b
📒 Files selected for processing (8)
.claude/skills/dt-migrate.mdpackages/dialtone-css/lib/build/js/dialtone_migration_helper/configs/size-to-layout.mjspackages/dialtone-css/lib/build/js/dialtone_migration_helper/configs/utility-class-to-token-stops.mjspackages/dialtone-css/lib/build/js/dialtone_migration_helper/tests/size-to-layout-test-examples.vuepackages/dialtone-css/lib/build/js/dialtone_migration_helper/tests/size-to-layout.test.mjspackages/dialtone-css/lib/build/js/dialtone_migration_helper/tests/utility-class-to-token-stops.test.mjspackages/eslint-plugin-dialtone/lib/rules/deprecated-pixel-utility-classes.jspackages/eslint-plugin-dialtone/tests/lib/rules/deprecated-pixel-utility-classes.js
✅ Files skipped from review due to trivial changes (3)
- packages/dialtone-css/lib/build/js/dialtone_migration_helper/configs/size-to-layout.mjs
- .claude/skills/dt-migrate.md
- packages/dialtone-css/lib/build/js/dialtone_migration_helper/configs/utility-class-to-token-stops.mjs
|
Thanks for the review! Finding 1 (duplicate Finding 2 (add |
|
✔️ Deploy previews ready! |
2 participants
🛠️ Type Of Change
📖 Jira Ticket
--dt-size-*references to off-scale layout tokens (bundled in this PR)📖 Description
Add off-scale pixel-indexed layout tokens (
--dt-layout-1px,--dt-layout-2px,--dt-layout-8px,--dt-layout-20px,--dt-layout-24px) alongside the scale-indexeddt-layout-25/50/…scale.Emit 35 new hyphenated sizing utility selectors —
.d-w-1px,.d-h-2px,.d-size-8px,.d-wmn-20px,.d-hmx-24px, etc. — across all 7 sizing properties: height, max-height, min-height, size, width, max-width, min-width.Docs: add a disambiguation notice to the 7 sizing utility pages. Token/utility tables interleave exceptions with scale tokens in ascending resolved-pixel-value order.
Naming rule:
pxsuffix in a stop = literal pixel value; bare integer = scale unit on the 64px base.Migration & lint updates (DLT-3332)
Extend existing migration helper configs and ESLint rule to cover the five off-scale values so consumers can migrate legacy callsites automatically:
utility-class-to-token-stopsmigration — extendedSIZING_MAPso.d-w1→.d-w-1px,.d-h24→.d-h-24px, etc. across all 6 sizing prefixes.size-to-layoutmigration — extendedLAYOUT_MAPsowidth: var(--dt-size-N)at the 5 small stops (100/200/400/525/550) routes tovar(--dt-layout-Npx)in layout-property context. Property-aware routing already existed; only the mapping data was missing. Spacing-context routing unchanged (regression guard in tests).deprecated-pixel-utility-classesESLint rule — extendedSIZING_PIXELS(in descending-length order) so.d-w1,.d-h24,.d-hmn8, etc. are flagged as deprecated in editors.What this does NOT do:
.d-w1/.d-h1/… calc-based path generation (stays on its existing deprecation timeline)💡 Context
The
dt-layout-*scale is indexed on a 64px base —100= 64px, each step of25= 16px. Works cleanly for 16px-and-up multiples, but sub-16px (1, 2, 8px) and off-grid-between-steps (20, 24px) values produce hostile token names (dt-layout-1.5625,dt-layout-37.5, …). Product teams need these values. This change provides them as first-class tokens with a predictable naming rule that humans and AI can pick from a spec without math.📦 Cross-Package Impact
dialtone-tokensdialtone-cssand all downstream Vue/docs/MCP buildsdialtone-cssLAYOUT_STOPSextended; 35 new hyphenated utility selectors; migration helper configs extendeddialtone-documentationand the MCP data blobsdialtone-documentation_data/width-height.jsonextendedeslint-plugin-dialtoneSIZING_PIXELSindeprecated-pixel-utility-classesrule extended.d-w1-style classesDependency flow: tokens → css → vue / docs / MCP / eslint
📄 Documentation Artifacts
dialtone-cssbuild outputs automatically/dt-migrateskill doc:size-to-layout+utility-class-to-token-stopsentries note off-scale coverage📝 Checklist
For all PRs:
For all CSS changes:
🔮 Next Steps
.d-w1/.d-h1/… Tier 1 calc-based path once consumers migrate.