Closed ryelle closed 5 months ago
Looks good. I'll defer to @fcoveram who may have more precise metrics, but from my angle the buttons look a bit bigger than they need to be (height/padding). This may just be an optical illusion so feel free to ignore, but from the original designs, it's 40px buttons with 12px padding left and right. I'd also use the lightest gray we have for the hover style.
Here is the link to the button component's design spec used for each tab, in case the design documentation is not fully clear.
Do those interaction states work?
Yes. The only change is in hover, where background color changes to Light grey 2.
Based on the screenshots, I would say that All
tab is not necessary if clicking on it takes you to see the same in a different page. Am I correct with the interaction flow?
Here is the link to the button component's design spec used for each tab, in case the design documentation is not fully clear.
It's not clear… which button-grouping are you referring to? The link landed me on the whole zoomed out artboard.
Based on the screenshots, I would say that All tab is not necessary if clicking on it takes you to see the same in a different page.
In this case, "All" would take you back to the main (unfiltered) Archive page. Which, yes, we probably don't need since the "All patterns" link in the breadcrumbs is right there.
Updated the hover color:
It's not clear… which button-grouping are you referring to? The link landed me on the whole zoomed out artboard.
Oh sorry for the useless deep link. The component is named Button / Small
, here is a screenshot from the Figma file.
@fcoveram Do I need to update anything for this PR?
Based on the image attached, and if styles per state are according to the library, then everything is fine.
I struggled with technical issues when trying to run the PR in my local, and asking for help with that might take more time than needed to give a proper answer.
Okay, I'll merge this & you can check it with the theme preview.
See https://github.com/WordPress/pattern-directory/issues/635 — The category navigation uses a row of links, so a navigation block is appropriate for the markup, but the individual items use "button" styles for active (and presumably hover/focus, though not mocked up there).
I've added this to the parent theme for two reasons: I thought this pattern might pop up again, maybe in plugins, and I wanted to easily share the block mixin code.
Screenshots
In the first set of screenshots, "Featured" is in the hover state, and "Posts" is currently selected. In the second, we're on the homepage so no current selection, and "Featured" is in the focus state.
cc @WordPress/meta-design Do those interaction states work?
How to test the changes in this Pull Request: