-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
ui: Native CSS Icon Composition #12461
Conversation
8a07d5a
to
ac38332
Compare
070b44d
to
a0ce393
Compare
Almost forgot but wanted to note here that in amongst the commits after writing the original description we went through all the icons we were actually using and only included those by commenting all the imports out and then uncommenting the ones we were using. This got us down to ~60kb CSS again (gzipped) which I'm still pretty happy with considering its got all our icons in it also. Although thats nowhere near the 192kb gzipped for all the icons as mentioned in the description above, it is (I think it was around) 5-6kb bigger than it was originally. It was originally around 56kb gzipped or something as far as I remember (I might come back here and add a more factual before/after). I figured this is because we are including both resolutions of icon whether you use both or not. Solution to this is just to even more finely-grain the imports, which should just be a little more fine tuning of the svg2icon generation script in For the sake of 5-6kb and time what we have on right now, I'm happy to punt this saving for another time. Little note, I'm letting this sit just a little while longer whilst I get a little more feedback on the approach, and make sure I've had time to take the blinkers off 😎 . I've rebased with main and PRed another PR on top here that was waiting on approval of this one so we can keep that moving. |
🍒 If backport labels were added before merging, cherry-picking will start automatically. To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/606744. |
It's a biggie.
TLDR;
From a review standpoint you are probably best looking at https://github.com/hashicorp/consul/tree/413b0b9785247450e38942e059836e63a876d58b/ui/packages/consul-ui/app/styles/base/icons specifically:
Viewing individual commits will also probably be easier. Please ask if you have any questions.
We perhaps infamously have always used CSS for all our icons in the Consul UI, and perhaps even more infamously used
%placeholder/@extends
in order to really easily add icons to HTML elements from The Outside.This PR begins to wean ourselves off of using a CSS processor for our icons. i.e. this PR sets us up for complete removal of
%placeholder/@extends
for our icon support, therefore only using native browser features for our decorative iconography.Probably the best place to continue reading are the docs themselves
Also Preview Site. We've also reverted this
From #12222
Whilst this reverts this change of approach, the approach here reverts and adds the option of using font based sizing or absolute based styling using
--icon-size: icon-000;
(font based, or size=none/000) or--icon-size: icon-300;
(absolute based: 16px/1rem)What
%placeholder/@extend
gave/gives usAbility to compose CSS multiple properties
@extend
,@apply
,composes:
etc. all pretty much achieve the same thing. They allow you to group up collections of CSS declarations (custom or not) and then assign them to specific selectors. This allows a very high level of reuse of collections of CSS declarations whilst keeping a good clear boundary between the HTML and CSS ('what it is' vs 'what it looks like') in that you avoid littering your HTML with ever long lists of classes. For example, we currently have%frame
placeholders which works like so:You can easily give a
div
this 'frame' from within CSS-land with a one liner:Selector Tagging/naming
Say you have a complicated selector, or a selector that you want to specifically expose to an end consumer to be styled how they like, using
%placeholder/@extend
you can tag specific important DOM nodes for easy styling purposes:This is almost like the inverse of using placeholders to compose properties, here you are using placeholders as an identifier to allow you to easily define properties from The Outside. Importantly, the author of the component is free to change the DOM selector without having to change how the consumer refers to the DOM node.
I added an extra div there, but the consumer continues to use
%component-name-header
. This is all without using potentially problematic classNames.CSS tree-shaking
Very importantly you should only ever use
@extend
along with%placeholder
s, if you start@extend
ing normal class names things will go boom very quickly. Apart from that,%placeholders
give you the additional characteristic that if you don't use them, they are excluded/'tree-shaken' out of the CSS build.Linting/'type' checking
@extend
will complain if the thing you are@extend
ing doesn't exist, therefore as an engineer you get a build time warning if you try to use an icon that doesn't exist. This will give you an extra headstart when things go wrong.So why are we moving away from '%placeholders/@extends'?
We currently use '%placeholders/@extends' in the Consul UI to great effect, at a rough guess I would say we use it in the high 100s of times and have had zero problems with that. It's been an incredibly useful tool for moving between different CSS approaches (bulma at al) and various refactors done over the lifetime of the v2 UI.
But, ideally I'd like to get to a place where we use no (or very little) CSS processing at all. As of right now, apart from
%placeholder/@extends
, we use no processor-only features. Almost everything is using standard CSS (we have a little nesting but that is easy to remove).Moreover we are moving to use a more modern web platform featured codebase overall. This means being able to use CSS-in-JS (importantly not write CSS-in-JS). We need to be able to consume all of our 100s of CSS files separately for individual components which pretty much makes any CSS pre-processing out of the question, especially processing with SASS (we could still process using Javascript at runtime). I realized we'd got to a point where using various different techniques we can remove our reliance on '%placeholders/@extends' for CSS composition entirely and move to a completely native Web Platform based approach.
The replacement(s)
Constructable Stylesheets
Importantly, Constructable Stylesheets are only available on ShadowDOM nodes and documents, and whilst not 100% supported across all our browser targets there is a perfectly good fallback for browsers that don't yet support them and a good library for abstracting that detail away from the engineer. Constructable Stylesheets are great for composing CSS rules this means that Constructable Stylesheets have the selectors grouped with them, you then compose the rules not the properties/declarations. Whilst this helps us to a great extent this means you cannot apply a set of CSS properties/declarations to a DOM node if you need to select it differently. So overall it incredibly useful, but not the total package.
CSS Parts
CSS parts give us the ability to select/tag DOM nodes and expose them to end consumers with a reliable name/selector that then allows us to change the DOM without affecting the end user, essentially the 'Selector Tagging/naming' usecase from above. We can now change this:
into
Again, this is only available to us when using ShadowDOM.
CSS Variables
We recently almost entirely moved away from SASS variables in our application (our external
iris
package has been based on CSS properties since day one which was converted to SASS as a build step). CSS properties are much more useful than SASS variables as they are runtime, but do have the double edged sword of them also cascading.As well as being used to specify single properties, additionally for some use cases CSS properties can be used for enabling multiple CSS properties/declarations using the conditional space hack/trick.
CSS Keyframes
It turns out we've had a partial solution to composing groups of CSS declarations in native CSS for quite sometime:
Sets of related compositions can be grouped together and applied by sending the timeline a specific point:
Turns out this last approach works wonderfully with our CSS icons. We can name the icons, provide aliasing, sizing, even icon variations similar to font-variations. All provided by native Web Platform CSS.
Tree-shaking vs explicit import/inclusion.
Probably the biggest thing to note is that as we are including all icons in our built output our CSS has ballooned to around 192kb gzipped and this is currently very noticeable when viewing the preview site. This is because we are no longer using the tree-shaking properties of
%placeholder
. Which means ~1MB worth of iconography is being downloaded by our users.The approach here is to only import what we use, i.e. explicit importing of individual icons.
Between now and our next major release we need to catalog all the icons we currently use and then just stop to including/importing all the icons we don't need instead of including everything and tree-shaking it out.
You'll see in the PR what we've split out all our icons into 100s of separate files, this means we can very granularly include what we want and leave the rest.
'Type checking'/Linting
Whilst not available in all our browser targets as yet,
CSS.registerProperty
is a thing coming soon, and handily is available in the browser we use most for building the UI. This means that we can userunInDebug
to useCSS.registerProperty
to add dev-time only enums/types for any CSS custom properties we add. Using this characteristic we can provide a warning to let us know when we need to import a new 'icon CSS bundle', i.e. if--icon-name: icon-brand-new;
is not a valid value, we'll get aconsole
error and know we need to import new CSS for the icon. Once its more widely available we can remove therunInDebug
and take advantage of runtime type/enum checking for all our icons/CSS. I'm thinking I can use this initially to help me with the cataloging of icons. Hopefully I can register--icon-name
, and slowly add the names that we are using as I see the errors inconsole
. Once I'm done there I'll have a full list of all the icons we currently use to use for configuring what we include.Lastly, again please make sure you read the docs themselves to see the approach, this description is mainly The Why and The How, not so much The What.
Things I'm not 100% happy about
This PR has almost 3000 files changed, they are 99.99% machine generated and shouldn't really be in this main repository. Everything is currently pointing towards an externally published package of some sort, something which I'm still thinking about. I'd be love to hear any thoughts/ideas/opinions on that. I don't think its a big deal that I'm adding all these files here temporarily.