iTwin / iTwinUI

A design system for building beautiful and well-working web interfaces.
https://itwin.github.io/iTwinUI/
MIT License
93 stars 35 forks source link

css modules #1302

Closed mayank99 closed 11 months ago

mayank99 commented 12 months ago

Changes

Created a styles.js folder inside src that uses Vite to do CSS-modules processing. After processing, the iui- prefix becomes _iui3- (for example iui-button becomes _iui3-button). Not including hash yet to ease migration, but it will be a one-line change to add it in v4.

Vite will output two files: styles.css and style.js, both of which get copied into the published output of @itwin/itwinui-react. The styles.css file will need to be imported manually by users, and the styles.js file will be used inside polymorphic to get the transformed class names.

Made some tweaks in @itwin/itwinui-css to remove previous scoping mechanism (from #1031). Added :where to @itwin/itwinui-variables and required iui-root in all global styles.

Pending issues:

See branch preview: https://itwin.github.io/iTwinUI/1302/react/

Testing

Tested running dev and prod build of playgrounds and storybook. More extensive testing needed.

Docs

Updated starter templates and "getting started" pages. Added multiple changesets.

Updated migration guide: https://github.com/iTwin/iTwinUI/wiki/iTwinUI-react-v3-migration-guide#breaking-changes

LostABike commented 11 months ago

See branch preview: https://itwin.github.io/iTwinUI/1302/react/

I have not been able to preview branch like this for yours or any other ones. Do you know why?

LostABike commented 11 months ago

See branch preview: https://itwin.github.io/iTwinUI/1302/react/

I have not been able to preview branch like this for yours or any other ones. Do you know why?

image

mayank99 commented 11 months ago

See branch preview: itwin.github.io/iTwinUI/1302/react

I have not been able to preview branch like this for yours or any other ones. Do you know why?

The recent push to main (via #1315) cleared all the branch deployments. Updating branch (which I just did) should redeploy it after a couple minutes.

LostABike commented 11 months ago

o.o I think the stylings are breaking image image

mayank99 commented 11 months ago

o.o I think the stylings are breaking

yeah... i realized that components defined as polymorphic.div (or similar) were not going through scoping, so in https://github.com/iTwin/iTwinUI/pull/1302/commits/c23814ed4f4a5e90f47a0d24bb8c1399d0ce7287 i moved the scoping mechanism from Box.tsx into polymorphic.tsx. should be better now

there are still some styles breaking because the elements get portaled outside the ThemeProvider. looking into that now fixed in https://github.com/iTwin/iTwinUI/pull/1302/commits/a6debbb5da3bb49ed060f5665a4d44284df6b6c8. (should also be improved after #1300)

gretanausedaite commented 11 months ago

What is the benefit of users importing style file themselves vs having it bundled with React code?

mayank99 commented 11 months ago

What is the benefit of users importing style file themselves vs having it bundled with React code?

More flexibility, fewer hacks, less chance of duplicate CSS getting bundled, and wider support in all environments.

Importing .css inside a javascript file is not valid syntax. We have been relying on bundlers to magically take care of it, which is not right.

edit: moved this change to separate PR #1322 with more explanation and for ease of review.

mayank99 commented 11 months ago

i've made a few changes in https://github.com/iTwin/iTwinUI/pull/1302/files/28f48d985fb3bd730f930d3867b71c663d802249..152ec5692db9dc22f827dbb15a4fb84fc9460443 so most of the visuals should be fixed now.

unfortunately, many of the visual tests are still failing because they target iui- selectors. https://github.com/iTwin/iTwinUI/blob/86a9acfcc0d305b8f7a832df2fd83885bccb1b3d/apps/storybook/src/AvatarGroup.test.ts#L20-L22

gretanausedaite commented 11 months ago

unfortunately, many of the visual tests are still failing because they target iui- selectors.

Do we have strategy how to update tests? Simply change selectors to _iui3 or maybe drop iui part entirely?

gretanausedaite commented 11 months ago

I get error when trying to run storybook locally image Maybe some build scripts are missing?

mayank99 commented 11 months ago

unfortunately, many of the visual tests are still failing because they target iui- selectors.

Do we have strategy how to update tests? Simply change selectors to _iui3 or maybe drop iui part entirely?

We need to stop targeting classes completely. For now, I will update the visual tests to find alternative means to select the elements and I will update the unit tests to _iui3. In the future, we can add a lint rule to avoid targeting classes. See this commit as an example of how to refactor tests: 3cfde90


I get error when trying to run storybook locally image Maybe some build scripts are missing?

Can you try to run yarn build first?

gretanausedaite commented 11 months ago

Can you try to run yarn build first?

Did not help

mayank99 commented 11 months ago

I get error when trying to run storybook locally image Maybe some build scripts are missing?

Can you try to run yarn build first?

Did not help

Maybe fixed by 2b900ff. Check again?

mayank99 commented 11 months ago

All styles and tests should be fixed now.