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

Update theme bridge v5 variables based on v5.0.0-alpha.6 #2436

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

r100-stack
Copy link
Member

@r100-stack r100-stack commented Feb 13, 2025

Changes

Updates the iTwinUI v5 variable names in the theme bridge to match those from the soon to be released @itwin/[email protected] version.

Since that alpha version also released a zebra background color (--ids-color-bg-page-zebra), updated --iui-color-background-zebra to use that variable. (88edec5). However, not sure if I used that variable correctly because I saw little to no different of the zebra color from the regular background color.

Updated the css-workshop and react-workshop to use @itwin/[email protected].

Testing

  • CI passes
  • Manually test to see that everything looks alright in the css-workshop and react-workshop.

Docs

  • Added changesets

@r100-stack r100-stack self-assigned this Feb 13, 2025
@mayank99 mayank99 mentioned this pull request Feb 14, 2025
@r100-stack r100-stack marked this pull request as ready for review February 14, 2025 22:15
@r100-stack r100-stack requested a review from a team as a code owner February 14, 2025 22:15
@r100-stack r100-stack requested review from mayank99 and smmr-dn and removed request for a team February 14, 2025 22:15
@r100-stack r100-stack changed the title Update iTwinUI v5 variable names in the theme bridge Update theme bridge v5 variables based on v5.0.0-alpha.6 Feb 14, 2025
@r100-stack r100-stack removed the request for review from smmr-dn February 14, 2025 22:17
--iui-color-background-zebra: var(--iui-color-background); /* TODO: Use proper value when added */
--iui-color-background: var(--ids-color-bg-page-base);
--iui-color-background-disabled: var(--ids-color-bg-glow-on-surface-disabled);
--iui-color-background-zebra: var(--ids-color-bg-page-zebra);
Copy link
Member Author

Choose a reason for hiding this comment

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

To confirm, am I using the zebra variable correctly? Because I saw little to no difference between the regular and zebra Table rows.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, good catch. It looks like this variable is not meant to be used on its own; it's supposed to be combined with bg-page-base.

For now, I would say let's keep using bg-page-base and leave the TODO there. It can be fixed in a future release.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
--iui-color-background-zebra: var(--ids-color-bg-page-zebra);
--iui-color-background-zebra: var(--iui-color-background); /* TODO: Use proper value when added */

'@itwin/itwinui-react': patch
---

Theme bridge uses latest token names introduced in `@itwin/[email protected]`.
Copy link
Contributor

Choose a reason for hiding this comment

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

It's more than token names; we can just say "breaking changes".

Suggested change
Theme bridge uses latest token names introduced in `@itwin/[email protected]`.
The **theme bridge** has been updated to handle the breaking changes introduced in `@itwin/[email protected]`.

Copy link
Contributor

@mayank99 mayank99 left a comment

Choose a reason for hiding this comment

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

LGTM after the two suggestions are applied.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants