Skip to content

chore: fixes get css variables from stylesheet utility#156

Merged
georgewrmarshall merged 2 commits into
mainfrom
fix/design-token-css
Nov 26, 2024
Merged

chore: fixes get css variables from stylesheet utility#156
georgewrmarshall merged 2 commits into
mainfrom
fix/design-token-css

Conversation

@georgewrmarshall

@georgewrmarshall georgewrmarshall commented Nov 26, 2024

Copy link
Copy Markdown
Contributor

Description

This PR addresses issues in the design token storybook related to rendering CSS variables for brand color, and light and dark theme colors. Previously, the function responsible for rendering these variables was updated to comply with ESLint rules for the design system monorepo. However, because Storybook is not a Node.js environment, the updates failed to function as intended. This issue was missed during migration PRs and was my oversight.

This PR:

  1. Reverts the function to its previous implementation to restore compatibility with Storybook.
  2. Improves the function to better align with the upcoming improvements in the Storybook environment as outlined in [PR #147](chore: updating storybook-react docs #147).

Reason for the change

  • Fix rendering of CSS variables in the design token stories for brand color and themes.
  • Ensure compatibility with the Storybook environment.
  • Resolve a regression introduced by the migration PRs.

Related issues

Part of #129

Manual testing steps

  1. Pull this branch
  2. Run yarn storybook
  3. Open the design token storybook.
  4. Navigate to the brand color and light/dark theme color stories.
  5. Verify that the CSS variables render correctly in both themes.

Screenshots/Recordings

Before

before.css.vars720.mov

After

after720.mov

Even further improvements will be made in #147 to address the text color here

Screenshot 2024-11-26 at 2 24 44 PM

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g., pull and build branch, run the app, test the code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence, such as recordings and/or screenshots.

@georgewrmarshall georgewrmarshall self-assigned this Nov 26, 2024
@georgewrmarshall georgewrmarshall marked this pull request as ready for review November 26, 2024 01:25
@georgewrmarshall georgewrmarshall requested a review from a team as a code owner November 26, 2024 01:25
Comment on lines +1 to +5
/* eslint-disable no-restricted-globals */
/* eslint-disable id-denylist */
/* eslint-disable prefer-destructuring */
/* eslint-disable @typescript-eslint/prefer-for-of */
/* eslint-disable require-unicode-regexp */

@georgewrmarshall georgewrmarshall Nov 26, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to read CSS variables from the browser's computed styles because Storybook runs in a browser environment, where we can't use Node.js file system calls to read CSS files directly from packages/design-tokens/src/css.

Instead, we:

  1. Access the styles that Storybook has already loaded into the browser
  2. Read the computed values from document.styleSheets
  3. Use DOM manipulation to test different theme variations

This is why we need to disable certain ESLint rules - to allow browser globals and DOM operations that wouldn't be allowed in our typical Node.js environment.

The stories in BrandColors.stories.tsx and ThemeColors.stories.tsx demonstrate this browser-based approach to accessing our design tokens.

@georgewrmarshall

Copy link
Copy Markdown
Contributor Author

@metamaskbot publish-preview

@georgewrmarshall georgewrmarshall merged commit 8efa1db into main Nov 26, 2024
@georgewrmarshall georgewrmarshall deleted the fix/design-token-css branch November 26, 2024 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants