-
-
Notifications
You must be signed in to change notification settings - Fork 520
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
refactor: Switch off LESS to vanilla CSS #1036
Conversation
+1 I like the move to vanilla css. Grabbing the actual values from |
@@ -3,7 +3,6 @@ | |||
min-width: 13em; | |||
padding: 0.5rem 1.5rem; | |||
margin: 0 1rem; | |||
box-shadow: 0 0.125rem 0 rgba(var(--color-brand), 0.3); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As --color-brand
is a hex value, it never worked here. I don't think it looks that great anyways and is practically invisible on the dark scheme, so I've removed it.
We do have --color-brand-triplet
to support this in other places now though.
@@ -131,183 +87,3 @@ progress-bar { | |||
transform: translateX(150%); | |||
} | |||
} | |||
|
|||
main .markup { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've always had a home.less
stylesheet that's practically empty -- I opted to move all the home-specific styles to it to clear up this index file a bit.
background: @color-corner; | ||
background: var(--color-brand-light); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it was intentional on my part. Don't mind changing it though
ol, | ||
ul { | ||
margin-left: 0; | ||
} | ||
|
||
select { | ||
font-size: 100%; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved into index.css
517759b
to
1d1c686
Compare
1d1c686
to
c18ac8b
Compare
@@ -1,5 +1,3 @@ | |||
@import '~style/helpers'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interestingly, this change shaves a huge chunk off all of our CSS bundles. Per the spec, I guess a.css
importing b.css
results in all of b
ending up in a
.
With the way we were importing our variables (through helpers
here), we ended up with a lot of copies of our :root
. If you load the tutorial and scroll through the style editor in your dev tools you'll see 4 full copies. Oops
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, reduces transfer size by 4.5kb and total size by nearly 7kb.
The page is still 381kb transferred so it's not a measurable improvement of course, just nice to see.
Easier to scan the diff on GitHub while I work than locally 😅
Toying with switching to plain CSS -- it's gotten pretty darn good and we don't make use of that much of LESS in all reality (15 vars & 5 mixins total, and some of these are long unused). There's a couple areas that get slightly more fiddly (the
sidebar-break
variable in particular -- we use it for media queries but that's still not supported in standard CSS, sadly) but by and large things are unchanged.TODO:
Enable PostCSS nesting plugin for old browsers, probably don't want to ship nested CSSDONEfilter: brightness()
is supported for swapping outlighten()
anddarken()
utilities. Right now I'm just grabbing the computed hex value from the browser and reinserting with a comment