-
Notifications
You must be signed in to change notification settings - Fork 219
Add to Featured Product block option to remove custom image #5886
Add to Featured Product block option to remove custom image #5886
Conversation
removing custom image will reset it back to the default product image if available
Size Change: +1.24 kB (0%) Total Size: 817 kB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tomasztunik This is working great on my end. I have two minor issues, otherwise, this is looking good to me.
- For the button label, I think
Reset
is better to describe the function of that button thanRemove
. When users click that button, if the product has a featured image, the button doesn't really "remove" the image from the users' perspective. If the label of the button isRemove custom featured image
, then usingRemove
makes sense, but it'd be too long. - For the buttons group containing only icons, we don't need a separator between buttons, but for buttons with text labels, I think a separator should be used to distinguish buttons.
Valid points @dinhtungdu. Tried to just implement existing flow but maybe we could improve this and Featured Category blocks. With the separator I think it wasn't added there to highlight that these two actions are related to the same background image selector context. Not sure what is the definitive way to go here. In general I feel like the dominating "Replace" button especially if category on Featured Category block or Product is selected these seem to refer to the context of a block so feature or product rather than the image. Maybe we should use icons there? Went with undo as trash felt too destructive and "remove" like a stop sign :) VS current with text labels @vivialice what do you think? Should we stick to current solution? go with icons? else? (Used Featured Category block as ref as I had it at hand, but they follow the same patterns) |
went with just Reset as it is commonly used in WordPress and will have translation available
Updated with "Reset" label and retained no buttons separation to keep the context of working with background media. I've seen this mentioned elsewhere that it would be nice if these were more unified with Cover block and I think any bigger changes for Categories and Product blocks should probably include this consideration. The UI on editing cover block is more clear and more user friendly and provides more options in the sidebar than we have available here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@tomasztunik could you create a good-first-issue (or directly a PR) to make sure the button label is aligned between both blocks? Currently it's Reset in Featured Product but Remove in Featured Category, ideally I think they should be the same. Do you agree? |
@Aljullu was planning to but the alignment issue got fixed by #5867 already! Regarding the reset/remove - here it resets to product image - so we still have an image, there it removes the image completely and uses a background color as selected with styles. Should we unify these despite slightly different behaviour? |
Does it? It seems to reset the image to whatever the category image is for me: Kooha-02-23-2022-12-02-52.mp4(Notice that if the category doesn't have an image, then yes, a background color is used) |
Ah I see, had a category without image when testing. Will update it to reflect that if image is present it will say reset else remove? or always reset? |
IMO always reset is good enough. To me, it makes sense in both contexts. |
* adds toolbar option to remove custom image removing custom image will reset it back to the default product image if available * update copy as per discussion went with just Reset as it is commonly used in WordPress and will have translation available
This adds ability to remove custom image from Featured Product block. Removing custom image will reset it back to the default product image if available. This follows solution from #5719 which added this option to Featured Category block.
Fixes #5725
Accessibility
UPDATE: "Remove" label replaced with "Reset"
Screenshots
Manual Testing
How to test the changes in this Pull Request:
User Facing Testing
These are steps for user testing (where "user" is someone interacting with this change that is not editing any code).
Note
misaligned "Shop Now" is an unrelated issue
Changelog