[PRO-858] Theme API: data-driven E2E expectations per theme (design tokens)#12239
Closed
[PRO-858] Theme API: data-driven E2E expectations per theme (design tokens)#12239
Conversation
- Add figma token conversion tool at handsontable/src/themes/figma/ - Store design tokens JSON as tokens.json (single source of truth) - Tool outputs CSS and JS variable files directly to src/themes/static/ - Add npm script: npm run build:design-tokens - Use .mjs extensions for ES module support within CommonJS package - Absolute paths via import.meta.url for CWD-independent execution Workflow: Export tokens from Figma → save as tokens.json → run build:design-tokens Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- New test helper: designTokens.js resolves token references to numeric px values - Exported functions: token(), tokenForThemes(), getTokenValue(), calcRowHeight(), calcColHeaderHeight(), forAllThemes() - Token resolution chain: tokens → density → sizing → px value - Registered as global test helper via test/helpers/index.js Enables tests to calculate expected dimensions from design tokens instead of using hardcoded magic numbers per theme. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tests Convert .forThemes blocks in 16 test/e2e spec files to calculate expected dimensions from design tokens instead of using magic numbers. Formulas: calcRowHeight(t), calcColHeaderHeight(t), forAllThemes() - Row height = lineHeight + (cellVerticalPadding * 2) + 1px border - Col header height = lineHeight + (cellVerticalPadding * 2) Files converted: - fixedRowsBottom, fixedRowsTop, scrollToFocusedCell, scrollViewportTo - viewportScroll, refreshDimensions, batch, getRowHeight - ColHeader, Core_view, Core_resize, Core_reCreate - beforeViewportScrollVertically, modifyRowHeightByOverlayName - beforeRefreshDimensions, afterRefreshDimensions Note: 65 plugin spec files remain to be converted. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Extend designTokens with token resolution and calcE2e* helpers - Allow E2E bundle to import theme sizing/density/token modules (babel) - Update E2E and plugin specs for classic, main, and horizon themes Made-with: Cursor
Contributor
Author
commit: |
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| main: fn('main'), | ||
| horizon: fn('horizon'), | ||
| }; | ||
| } |
Contributor
There was a problem hiding this comment.
Exported helper functions never used anywhere in codebase
Low Severity
tokenForThemes (line 165) and forAllThemes (line 212) are exported but have zero call sites in the codebase — the only grep matches are their own definitions. Since index.js calls exportToWindow(designTokens), these get assigned to window at test bundle time, unnecessarily polluting the global namespace and adding dead code to the test bundle.
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
2 participants
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.
Context
Handsontable ships multiple themes (classic, main, horizon) with different typography, spacing, and density. Many E2E and plugin specs asserted pixel-perfect dimensions as hardcoded numbers for a single theme, which made those tests fragile when the loaded theme changed and did not document the relationship to our design system.
This change aligns with AGENTS.md testing guidance: calculate expectations from design tokens (and documented formulas) where possible, and isolate remaining Chromium-only layout measurements as explicit fixture values.
What this PR does
handsontable/test/helpers/designTokens.jssizing,density, and per-theme token maps (classic/main/horizon).calcRowHeight,calcColHeaderHeight,getTokenValue, and manycalcE2e*functions used by specs.calcRowHeight/calcColHeaderHeight/getTokenValueplus small, documented E2E fixture deltas (browser/layout margins that are not individual Figma tokens).calcE2eMinMasterCellOuterWidth: uses the documented default column width (50px, per meta schema) adjusted by horizontal cell padding vs classic, with a Horizon-only empirical adjustment where the DOM still differs from a pure padding delta.calcE2e*values remain intentional Chromium measurements (e.g. large scroll positions, auto-sized column widths from content) where a stable token-only formula does not exist.handsontable/babel.config.jssizing,density, theme token files) used bydesignTokens.js, so helpers run inside the E2E test bundle.E2E and plugin
*.spec.jsupdatesspec()/ theme helpers fromtest/helpers/designTokens.js(and existing test infrastructure) so expectations vary correctly withgetLoadedTheme()for classic, main, and horizon.What this PR does not do
How has this been tested?
npm run test:e2e.dumpinhandsontable/— confirms the E2E webpack bundle builds withdesignTokensand theme variable imports.pnpm --filter handsontable run test:e2e(or targeted Puppeteer specs for the areas touched).Types of changes
Related issue(s):
Affected project(s):
handsontable@handsontable/angular-wrapper@handsontable/react-wrapper@handsontable/vue3Checklist:
designTokens.jsdocuments the approach.Additional (AGENTS.md)
bin/changelog entrybefore merge if required for test-only changes inhandsontable/.[skip changelog]
Note
Low Risk
Changes are limited to test infrastructure, build scripts, and E2E expectations; no production runtime behavior or public API is modified. Main risk is CI noise if any token-to-DOM mapping or Chromium fixture value is off, causing flaky E2E failures.
Overview
Refactors a large set of Handsontable E2E/plugin specs to replace hardcoded pixel assertions with theme-aware calculations (e.g.
calcRowHeight,getTokenValue, andcalcE2e*helpers), making expectations consistent across classic, main, and horizon.Adds a new
test/helpers/designTokens.jsresolver that reads generated theme token/density/sizing modules and exposes both token-derived formulas and a small set of explicit Chromium fixture values (e.g. scrollbar width/empirical deltas) used by the updated specs. Build/test wiring is updated to support these imports (Babel allowlist + newbuild:design-tokensscript), and testing docs now include a runtime--specfilter flow for running a single E2E suite.Written by Cursor Bugbot for commit bc10408. This will update automatically on new commits. Configure here.