Enable more style options for the Featured Product block #6181
Conversation
rubikuserbot
requested review from
a team and
albarin
and removed request for
a team
|
Size Change: +999 B (0%) Total Size: 862 kB
ℹ️ View Unchanged
|
sunyatasattva
force-pushed
the
feature/featured-product-block-styles
branch
from
644e914 to
7b87381
Compare
sunyatasattva
added
type: enhancement
|
@sunyatasattva While testing and comparing with Cover, I've noticed a couple of things:
Screen.Capture.on.2022-04-06.at.11-19-15.mp4
Screen.Capture.on.2022-04-06.at.11-17-32.mp4 |
| }, | ||
| spacing: { | ||
| padding: true, | ||
| __experimentalSkipSerialization: true, |
There was a problem hiding this comment.
Given this is __experimental would we want the spacing to be wrapped in isFeaturePluginBuild() below?
There was a problem hiding this comment.
Looks like we should! Thanks for the catch, I'm still a bit confused with the whole __experimental thing.
|
👋 Heya! A few comments: As @albarin mentioned above, it would be nice to keep that overlay behaviour once the padding is implemented! The image won't fill the background by default with a smaller image. It's obviously not the best regarding image quality, but can we align with the cover block?
I noticed when you put a high number for padding on one side it can push the elements "off" the block. Here's a screenshot where I input the same values for the cover and the featured product block:
|
|
Thinking out loud here: For both the Featured Product/Category blocks we want to add more advanced media control options. From what I hear @vivialice saying is that these capabilities should match the options we have on the Cover block. Instead of reinventing the wheel, could we potentially import/use the actual In an ideal case would be neat if we didn't have to duplicate all the media-related functionality the Cover block already has. If we end up having to duplicate it, I'm wondering if we could achieve it in a way where we can reuse that functionality within Woo Blocks at least. It does seem like we could use it in a number of blocks if it "just works".
|
Yea, I noticed that. It's really nice and useful feature indeed. I tried to figure out how that's implemented but couldn't manage yet. I posted in #gutenberg to see if anybody can give me pointers. Otherwise I'll do some more research.
Nice catch! Will look into that!
Yep, I agree. I'll try my best. Just a clarification: padding is already implemented the same way as in the cover block. It just doesn't show the overlay.
I've got a little qualm here. My problem is not the image quality, but rather the behavior of the focal point which I mentioned in the OP. If you see in the cover block you can't really set the focal point of your image. You can just scroll it vertically, because the image is contained to the borders of the block (so the UI feels weird and the users can't achieve what they want). With my implementation, you can actually meaningfully change the focal point as the UI and the result actually match. focal-point-bug.movI think the solution here is to add an option for the user to constrain the image to the borders of the block (i.e. if it's too large, it will scale it down; otherwise, will scale it up). What are your thoughts?
Interesting! So, the cover block scales up with the padding, I see. Good catch!
Yea, I am trying to reuse as much logic as possible from the existing Gutenberg core APIs. Though pretty much none of them are documented, and I have to do some reverse engineering, if you look at the code, you can see a lot of stuff is imported and reused. The comments from @nerrad on the spreadsheet implied that we should look to move away from custom implementations, and that's what I am trying to do. Just note, re: media controls, that they are not part of this MVP PR, as mentioned in this comment by @Aljullu. |
|
So, I pushed a bunch of commits addressing the feedback I got. In the meantime, I also fixed other bugs I found along the way, I'll update the OP so that it's completely reflective of what this PR is currently doing. Regarding the single issues that have been brought up:
As @frontdevde is off for a while, @albarin the onus of approving this PR will be on you 😸 |
|
@sunyatasattva The Focal Point picker rectangle seems too big and there's nothing under the Dimensions section: Tried it also on the Cover block and the same happens on the Focal Point picker, also the Minimum height of cover input has the wrong height. |
|
Oh, I realize I forgot to mention a really important part of the feedback I had received: the padding indicators! I had a conversation with people over at Gutenberg, and I wanted to share with people who asked here and whoever comes across this PR. It turns out it's called BoxControlVisualizer and currently I have been heavily discouraged from using it. cf. this PR: WordPress/gutenberg#40057 where it is mentioned to be in an “unusable state”. I have been told heavily rework is going on there and we shouldn't touch it until the rework is done. I am following the above mentioned issue so I can see when things are moving. |
The container now resizes if, e.g., padding is increased, and can't be resized down with the handle to a height lower than it's minimum size determined by content and padding box.
Previously, `overflow: hidden` was needed because the height constraint and padding could push the content out of the container. Now this should not be possible.
sunyatasattva
force-pushed
the
feature/featured-product-block-styles
branch
from
433115b to
8f5ca39
Compare
Script Dependencies ReportThe
This comment was automatically generated by the |
This PR adds several style options to the Featured Product block. The current status of features supported and planned for each block can be seen in this spreadsheet.
This PR fixes the following problems:
Colorsection of the sidebar, while its opacity was controlled elsewhere. This PR consolidates the controls under theOverlaysection.This PR adds the following features:
Closes #6150
Screenshots
Notice how, thanks to the fixed focal point picker and how the padding was implemented, I can set my image on the top right and my text on the bottom left instead of being constricted to everything being centered.
Testing
Automated Tests
Manual Testing
How to test the changes in this Pull Request:
Overlaysection and change the overlay color to a gradient.padding-topof the block and notice that the content can't get pushed out of the container, but instead the container resizes.User Facing Testing
These are steps for user testing (where "user" is someone interacting with this change that is not editing any code).
❗ Note: Since this PR includes several changes, we agreed that it should have a multi-line changelog. I checked the automation script which creates the changelog, and it seems to be fine with that. There might be some formatting issues with newlines and such, so perhaps, release lead, you might have to adjust a few things before publishing.
Changelog