Theme bridge to `v5.0.0` #2373

r100-stack · 2024-12-16T13:32:13Z

Changes

#2364 refactors some code to introduce private css variables that help with the bridge code mapping (more info below). This PR will be merged after that PR.

Since @itwin/itwinui-react@5.0.0-alpha.0 is now published, this PR implements a bridge for existing iTwinUI applications and packages to look similar to this future version and help in migration and early adoption.

The ThemeProvider's themeOptions now accepts a new option called bridgeToFutureVersions which when passed adds data-iui-bridge='true' to the iui-root. It also wraps the iui-root with Root from itwinui-react v5 in order to load its CSS variables (--kiwi-…).

React:

<ThemeProvider
  themeOptions={{
    bridgeToFutureVersions: true
  }}
>
  …
</ThemeProvider>

Rendered:

+ <div class="🥝-root" data-kiwi-theme="dark" data-kiwi-density="dense">
    <div
      class="_iui3160-root"
      data-iui-theme="dark"
      data-iui-contrast="default"
+   data-iui-bridge="true"
    >
      …
    </div>
+ </div>

The presence of this data attribute activates the bridge code that maps the existing --iui-… CSS variables to --kiwi-… variables from v5. It also adds some limited styles only whenever it was needed. Since some of these mappings are subjective, especially when something similar doesn't exist in iTwinUI-react v5, open to other views.

[data-iui-bridge='true'] {
  * {
    --iui-color-background: var(--kiwi-color-bg-neutral-base);
    … // other mappings
  }
}

The bridgeToFutureVersions option and data-iui-bridge names are bikesheddable. I also instead considered adding new accepted themes itself like dark-v5 and light-v5 that the theme prop could accept. However, I felt those would lead to many code changes and potentially confusion and complexity. Open to other suggestions.

The mapping code lies in bridge.scss that is exported by itwinui-css in the itwinui.bridge layer. There are some TODOs in that file for whenever I wasn't sure of the mapping. Open to comments on those, if any.

This mapping was most tested in the dark theme. Some testing was done with the light theme too and will test more too. The high-contrast dark and high-contrast light themes continue using the regular dark and regular light themes. Once iTwinUI-react v5 supports high-contrast themes, we can make the necessary changes, if any, in the bridge.

TODO:

Make comments like "To match Kiwi/iTwinUI" to something more descriptive.

Testing

Manually tested in react-workshop, css-workshop, and vite-playground. Added bridgeToFutureVersions toggles in react-workshop and css-workshop next to the theme toggles.

Docs

