Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a rounded corners block style to the Image and Gallery blocks #16949

Closed
wants to merge 3 commits into from

Conversation

kjellr
Copy link
Contributor

@kjellr kjellr commented Aug 7, 2019

This PR adds a simple "Rounded Corners" block style to the Image and Gallery blocks. This seems like something we might as well include, separately from the effort in #16130. I went with 8px rounded corners since those seemed visibly rounded at a glance.


Image Block

Screen Shot 2019-08-07 at 3 24 36 PM

Screen Shot 2019-08-07 at 3 25 29 PM


Gallery Block

Screen Shot 2019-08-07 at 3 24 28 PM

Screen Shot 2019-08-07 at 3 25 58 PM

@kjellr kjellr added [Type] Enhancement A suggestion for improvement. Needs Design Feedback Needs general design feedback. [Block] Image Affects the Image Block [Block] Gallery Affects the Gallery Block - used to display groups of images labels Aug 7, 2019
@kjellr kjellr requested review from mapk and jasmussen August 7, 2019 19:27
@kjellr kjellr self-assigned this Aug 7, 2019
@shaunandrews
Copy link
Contributor

shaunandrews commented Aug 7, 2019

It could be cool to have a drag handle somewhere to adjust the border radius. Then you could remove the need for multiple variations in the style picker.

Copy link
Contributor

@mapk mapk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The block itself and output work great!

I double checked the style variation previews and on the individual Image block, they're perfect.

Screen Shot 2019-08-07 at 2 57 44 PM

But on the Gallery block style variations, they looked a bit weird.

Screen Shot 2019-08-07 at 2 57 17 PM

I'm not sure how to get around that one because I believe the Preview just pulls the same styles as the block.

@jasmussen
Copy link
Contributor

😍

LOVE IT!

It could be cool to have a drag handle somewhere to adjust the border radius. Then you could remove the need for multiple variations in the style picker.

This actually echoes a challenge we have with adding features to the separator variations. The gist is that because themes can (and should if they'd like to) override the individual variations, whatever user-customizable style features we add in addition the variations should probably be generic to the block itself. I.e. if we were to add a border radius handle, it should probably be block level and not variation specific.

That brings me to a question to you Kjell: this 8px radius is easily overridable by themes, right? For instance if I had a theme where I use a small 4px radius for many buttons, I might want the rounded corner variation to have the same.

Mark:

But on the Gallery block style variations, they looked a bit weird.

The new preview that was JUST MERGED should fix this right up, I believe!

@karmatosed
Copy link
Member

This is great, really like it. I second the point it needs to be easy to override in theme, but as long as that is case :shipit: :)

@karmatosed karmatosed removed the Needs Design Feedback Needs general design feedback. label Aug 8, 2019
@mtias
Copy link
Member

mtias commented Aug 8, 2019

I do think this would be better as a block setting and not a variation. Variations work best when they can be thought of as booleans. A style variation that would make sense for "images" would be a grayscale filter, for example.

@kjellr
Copy link
Contributor Author

kjellr commented Aug 8, 2019

That brings me to a question to you Kjell: this 8px radius is easily overridable by themes, right? For instance if I had a theme where I use a small 4px radius for many buttons, I might want the rounded corner variation to have the same.

Yeah, it's just a class — so it's just as overridable as the button styles are.

I do think this would be better as a block setting and not a variation. Variations work best when they can be thought of as booleans. A style variation that would make sense for "images" would be a grayscale filter, for example.

I understand this, but I also wonder if it makes things too granular for users? Oh a high level, rounded corners feel like a "style" change, so the "Styles" panel seems like the right place to look for it.

The non-boolean aspect of this is the radius, right? And moving out of Block Styles would afford us the ability to let users fine-tune the rounded corner radius. But I'm not totally sure that's important to do at a per-block level. Like some of the recent separator block work, I wonder if those fine-tuned settings make more sense as global settings (once we have a place for them).

In any case, I've never been 100% clear on when to use Block Styles or not, so I'm all for developing some better guidelines for them — it seems like the previous guidance was largely just "Can it be done with only CSS? Then it can be a Block Style." 😄

@ChemicalSailor
Copy link

This PR seems like a good first step for adding rounded corners, and is something that I think will be a benefit to a great number of people. The adjustments part can come later.

Oh a high level, rounded corners feel like a "style" change, so the "Styles" panel seems like the right place to look for it.

I agree with this. Rounded corners are a style, just as grayscale, reflected and framed could be, and each of these styles could have their own adjustments. In fact any of these styles could be combined together. I'm hoping that at some point in the future the Block Styles API will support this.

I think limiting block styles to a simple boolean change is too restrictive, and developers will just extend on top of it anyway. The shape divider block is a good example of combining block styles and adjustments. On a generic level, how and if these adjustments are exposed will likely vary between blocks, and site owners may also want to only allow the theme defaults to be used.

@kjellr
Copy link
Contributor Author

kjellr commented Sep 24, 2019

Based on the 👍 reactions above, it seems like folks would prefer this be a setting and not just a block style. I'll close this for now.

@kjellr kjellr closed this Sep 24, 2019
@aristath aristath deleted the add/rounded-corners-block-style branch November 10, 2020 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Gallery Affects the Gallery Block - used to display groups of images [Block] Image Affects the Image Block [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants