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: Border radius feature to button and update button styles. #17253

Merged

Conversation

jorgefilipecosta
Copy link
Member

@jorgefilipecosta jorgefilipecosta commented Aug 29, 2019

Description

Closes: #16481

This PR refactors the button styles two contain two styles:
- Fill (default)
- Outline
It removes the squared style.
We add a new feature that allows changing the border-radius of a button.
We add a migration logic that migrates blocks with squared style into Fill style blocks with border-radius 0.

How has this been tested?

I checked the border-radius functionality works as expected.
I tested both styles work as expected.
I pasted the following code containing button blocks created with Gutenberg 6.4 into the code editor:

<!-- wp:button {"customBackgroundColor":"#a14b0e","customTextColor":"#13b87b"} -->
<div class="wp-block-button"><a class="wp-block-button__link has-text-color has-background" style="background-color:#a14b0e;color:#13b87b">xcxcx</a></div>
<!-- /wp:button -->

<!-- wp:button {"customBackgroundColor":"#c8651e","customTextColor":"#21b47e","className":"is-style-outline"} -->
<div class="wp-block-button is-style-outline"><a class="wp-block-button__link has-text-color has-background" style="background-color:#c8651e;color:#21b47e">xcxcx</a></div>
<!-- /wp:button -->

<!-- wp:button {"customBackgroundColor":"#aa5a20","customTextColor":"#1b9b6c","className":"is-style-squared"} -->
<div class="wp-block-button is-style-squared"><a class="wp-block-button__link has-text-color has-background" style="background-color:#aa5a20;color:#1b9b6c">xcxcx</a></div>
<!-- /wp:button -->

I verified there was no invalid blocks warning and the editor converted the squared style block (last one) into a Fill style block with a border-radius of 0.

@jorgefilipecosta jorgefilipecosta changed the title Add: Border radius button to button and update button styles. Add: Border radius feature to button and update button styles. Aug 30, 2019
@jorgefilipecosta jorgefilipecosta added [Block] Buttons Affects the Buttons Block [Type] Enhancement A suggestion for improvement. Needs Design Feedback Needs general design feedback. labels Aug 30, 2019
@karmatosed karmatosed self-requested a review August 31, 2019 17:42
Copy link
Member

@karmatosed karmatosed left a comment

Choose a reason for hiding this comment

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

This works for me as per the issue's agreed direction.

One thought I have would be that as an iteration to consider the order of the panel. For example on a smaller size screen (13 macbook), I can't see without scrolling the border radius. I wonder if putting the following order would be better:

  • Style
  • Border radius
  • Colors

That said, this isn't a blocker but more a suggested iteration.

@karmatosed karmatosed removed the Needs Design Feedback Needs general design feedback. label Aug 31, 2019
@@ -41,6 +41,99 @@ const blockAttributes = {
};

const deprecated = [
{
Copy link
Contributor

@talldan talldan Sep 2, 2019

Choose a reason for hiding this comment

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

It'd be great if a fixture could be added for this new deprecation:
https://github.com/WordPress/gutenberg/tree/master/packages/e2e-tests/fixtures/blocks

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, I was wondering if a deprecation is required. It seems as though the existing test fixture didn't fail.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @talldan, you are right the existing test fixture did not fail that was the reason we used the isEligible mechanism, but deprecation ensures we migrate the previous style to a new style with a border-radius 0.

Copy link
Member Author

Choose a reason for hiding this comment

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

A fixture was added as suggested 👍

@jorgefilipecosta jorgefilipecosta force-pushed the add/border-radius-feature-button-and-update-styles branch from fa0b9d5 to 54a06c4 Compare September 2, 2019 12:06
@jorgefilipecosta jorgefilipecosta force-pushed the add/border-radius-feature-button-and-update-styles branch from 54a06c4 to 8bd5e99 Compare September 2, 2019 13:35
@jorgefilipecosta jorgefilipecosta merged commit 79d2886 into master Sep 2, 2019
@jorgefilipecosta jorgefilipecosta deleted the add/border-radius-feature-button-and-update-styles branch September 2, 2019 14:49
@youknowriad youknowriad added this to the Gutenberg 6.5 milestone Sep 14, 2019
@youknowriad
Copy link
Contributor

Nice one here, good job @jorgefilipecosta

@courtneylt
Copy link

Is there any way to disable "Border Settings" at a code level? I do not want my users to have the ability to alter the border radius of buttons; I want them to be restricted to the styles that I provide.

@JoshuaDoshua
Copy link
Contributor

JoshuaDoshua commented Dec 4, 2019

@courtneylt I think that would be the case for most theme developers. We put a great deal of work into keeping the WordPress admin "unbreakable" with regards to the style guide we provide and options like these make it difficult to work with Gutenberg

@aandrewjoyce
Copy link

aandrewjoyce commented Oct 1, 2020

Wondering if there's any update on at least setting a default border radius, or disabling user-control features like font, border radius, etc. A CSS custom property (e.g. --gutenberg-default-button-radius would at least allow theme developers to set something sensible)

@jorgefilipecosta
Copy link
Member Author

Yes, I agree themes should have a way to set a sensible default value. As part of the global styles theme.json work I think that will soon be possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Buttons Affects the Buttons Block [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Simplify Button Block Styles
7 participants