Overhaul tokens #735
Overhaul tokens #735
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
I didn't yet get as far as updating the |
| --sk-page-padding-top: 8rem; | ||
| --sk-page-padding-side: 4.8rem; | ||
| --sk-secondary-nav-height: 6rem; | ||
| } |
There was a problem hiding this comment.
was the split really necessary? There's already a TODO in colours.css about "move this somewhere else?", and it's only going to get tricker as time goes. Like, we may have a --border-1 setting for 1px solid var(--...) at some point, and then the question is where does that even go. That would be much easier if everything was in one file, and then you could just put it next to the border stuff.
There was a problem hiding this comment.
I find it a lot more manageable, yeah. I resolved that TODO by moving the raised button styles into buttons.css (even if they're not always used with a <button>, the things that use those styles are always 'buttons')
| --sk-border-light: hsl(0, 0%, 95%); | ||
| --sk-border-medium: hsl(0, 0%, 92%); | ||
| --sk-border-dark: hsl(0, 0%, 86%); |
There was a problem hiding this comment.
are these the right names? in dark mode it's the opposite of what they say. Going with the -1/2/3 of the bg/fg seems more straightforward
There was a problem hiding this comment.
in dark mode it's the opposite of what they say.
No it's not, check again. This isn't the same as -1, -2, -3 etc because for foreground/background colours you're coming from the 'edges' — --sk-fg-1 is the darkest in light mode and the lightest in dark mode, and vice versa for bg (or should be, except that we currently do things in a confusing way which will be fixed in the follow-up PR).
For borders on the other hand, lighter should mean lighter, since it's used as a highlight for the top and left borders. (In fact maybe it should be --sk-border, --sk-border-highlight and --sk-border-shadow or something)
| padding-bottom: 1.3rem; | ||
| border-radius: 0 0 var(--sk-border-radius) var(--sk-border-radius); | ||
| } | ||
| border-radius: var(--sk-border-radius-inner); |
There was a problem hiding this comment.
These changes make it look like this, which is worse IMO
There was a problem hiding this comment.
Interesting... I was driven to make this change by noticing how the focus ring looks when you use keyboard navigation — this to me looks very silly indeed:
Personally I think it looks better. Even without the focus ring, on main we use weird padding tricks that result in the top and bottom items being larger when they're hovered. The version in this PR more closely reflects the way things work in macOS, which is the authority I trust on these sorts of questions more than any other OS:
Our CSS tokens are a bit of a mess. This PR tidies them up a bit — gets rid of some unused vars, consolidates duplicates, and organises things a bit more cleanly.
It also makes a couple of style tweaks — the focus ring now uses
--sk-fg-accent(formerly--sk-theme-1) rather than a faded version, and I think it looks better.