Add billboard component (Fixes #93) #105
Conversation
|
/cc @vincejoyiv |
|
Ok, I think this is now ready for review / merge |
|
Ok, one further amend to make. |
alexgibson
added
Do Not Merge ⚠️
|
Ok, last design tweaks are in place. This is ready for code review. |
There was a problem hiding this comment.
Looks great. I'm a little wary of some of the "magic numbers" that appear without explanation. I think we should define those as variables (just in this file, not tokens for consumption) with some maths to make it clear how those values are derived. That could also cut down on some of the repetition.
| &:hover, | ||
| &:active, | ||
| &:focus { | ||
| @include transition(border-bottom .1s ease-in-out); |
There was a problem hiding this comment.
The nittiest of nitpicks: I think this could be border-bottom-color, though I doubt that makes any real difference.
I've also been favoring milliseconds over decimals lately, based on something I saw in a presentation by someone... somewhere... the gist being "browsers think in milliseconds anyway, you might as well declare them explicitly and save them some work." So this could be 100ms instead of .1s
Demo