Skip to content

Add: Border radius feature to button and update button styles.#17253

Merged
jorgefilipecosta merged 1 commit intomasterfrom
add/border-radius-feature-button-and-update-styles
Sep 2, 2019
Merged

Add: Border radius feature to button and update button styles.#17253
jorgefilipecosta merged 1 commit intomasterfrom
add/border-radius-feature-button-and-update-styles

Conversation

@jorgefilipecosta
Copy link
Copy Markdown
Member

@jorgefilipecosta jorgefilipecosta commented Aug 29, 2019 •
edited
Loading

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
@jorgefilipecosta jorgefilipecosta requested a review from mapk August 30, 2019 17:41
@karmatosed karmatosed self-requested a review August 31, 2019 17:42
karmatosed
karmatosed approved these changes Aug 31, 2019
View reviewed changes
Copy link
Copy Markdown
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
talldan
talldan reviewed Sep 2, 2019
View reviewed changes
Comment thread packages/block-library/src/button/deprecated.js
};

const deprecated = [
{
Copy link
Copy Markdown
Contributor

@talldan talldan Sep 2, 2019 •
edited
Loading

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
Copy Markdown
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.

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

Copy link
Copy Markdown
Member Author

@jorgefilipecosta jorgefilipecosta Sep 2, 2019

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
Copy Markdown
Member Author

@jorgefilipecosta jorgefilipecosta Sep 2, 2019

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
Copy Markdown
Contributor

youknowriad commented Sep 14, 2019

Nice one here, good job @jorgefilipecosta

@courtneylt
Copy link
Copy Markdown

courtneylt commented Nov 27, 2019

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
Copy Markdown
Contributor

JoshuaDoshua commented Dec 4, 2019 •
edited
Loading

@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

@talldan talldan mentioned this pull request Feb 4, 2020
Closed
@aandrewjoyce
Copy link
Copy Markdown

aandrewjoyce commented Oct 1, 2020 •
edited
Loading

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
Copy Markdown
Member Author

jorgefilipecosta commented Oct 12, 2020

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.

@ryanAllMad ryanAllMad mentioned this pull request May 18, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@talldan talldan talldan left review comments

@karmatosed karmatosed karmatosed approved these changes

@ajitbohra ajitbohra Awaiting requested review from ajitbohra

@Soean Soean Awaiting requested review from Soean

@mapk mapk Awaiting requested review from mapk

@gziolo gziolo Awaiting requested review from gziolo

@nerrad nerrad Awaiting requested review from nerrad

@ntwb ntwb Awaiting requested review from ntwb

Assignees

No one assigned

Labels

[Block] Buttons Affects the Buttons Block [Type] Enhancement A suggestion for improvement.

Projects

None yet

Milestone

Gutenberg 6.5

Development

Successfully merging this pull request may close these issues.

Simplify Button Block Styles

7 participants

@jorgefilipecosta @youknowriad @courtneylt @JoshuaDoshua @aandrewjoyce @karmatosed @talldan