Closed ryelle closed 4 months ago
Looks good, and is, I believe, where consensus arrived. 👍 👍
Looking at @adamwoodnz's video, that button state flashes black. If that's the case, can we prevent that?
that button state flashes black. If that's the case, can we prevent that?
That's the active state, it was designed that way for all buttons. Here are two other places this happens (the first two I happened to grab in my recording view):
https://github.com/WordPress/wporg-mu-plugins/assets/541093/7f6ecaae-2bc2-46df-8416-ecc9332d9788
although my existing count data (from repo setup) got cleared, not sure why:
It re-tallies all the users who have a pattern favorited when updating the count, so when you update it locally, the count goes back to 1 as you're the only user on the local site to favorite it.
That's the active state, it was designed that way for all buttons. Here are two other places this happens (the first two I happened to grab in my recording view):
I understand that but it feels distracting in this case. Looking at Adam's video, the third button doesn't have that flash (is that a video problem?) and it feels like a better experience.
Looking at Adam's video, the third button doesn't have that flash (is that a video problem?) and it feels like a better experience.
It's probably a faster click, the active state only appears as long as you're holding the mouse button down. Is the active state a blocker for this PR? If so, we'll probably need a different active state from design.
I'm still planning on addressing @adamwoodnz's feedback, I appreciate the suggestions!
I'd tend to agree, that active state is a bit curious. I wonder: do we actually need one? CC: @fcoveram as I think you have past knowledge on buttons.
I've been adding the active state to almost all new components created in the Design Library to follow what was already implemented. I'm unaware of any principle we're attached to or any discussions on pros and cons.
I'd tend to agree, that active state is a bit curious. I wonder: do we actually need one?
I believe they're useful on mobile where there is no hover state, so active works to indicate what you tapped. I've also seen that it can be helpful for users with mobility issues, to make it clear when the click is happening. That said, we could probably make it less dramatic, either here or in general.
I've merged this to move forward, but if we want to update anything (like the active state), let's continue that in a new issue.
The favorite button is used across the pattern, theme, and plugin directories. This PR combines the Favorite block from the Theme Directory & Pattern Directory to create a shared component that can be used across all 3 sites.
See https://github.com/WordPress/wporg-mu-plugins/discussions/617
To solve the issue of each directory having their own favoriting method, this block adds a simple API endpoint to create/delete, which calls callbacks that the child themes can set up to handle the favorite process. If they're not set up, the API errors, but doesn't do anything visually. The callbacks receive the current post ID & request object, and can return true (success), a WP_Error (failure), or the new count of favorites (also a success).
This uses the same filter process as our other blocks (Query Filters, Ratings, etc) to set up configuration, including the callbacks. Child themes should filter
wporg_favorite_button_settings
, and return:add_callback
.The
count
can be omitted ifshowCount
is not used.Attributes
showCount
, default false Controls whether the favorite count is visible. If this is used, the callback should return the favorite count, and settings should includecount
.variant
The block has two variants,default
(outlined button, on single patterns & themes) andsmall
(no outline, small padding, on patterns grid).See https://github.com/WordPress/wporg-theme-directory/compare/try/mu-plugins-favorite-button, https://github.com/WordPress/pattern-directory/compare/try/mu-plugins-favorite-button for examples in use.
Screenshots
Themes (no visual changes)
Patterns (keeping the count)
To test:
It's probably easiest to test with one of the other repos, I've set up the new block in themes and patterns in the following branches:
There are site-specific test instructions in the above PRs ^