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

Fix: Button border radius 0 does not appear, and undefined border appears as 0 #22393

Merged
merged 2 commits into from
May 19, 2020

Conversation

jorgefilipecosta
Copy link
Member

Fixes: #22367
This PR fixes a wrong CSS rule that made the border 0 radius in button not appear as expected.

It also fixes a bug, when the border-radius is not set (it is undefined and the theme value set with CSS is being used) we were displaying 0 as the border-radius by default, in that case, we should show the input empty.

How has this been tested?

I enabled 2019 theme.
I added a button I set the border-radius to 0 and verified it appeared as expected on the editor and on the front end.
I added a button in 2019 (by default the button has some border) I verified the border did not appear as 0.

@jorgefilipecosta jorgefilipecosta added [Type] Bug An existing feature does not function as intended [Block] Buttons Affects the Buttons Block labels May 15, 2020
@@ -43,7 +43,7 @@ $blocks-button__height: 56px;
// the first selector is required for old buttons markup

.wp-block-button.no-border-radius,
.wp-block-button__link.wp-block-button.no-border-radius {
.wp-block-button .wp-block-button__link.no-border-radius {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we remove .wp-block-button here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Suggestion applied :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Suggestion applied :)

@youknowriad
Copy link
Contributor

@ItsJonQ this seems a bit related to the "reset" behaviors we were discussing?

@github-actions
Copy link

github-actions bot commented May 15, 2020

Size Change: +275 kB (24%) 🚨

Total Size: 1.11 MB

Filename Size Change
build/annotations/index.js 3.62 kB +2 B (0%)
build/block-editor/index.js 105 kB +73 B (0%)
build/block-library/editor-rtl.css 7.22 kB +31 B (0%)
build/block-library/editor.css 7.22 kB +31 B (0%)
build/block-library/index.js 119 kB +496 B (0%)
build/block-library/style-rtl.css 7.48 kB +1 B
build/block-library/style.css 7.48 kB -1 B
build/blocks/index.js 48.1 kB +1 B
build/components/index.js 182 kB +122 B (0%)
build/components/style-rtl.css 17.1 kB +46 B (0%)
build/components/style.css 17.1 kB +45 B (0%)
build/compose/index.js 6.68 kB +3 B (0%)
build/core-data/index.js 11.4 kB +3 B (0%)
build/data-controls/index.js 1.29 kB -2 B (0%)
build/data/index.js 8.42 kB -4 B (0%)
build/dom-ready/index.js 568 B -1 B
build/edit-navigation/index.js 6.6 kB +4 B (0%)
build/edit-post/index.js 302 kB +274 kB (90%) 🆘
build/edit-post/style-rtl.css 12.2 kB -31 B (0%)
build/edit-post/style.css 12.2 kB -31 B (0%)
build/edit-site/index.js 12.8 kB +757 B (5%) 🔍
build/edit-widgets/index.js 7.73 kB -138 B (1%)
build/edit-widgets/style-rtl.css 4.59 kB -102 B (2%)
build/edit-widgets/style.css 4.59 kB -102 B (2%)
build/editor/index.js 44.3 kB +6 B (0%)
build/element/index.js 4.65 kB -2 B (0%)
build/hooks/index.js 2.13 kB +1 B
build/keyboard-shortcuts/index.js 2.51 kB +1 B
build/keycodes/index.js 1.94 kB +2 B (0%)
build/list-reusable-blocks/index.js 3.13 kB -3 B (0%)
build/media-utils/index.js 5.29 kB +3 B (0%)
build/notices/index.js 1.79 kB +1 B
build/nux/index.js 3.4 kB +1 B
build/plugins/index.js 2.56 kB -2 B (0%)
build/primitives/index.js 1.5 kB +1 B
build/redux-routine/index.js 2.85 kB +1 B
build/rich-text/index.js 14.8 kB +5 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.02 kB 0 B
build/api-fetch/index.js 3.39 kB 0 B
build/autop/index.js 2.83 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.93 kB 0 B
build/block-directory/style-rtl.css 790 B 0 B
build/block-directory/style.css 791 B 0 B
build/block-editor/style-rtl.css 10.8 kB 0 B
build/block-editor/style.css 10.8 kB 0 B
build/block-library/theme-rtl.css 683 B 0 B
build/block-library/theme.css 685 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom/index.js 3.11 kB 0 B
build/edit-navigation/style-rtl.css 857 B 0 B
build/edit-navigation/style.css 856 B 0 B
build/edit-site/style-rtl.css 5.22 kB 0 B
build/edit-site/style.css 5.22 kB 0 B
build/editor/editor-styles-rtl.css 425 B 0 B
build/editor/editor-styles.css 428 B 0 B
build/editor/style-rtl.css 5.07 kB 0 B
build/editor/style.css 5.08 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.64 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 711 B 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/priority-queue/index.js 789 B 0 B
build/server-side-render/index.js 2.68 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.02 kB 0 B
build/viewport/index.js 1.84 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@jorgefilipecosta jorgefilipecosta force-pushed the fix/button-border-radius branch from 32d8d10 to dc4607c Compare May 18, 2020 19:41
@ItsJonQ
Copy link

ItsJonQ commented May 19, 2020

@jorgefilipecosta Ah! Thanks for catching this! I spotted a bug in the guard for the floatClamp util, which should tidy it up a bit.

Before:

	if ( ! isFinite( value ) ) {
		return null;
	}

After:

	if ( typeof value !== 'number' ) {
		return null;
	}

This should handle "" values. I made that update on my end, and it looks like it's working.
I'll push it up. Would you be able to confirm?

Thank you! 🙏

@jorgefilipecosta jorgefilipecosta merged commit 2b41fbc into master May 19, 2020
@jorgefilipecosta jorgefilipecosta deleted the fix/button-border-radius branch May 19, 2020 16:55
@jorgefilipecosta
Copy link
Member Author

Thank you for the enhancement @ItsJonQ. It worked great on my tests 👍

@github-actions github-actions bot added this to the Gutenberg 8.2 milestone May 19, 2020
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] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Border radius 0 in button block not working
3 participants