Added changesets.
Add instructions on how to setup iTwinUI-react@5's <Root> on the users' side (#2373 (comment)).

After PR TODOs

Added flex-shrink: 0 to iTwinUI v3's iui-button-icon (#2373 (comment))
Are status-hover variables not being used anywhere in iTwinUi v3? Investigate. (162008f)

apps/css-workshop/src/pages/_layout.astro

apps/css-workshop/package.json

packages/itwinui-react/src/styles.js/styles.module.css

packages/itwinui-css/src/bridge.scss

packages/itwinui-react/src/core/ThemeProvider/ThemeProvider.tsx

packages/itwinui-css/src/bridge.scss

Co-authored-by: Mayank <mayank99@users.noreply.github.com>

mayank99

🚀 3.17.0-dev.0 is released with accompanying instructions for consumers to try it out.

Early next week, we'll do some final cleanup before merging into main.

packages/itwinui-react/src/core/ThemeProvider/ThemeProvider.tsx

apps/css-workshop/src/pages/_layout.astro

pnpm-lock.yaml

packages/itwinui-react/src/core/ThemeProvider/ThemeProvider.tsx

mayank99 · 2024-12-24T15:54:29Z

apps/react-workshop/.ladle/components.tsx

@@ -74,4 +86,8 @@ export const argTypes = {
    control: { type: 'boolean' },
    defaultValue: false,
  },
+  bridgeToFutureVersions: {
+    control: { type: 'boolean' },
+    defaultValue: true,


Are we good to flip the defaults back now and make CI pass?

Also we should rename this setting to "theme bridge" in our workshops as well.

Yes, flipped the defaults back (3dea59e).

Will investigate into the remaining CI failures.

mayank99 · 2024-12-24T15:57:33Z

apps/react-workshop/.ladle/components.tsx

+    document.body.dataset.iuiTheme = theme;
+    document.body.dataset.iuiBridge = bridgeToFutureVersions;


q: Can we set everything on <html> instead of setting some of these on <body>?

Yes, those two lines don't seem necessary, removed (52020e8).

mayank99 · 2024-12-24T15:58:34Z

packages/itwinui-css/src/button/base.scss

  display: inline-flex;
+  flex-shrink: 0;


Do we want to move this change out into a separate PR?

Yes, that sounds better, removed that line from this PR (52020e8). Added an after PR TODO to readd it in another PR.

We should make another release 3.17.0-dev.1 before this PR merges. We can temporarily add it back for the purpose of that release (otherwise the SideNavigation would look broken).

Ohh, right, added back (46eb69d).

packages/itwinui-css/src/expandable-block/expandable-block.scss

mayank99 · 2024-12-24T16:01:05Z

apps/css-workshop/src/components/theme.js

+    const root = document.body;
+    const v5Root = document.documentElement;


Similar comment here as Ladle: can we set all attributes on the same element (<body> or <html>) instead of mixing and matching?

iui-root cannot be on html since iui-root sets a font-size of 0.875rem (code) which would make the root font-size as 0.875rem and not 1rem.

We can set the data attributes on html without adding iui-root as we are currently doing in react-workshop for the page background ((code)). However, then iui-root + its theme related data attributes will still need to be placed somewhere.

To just have one place with the data attributes + the iui-root class, we chose <body> as that place in css-workshop.

Regarding the selected lines, the reason we set color-scheme on <html> and not <body> is because when placed on <html> those iTwinUI v5 variables are available throughout the page and not just within 🥝-root.

apps/css-workshop/src/components/theme.js

mayank99 · 2024-12-24T17:45:17Z

packages/itwinui-css/src/bridge.scss

+
+  .iui-breadcrumbs {
+    // Neutral color instead of accent
+    --_iui-breadcrumbs-content-color-default: var(--iui-color-text);


I think we can get rid of this, right? We decided to use the same set of color variables as current iTwinUI v3 implementation.

True, removed (46eb69d). Will update #2364 soon.

packages/itwinui-react/src/core/ThemeProvider/ThemeProvider.tsx

apps/css-workshop/package.json

mayank99 · 2024-12-24T17:52:47Z

packages/itwinui-css/src/bridge.scss

+
+  .iui-list-item {
+    // Since iTwinUI v3 has hover background color but iTwinUI v5 doesn't
+    --_iui-list-item-background-hover: transparent;


I don't think this is correct anymore. We do have a background hover in Kiwi.

Ohh, I see. I wasn't sure what hover color to give since we don't want to make the code more complex with color-mix. So, I left the custom property declaration as-is but just updated the comment to reflect the new reason why it is there. (46eb69d).

If we indeed want a hover color, open to ideas for replacements.

Well, we shouldn't need this component-level override, right? It should automatically use whatever color we are setting globally.

True, but here there is no default background. On hover, it goes to --iui-color-background-hover which is --iui-color-background itself. Going from no background to background on hover didn't look correct to me in v5:

demo.mov

So, I thought to override it and remove it on hover. If others feel differently, I can change it.

packages/itwinui-react/src/core/ThemeProvider/ThemeProvider.tsx

r100-stack added 2 commits December 16, 2024 08:24

🛫

17f2f4d

Leftover

cd6e72c

r100-stack self-assigned this Dec 16, 2024

r100-stack added 7 commits December 16, 2024 17:23

Partial fixes

7a1a845

Fixed theme toggles

44d3e9b

Merge branch 'r/private-css-vars' into r/bridge

4b0836c

Fixes

7b35137

Fixes

950f796

Cleanup

8b213e9

Simplification

ca20e05

This comment was marked as off-topic.

Sign in to view

r100-stack marked this pull request as ready for review December 17, 2024 17:16

r100-stack requested a review from a team as a code owner December 17, 2024 17:16

r100-stack requested review from mayank99, smmr-dn and FlyersPh9 and removed request for a team December 17, 2024 17:16

This comment was marked as resolved.

Sign in to view

r100-stack added 2 commits December 17, 2024 13:19

Leftover

ee51d4b

Merge branch 'r/private-css-vars' into r/bridge

3303613

r100-stack mentioned this pull request Dec 17, 2024

Abstraction of style values into private css variables #2364

Open

1 task

This comment was marked as resolved.

Sign in to view

r100-stack added 3 commits December 18, 2024 09:16

nit

ba8e6df

Merge remote-tracking branch 'origin/r/private-css-vars' into r/bridge

7c778a1

pnpm-lock.yml

864fbe2

mayank99 reviewed Dec 18, 2024

View reviewed changes

apps/css-workshop/src/pages/_layout.astro Outdated Show resolved Hide resolved

apps/css-workshop/package.json Outdated Show resolved Hide resolved

mayank99 reviewed Dec 18, 2024

View reviewed changes

packages/itwinui-react/src/styles.js/styles.module.css Outdated Show resolved Hide resolved

packages/itwinui-css/src/bridge.scss Outdated Show resolved Hide resolved

mayank99 reviewed Dec 18, 2024

View reviewed changes

packages/itwinui-react/src/core/ThemeProvider/ThemeProvider.tsx Outdated Show resolved Hide resolved

packages/itwinui-react/src/core/ThemeProvider/ThemeProvider.tsx Outdated Show resolved Hide resolved

mayank99 reviewed Dec 18, 2024

View reviewed changes

packages/itwinui-css/src/bridge.scss Show resolved Hide resolved

Comments

808056b

r100-stack added 5 commits December 19, 2024 19:17

Contrast improvements, TODOs

1eb5b25

Merge branch 'r/private-css-vars' into r/bridge

de35da6

Backgrounds, status borders, non-current breadcrumbs

c45806f

Button group fix

2eb99bf

scoping, few hsl variables

4ccd98c

r100-stack force-pushed the r/bridge branch from 81d413e to 4ccd98c Compare December 20, 2024 20:05

r100-stack and others added 3 commits December 20, 2024 15:55

HSL, fix tree selection background and others.

b9d90ce

Co-authored-by: Mayank <mayank99@users.noreply.github.com>

nit

55fbde0

Co-authored-by: Mayank <mayank99@users.noreply.github.com>

Bumping to dev version.

9307ef9

Co-authored-by: Mayank <mayank99@users.noreply.github.com>

mayank99 reviewed Dec 20, 2024

View reviewed changes

packages/itwinui-react/src/core/ThemeProvider/ThemeProvider.tsx Outdated Show resolved Hide resolved

future.themeBridge prop

2199938

mayank99 reviewed Dec 24, 2024

View reviewed changes

r100-stack added 5 commits December 24, 2024 11:13

Comments

3dea59e

bridge.scss simplification

a338de9

Other comments

52020e8

Merge branch 'r/private-css-vars' into r/bridge

af04c6d

Leftover

162008f

mayank99 reviewed Dec 24, 2024

View reviewed changes

smmr-dn reviewed Dec 24, 2024

View reviewed changes

packages/itwinui-react/src/core/ThemeProvider/ThemeProvider.tsx Show resolved Hide resolved

smmr-dn reviewed Dec 24, 2024

View reviewed changes

apps/css-workshop/package.json Outdated Show resolved Hide resolved

mayank99 reviewed Dec 24, 2024

View reviewed changes

packages/itwinui-react/src/core/ThemeProvider/ThemeProvider.tsx Show resolved Hide resolved

r100-stack added 7 commits December 24, 2024 12:59

Bump @astrojs/react

52252cf

Pass future to portal container

0bcb005

More comments

46eb69d

Remove unnecessary hardcoded theme

be0762e

More comments in bridge

1412dac

Default prop value for future

fc42aea

Update itwinui-react dev version

5ac59aa

r100-stack mentioned this pull request Jan 9, 2025

Add missing flex-shrink: 0 to iui-button-icon #2396

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Theme bridge to `v5.0.0` #2373

Theme bridge to `v5.0.0` #2373

r100-stack commented Dec 16, 2024 •

edited

Loading

This comment was marked as off-topic.

This comment was marked as resolved.

This comment was marked as resolved.

mayank99 left a comment

mayank99 Dec 24, 2024

mayank99 Dec 24, 2024

r100-stack Dec 24, 2024

mayank99 Dec 24, 2024

r100-stack Dec 24, 2024

mayank99 Dec 24, 2024

r100-stack Dec 24, 2024

mayank99 Dec 24, 2024

r100-stack Dec 24, 2024

mayank99 Dec 24, 2024

r100-stack Dec 24, 2024

mayank99 Dec 24, 2024

r100-stack Dec 24, 2024 •

edited

Loading

mayank99 Dec 24, 2024

r100-stack Dec 24, 2024

mayank99 Dec 24, 2024 •

edited

Loading

r100-stack Dec 24, 2024 •

edited

Loading

		document.body.dataset.iuiTheme = theme;
		document.body.dataset.iuiBridge = bridgeToFutureVersions;

		const root = document.body;
		const v5Root = document.documentElement;

Theme bridge to v5.0.0 #2373

Are you sure you want to change the base?

Theme bridge to v5.0.0 #2373

Conversation

r100-stack commented Dec 16, 2024 • edited Loading

Changes

Testing

Docs

After PR TODOs

This comment was marked as off-topic.

This comment was marked as resolved.

This comment was marked as resolved.

mayank99 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

r100-stack Dec 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mayank99 Dec 24, 2024 • edited Loading

Choose a reason for hiding this comment

r100-stack Dec 24, 2024 • edited Loading

Choose a reason for hiding this comment

Theme bridge to `v5.0.0` #2373

Theme bridge to `v5.0.0` #2373

r100-stack commented Dec 16, 2024 •

edited

Loading

r100-stack Dec 24, 2024 •

edited

Loading

mayank99 Dec 24, 2024 •

edited

Loading

r100-stack Dec 24, 2024 •

edited

Loading