svelte: Reduce imported global CSS#62457
Conversation
Until now we've simply imported the global `base.scss` file, which in turn imports a bunch of other files and defines global CSS classes. Many (most?) of them are actually not used. This commit reduces to the number of imported files to the ones that contain styles that are used. My methodology was to inspect every file that `base.scss` imports and find out whether it defines custom properties (variables) or global classes. In some cases I extracted variables into their own files.
9cfc089 to
8172741
Compare
There was a problem hiding this comment.
Made a couple of changes here and related files that removed the use of classes for "outer component" styling.
|
|
||
| form, | ||
| div, | ||
| :global([data-scroller]) { |
There was a problem hiding this comment.
This was affecting all scroll instances, which is wrong.
| @@ -0,0 +1,43 @@ | |||
| :root { | |||
There was a problem hiding this comment.
Why do we have these variables on the global level? Should it be incorporated with a special Dropdown component instead of variables?
There was a problem hiding this comment.
Should it be incorporated with a special Dropdown component instead of variables?
That's what I would prefer. I haven't audited the code where these variables are used. In this PR I'm really just trying to remove the styles we are not using. Refactoring everything to properly scope it should be a separate PR (IMO).
There was a problem hiding this comment.
Agree, maybe we should leave a comment here that these variables are used at the moment but they shouldn't be used in any new components freely
| @@ -0,0 +1,9 @@ | |||
| :root { | |||
There was a problem hiding this comment.
Same question as about dropdown variables: why do we need to have them on the global level instead of having them on the popover level?
We still can have some global variable that can effect popover variables like different box-shadows levels for instance.
|
Havent' tested this manually yet, so just left a few questions, will review it properly by the end of the day |
vovakulikov
left a comment
There was a problem hiding this comment.
Checked this manually, haven't noticed any UI regressions, thanks
Part of #62428
Until now we've simply imported the global
base.scssfile, which in turn imports a bunch of other files and defines global CSS classes. Many (most?) of them are actually not used.This commit reduces to the number of imported files to the ones that contain styles that are used. My methodology was to inspect every file that
base.scssimports and find out whether it defines custom properties (variables) or global classes. In some cases I extracted variables into their own files.I can't guarantee that the included files only declare styles that are used, but it's a first step.
Test plan
Compared all pages "side-by-side", switching between S2 and local tabs to catch any "drift", including the revision selector, search results popovers and search input suggestions.