Closed sarahmonster closed 4 years ago
Oh, regarding navigationDirection
… I want to make it clear that the direction is for the icon, so let's call it navigationIcon
and just allow for "left" | "right" | "up" | "down"
as props. I think that's fine!
Also: I like the bigger icon sizes! :shipit:
Got distracted and decided to make our semantic buttons a bit more semantic, using shape to underscore their meaning:
Okay! They're still not perfect, but I think a lot better. Good catch with the focus styles, there were a few things going on there that would prove to be problematic that I've patched.
We'll probably want to do a little bit of housekeeping with the interactive tokens (how did I not know these were here!?) in the future to make it more themeable and flexible, but I think that'll be cause for another, larger PR—and easier to tell what we really need once we've had a chance to work with the icon buttons a bit more!
Ready for review, finally! It only took me over a month. 😜
There are a number of changes here, so good to take it for a spin and really test it out to get a feel for how it works in practise.
Here are the major changes:
navigationDirection
instead? (And have back/forward/up/down as options?)Before:
After:
Things that we may want to do prior to pushing:
<Button navigation trailingIcon={null}
but I might have the syntax wrong?)Fix #216.