Fix issue with tertiary button hit areas#10888
Conversation
|
Whoops, was sure I'd reviewed this, but must have closed the tab before commenting. Anyway, I noticed a couple of things on mobile breakpoints that might need to be fixed. The save icon button is now misaligned with the 'saving' and 'saved' icons. Then there's also a rogue preview button that pops up momentarily when a non-draft is in the process of being saved, but I don't think it was caused by this PR: |
|
Good catch, taking a look. |
This addressed feedback in #10552 (comment). It makes the tertiary button style be born with the same hit area as a button, because it is intended to be used as such, just with a different styling.
fdbc5b5 to
0f7e464
Compare
|
Okay I believe I fixed it. Desktop:
Mobile:
|
tofumatt
left a comment
There was a problem hiding this comment.
Way better, nothing is worse than tiny button click targets. 👍
| &.is-tertiary { | ||
| color: theme(outlines); | ||
|
|
||
| // Matches default button in hit area. |
There was a problem hiding this comment.
Might be handy to document how you got these values, I'm guessing they're based on the size of these tertiary buttons and the gap between them and the default button's size?
There was a problem hiding this comment.
It's from
.What would be a good comment to point to that?
There was a problem hiding this comment.
Huh, so they're basically the same values (and should stay linked, right?)
Using/adding a variable so they don't go out of sync would be good; otherwise you can just reference the file name/line number directly (that GitHub link is fine, even).
Is the lack of 1px padding-bottom intentional?
There was a problem hiding this comment.
I'll look at adding a variable, or addressing the line number, or both.
Yes the lack of bottom padding is intentional, it's there for the other button styles because they have a thicker bottom border to imply bevel with topdown shadow, and the bottom padding does something to something there.
There was a problem hiding this comment.
I clarified the comment a bit, but decided to not variableize anything for now. Reason being, right now we're in that place where some styles are inherited from upstream WordPress styles, and it's a bit messy. We want to move to a place where things are more componentized, and that will require a refactor which should make this file much simpler.
* Fix issue with tertiary button hit areas This addressed feedback in WordPress#10552 (comment). It makes the tertiary button style be born with the same hit area as a button, because it is intended to be used as such, just with a different styling. * Address feedback. * Clarify comment.
This addressed feedback in #10552 (comment).
It makes the tertiary button style be born with the same hit area as a button, because it is intended to be used as such, just with a different styling.