Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

upcoming: [M3-8754] - Add Alias Tokens to the Theme #11138

Merged

Conversation

harsh-akamai
Copy link
Contributor

Description 📝

Alias tokens should be available to components via theme.*

Changes 🔄

  • Added alias tokens:
    • accentTokens
    • actionTokens
    • backgroundTokens
    • borderTokens
    • contentTokens
    • elevationTokens
    • interactionTokens
    • typographyTokens
    • radiusTokens

Target release date 🗓️

10/24

How to test 🧪

Verification steps

  • Ensure everything builds properly

As an Author I have considered 🤔

Check all that apply

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

@harsh-akamai harsh-akamai requested a review from a team as a code owner October 22, 2024 14:46
@harsh-akamai harsh-akamai requested review from mjac0bs and bnussman-akamai and removed request for a team October 22, 2024 14:46
@harsh-akamai harsh-akamai self-assigned this Oct 22, 2024
@harsh-akamai harsh-akamai added the Design Tokens Laying the groundwork for Design Tokens label Oct 22, 2024
@harsh-akamai harsh-akamai changed the title upcoming: [M3-6538] - Add Alias Tokens to the Theme upcoming: [M3-8754] - Add Alias Tokens to the Theme Oct 22, 2024
InteractionTypes as InteractionTypesLight,
TypographyTypes,
RadiusTypes,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Typography and Radius are the same for light/dark so simplified our Theme/ThemeOptions for now

Copy link

github-actions bot commented Oct 22, 2024

Coverage Report:
Base Coverage: 87.05%
Current Coverage: 87.05%

Copy link
Member

@bnussman-akamai bnussman-akamai left a comment

Choose a reason for hiding this comment

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

Rather than having things like theme.borderTokens, theme.chartTokens, I wonder if it would make sense to do something like theme.tokens.border, theme.tokens.chart @jaalah @harsh-akamai

@harsh-akamai
Copy link
Contributor Author

@bnussman-akamai This is a good suggestion. Though it would require a change in the ui package (breakpoints.ts)

I believe I will have to create a separate PR for a change in the ui package?

Copy link
Contributor

@mjac0bs mjac0bs left a comment

Choose a reason for hiding this comment

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

Confirming that Cloud built using the new tokens. (The interaction.Background.Secondary uses a darker shade of black in dark mode.)

Looks like there are merge conflicts to resolve in a few files, @harsh-akamai.

@mjac0bs
Copy link
Contributor

mjac0bs commented Oct 28, 2024

I also think we're going to want to be consistent with naming conventions. The following PRs (cc @pmakode-akamai , @zaenab-akamai) are using the previous method of naming with Tokens:

@jaalah-akamai Do we want to proceed across the board with <alias>Tokens or use tokens.<alias> as Banks previously suggested?

@jaalah-akamai
Copy link
Contributor

@harsh-akamai @bnussman-akamai @mjac0bs Let's do theme.tokens.* - I like that a lot

@harsh-akamai
Copy link
Contributor Author

harsh-akamai commented Oct 29, 2024

The interaction.Background.Secondary uses a darker shade of black in dark mode.

@mjac0bs Looks like there is no perfect replacement for bgAccessRow amongst the existing design tokens. We might have to bring back bgAccessRow.

cc @jaalah-akamai

@bnussman-akamai bnussman-akamai added the Add'tl Approval Needed Waiting on another approval! label Nov 1, 2024
@mjac0bs mjac0bs added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Nov 1, 2024
@bnussman-akamai bnussman-akamai merged commit 28ad446 into linode:develop Nov 1, 2024
23 checks passed
Copy link

cypress bot commented Nov 1, 2024

Cloud Manager E2E    Run #6774

Run Properties:  status check passed Passed #6774  •  git commit 28ad446c8d: upcoming: [M3-8754] - Add Alias Tokens to the Theme (#11138)
Project Cloud Manager E2E
Branch Review develop
Run status status check passed Passed #6774
Run duration 26m 49s
Commit git commit 28ad446c8d: upcoming: [M3-8754] - Add Alias Tokens to the Theme (#11138)
Committer Harsh Shankar Rao
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 1
Tests that did not run due to a developer annotating a test with .skip  Pending 2
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 445
View all changes introduced in this branch ↗︎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge! Design Tokens Laying the groundwork for Design Tokens
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants