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

Button: Adopt support for border color, style, and width #44574

Merged
merged 5 commits into from
Feb 7, 2023

Conversation

aaronrobertshaw
Copy link
Contributor

What?

Adopts border color, style, and width, supports for the Button block.

Why?

  • Provides greater control over button borders
  • Allows any borders configured via theme.json to be tweaked via Global Styles.

How?

  • Opts into color, style, and width, support via the Button block.json

Testing Instructions

  1. Add some button styles via your theme.json blocks section. (See snippet below)
  2. Load the editor add a Buttons block and a few buttons within it. They should reflect the styles in your theme.json
  3. Save the post and switch to the site editor
  4. Navigate to Global Styles > Blocks > Button > Layout and adjust the border there.
  5. Switch back to your post, the buttons should now reflect the Global Styles values.
  6. Select a button block and configure that individually via the inspector controls sidebar.
  7. Ensure all the styles are applied correctly on the frontend.

Example theme.json snippet:

	"styles": {
		"blocks": {
			"core/button": {
				"border": {
					"radius": "0",
					"color": "fuchsia",
					"style": "solid",
					"width": "10px"
				},
			}
		}
	}

Screenshots or screencast

Screen.Recording.2022-09-29.at.8.52.17.pm.mp4

@aaronrobertshaw aaronrobertshaw added [Type] Enhancement A suggestion for improvement. [Block] Buttons Affects the Buttons Block [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi labels Sep 29, 2022
@aaronrobertshaw aaronrobertshaw self-assigned this Sep 29, 2022
@@ -79,3 +79,40 @@ div[data-type="core/button"] {
.editor-styles-wrapper .wp-block-button[style*="text-decoration"] .wp-block-button__link {
text-decoration: inherit;
}

.editor-styles-wrapper .wp-block-button .wp-block-button__link {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not a huge fan of having to add this much CSS to provide defaults that can overcome styles in core. If anyone has any ideas on a better approach, I'd be happy to evolve this one.

Without these tweaks the user has to set multiple fields before a border shows up. Even now the default border color is currentColor so when the background is white and the button text is white, the border isn't really visible.

Copy link
Contributor

@glendaviesnz glendaviesnz Feb 7, 2023

Choose a reason for hiding this comment

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

It doesn't seem like we need to replicate this in editor.scss and styles.scss - if I removed this section from here the CSS from styles.scss seemed to be applied in the editor and the frontend? I did a fresh build and a hard reload to make sure it wasn't just a case of the editor styles being cached.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do need to add this CSS to overcome core's theme.json button element styles which reset border-width to 0.

The theme.json styles get the .editor-styles-wrapper class prefixed to their selectors in the editor. As a result, those styles override the styles in the frontend style.scss.

Without these styles, a user needs to adjust the border width before they can see any border.

To witness this behaviour:

  1. Delete or disable the styles here in the editor.scss file
  2. Make sure you don't have any core/button styles in your theme.json
  3. Edit a post and add a button
  4. Select the button and select only a border color
  5. Note that there is no visible border due to the core theme.json border width reset for button elements.
Screen.Recording.2023-02-07.at.3.13.43.pm.mp4

Copy link
Contributor

@glendaviesnz glendaviesnz Feb 7, 2023

Choose a reason for hiding this comment

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

Thanks for the extra detail - I was adding the border width and then the color so wasn't picking this up - if I had read back through the old comments I would have picked all that up - sorry for wasting your time here!

I have modified the comment to specify this detail like the one in style.scss does.

@github-actions
Copy link

github-actions bot commented Sep 29, 2022

Size Change: +1.83 kB (0%)

Total Size: 1.32 MB

Filename Size Change
build/block-editor/index.min.js 192 kB -234 B (0%)
build/block-editor/style-rtl.css 14.4 kB +58 B (0%)
build/block-editor/style.css 14.4 kB +62 B (0%)
build/block-library/blocks/button/editor-rtl.css 587 B +102 B (+21%) 🚨
build/block-library/blocks/button/editor.css 587 B +102 B (+21%) 🚨
build/block-library/blocks/button/style-rtl.css 628 B +96 B (+18%) ⚠️
build/block-library/blocks/button/style.css 627 B +95 B (+18%) ⚠️
build/block-library/blocks/image/editor-rtl.css 830 B +1 B (0%)
build/block-library/blocks/image/editor.css 829 B +1 B (0%)
build/block-library/blocks/image/style-rtl.css 646 B +19 B (+3%)
build/block-library/blocks/image/style.css 652 B +22 B (+3%)
build/block-library/editor-rtl.css 11.6 kB +96 B (+1%)
build/block-library/editor.css 11.6 kB +97 B (+1%)
build/block-library/index.min.js 199 kB -73 B (0%)
build/block-library/style-rtl.css 12.6 kB +111 B (+1%)
build/block-library/style.css 12.6 kB +111 B (+1%)
build/components/index.min.js 204 kB +560 B (0%)
build/data/index.min.js 8.56 kB +504 B (+6%) 🔍
build/edit-site/index.min.js 64.2 kB +83 B (0%)
build/edit-site/style-rtl.css 9.68 kB -1 B (0%)
build/edit-site/style.css 9.67 kB -1 B (0%)
build/editor/index.min.js 45.4 kB -14 B (0%)
build/experiments/index.min.js 938 B +33 B (+4%)
ℹ️ View Unchanged
Filename Size
build/a11y/index.min.js 993 B
build/annotations/index.min.js 2.78 kB
build/api-fetch/index.min.js 2.27 kB
build/autop/index.min.js 2.15 kB
build/blob/index.min.js 483 B
build/block-directory/index.min.js 7.2 kB
build/block-directory/style-rtl.css 1.04 kB
build/block-directory/style.css 1.04 kB
build/block-editor/content-rtl.css 4.11 kB
build/block-editor/content.css 4.1 kB
build/block-editor/default-editor-styles-rtl.css 403 B
build/block-editor/default-editor-styles.css 403 B
build/block-library/blocks/archives/editor-rtl.css 61 B
build/block-library/blocks/archives/editor.css 60 B
build/block-library/blocks/archives/style-rtl.css 90 B
build/block-library/blocks/archives/style.css 90 B
build/block-library/blocks/audio/editor-rtl.css 150 B
build/block-library/blocks/audio/editor.css 150 B
build/block-library/blocks/audio/style-rtl.css 122 B
build/block-library/blocks/audio/style.css 122 B
build/block-library/blocks/audio/theme-rtl.css 138 B
build/block-library/blocks/audio/theme.css 138 B
build/block-library/blocks/avatar/editor-rtl.css 116 B
build/block-library/blocks/avatar/editor.css 116 B
build/block-library/blocks/avatar/style-rtl.css 91 B
build/block-library/blocks/avatar/style.css 91 B
build/block-library/blocks/block/editor-rtl.css 305 B
build/block-library/blocks/block/editor.css 305 B
build/block-library/blocks/buttons/editor-rtl.css 337 B
build/block-library/blocks/buttons/editor.css 337 B
build/block-library/blocks/buttons/style-rtl.css 332 B
build/block-library/blocks/buttons/style.css 332 B
build/block-library/blocks/calendar/style-rtl.css 239 B
build/block-library/blocks/calendar/style.css 239 B
build/block-library/blocks/categories/editor-rtl.css 84 B
build/block-library/blocks/categories/editor.css 83 B
build/block-library/blocks/categories/style-rtl.css 100 B
build/block-library/blocks/categories/style.css 100 B
build/block-library/blocks/code/editor-rtl.css 53 B
build/block-library/blocks/code/editor.css 53 B
build/block-library/blocks/code/style-rtl.css 121 B
build/block-library/blocks/code/style.css 121 B
build/block-library/blocks/code/theme-rtl.css 124 B
build/block-library/blocks/code/theme.css 124 B
build/block-library/blocks/columns/editor-rtl.css 108 B
build/block-library/blocks/columns/editor.css 108 B
build/block-library/blocks/columns/style-rtl.css 406 B
build/block-library/blocks/columns/style.css 406 B
build/block-library/blocks/comment-author-avatar/editor-rtl.css 125 B
build/block-library/blocks/comment-author-avatar/editor.css 125 B
build/block-library/blocks/comment-content/style-rtl.css 92 B
build/block-library/blocks/comment-content/style.css 92 B
build/block-library/blocks/comment-template/style-rtl.css 199 B
build/block-library/blocks/comment-template/style.css 198 B
build/block-library/blocks/comments-pagination-numbers/editor-rtl.css 123 B
build/block-library/blocks/comments-pagination-numbers/editor.css 121 B
build/block-library/blocks/comments-pagination/editor-rtl.css 222 B
build/block-library/blocks/comments-pagination/editor.css 209 B
build/block-library/blocks/comments-pagination/style-rtl.css 235 B
build/block-library/blocks/comments-pagination/style.css 231 B
build/block-library/blocks/comments-title/editor-rtl.css 75 B
build/block-library/blocks/comments-title/editor.css 75 B
build/block-library/blocks/comments/editor-rtl.css 840 B
build/block-library/blocks/comments/editor.css 839 B
build/block-library/blocks/comments/style-rtl.css 637 B
build/block-library/blocks/comments/style.css 636 B
build/block-library/blocks/cover/editor-rtl.css 612 B
build/block-library/blocks/cover/editor.css 613 B
build/block-library/blocks/cover/style-rtl.css 1.57 kB
build/block-library/blocks/cover/style.css 1.56 kB
build/block-library/blocks/embed/editor-rtl.css 293 B
build/block-library/blocks/embed/editor.css 293 B
build/block-library/blocks/embed/style-rtl.css 410 B
build/block-library/blocks/embed/style.css 410 B
build/block-library/blocks/embed/theme-rtl.css 138 B
build/block-library/blocks/embed/theme.css 138 B
build/block-library/blocks/file/editor-rtl.css 300 B
build/block-library/blocks/file/editor.css 300 B
build/block-library/blocks/file/style-rtl.css 253 B
build/block-library/blocks/file/style.css 254 B
build/block-library/blocks/file/view.min.js 353 B
build/block-library/blocks/freeform/editor-rtl.css 2.44 kB
build/block-library/blocks/freeform/editor.css 2.44 kB
build/block-library/blocks/gallery/editor-rtl.css 984 B
build/block-library/blocks/gallery/editor.css 988 B
build/block-library/blocks/gallery/style-rtl.css 1.55 kB
build/block-library/blocks/gallery/style.css 1.55 kB
build/block-library/blocks/gallery/theme-rtl.css 122 B
build/block-library/blocks/gallery/theme.css 122 B
build/block-library/blocks/group/editor-rtl.css 654 B
build/block-library/blocks/group/editor.css 654 B
build/block-library/blocks/group/style-rtl.css 57 B
build/block-library/blocks/group/style.css 57 B
build/block-library/blocks/group/theme-rtl.css 78 B
build/block-library/blocks/group/theme.css 78 B
build/block-library/blocks/heading/style-rtl.css 76 B
build/block-library/blocks/heading/style.css 76 B
build/block-library/blocks/html/editor-rtl.css 332 B
build/block-library/blocks/html/editor.css 333 B
build/block-library/blocks/image/theme-rtl.css 137 B
build/block-library/blocks/image/theme.css 137 B
build/block-library/blocks/latest-comments/style-rtl.css 298 B
build/block-library/blocks/latest-comments/style.css 298 B
build/block-library/blocks/latest-posts/editor-rtl.css 213 B
build/block-library/blocks/latest-posts/editor.css 212 B
build/block-library/blocks/latest-posts/style-rtl.css 478 B
build/block-library/blocks/latest-posts/style.css 478 B
build/block-library/blocks/list/style-rtl.css 88 B
build/block-library/blocks/list/style.css 88 B
build/block-library/blocks/media-text/editor-rtl.css 266 B
build/block-library/blocks/media-text/editor.css 263 B
build/block-library/blocks/media-text/style-rtl.css 507 B
build/block-library/blocks/media-text/style.css 505 B
build/block-library/blocks/more/editor-rtl.css 431 B
build/block-library/blocks/more/editor.css 431 B
build/block-library/blocks/navigation-link/editor-rtl.css 716 B
build/block-library/blocks/navigation-link/editor.css 715 B
build/block-library/blocks/navigation-link/style-rtl.css 115 B
build/block-library/blocks/navigation-link/style.css 115 B
build/block-library/blocks/navigation-submenu/editor-rtl.css 299 B
build/block-library/blocks/navigation-submenu/editor.css 299 B
build/block-library/blocks/navigation/editor-rtl.css 2.13 kB
build/block-library/blocks/navigation/editor.css 2.14 kB
build/block-library/blocks/navigation/style-rtl.css 2.22 kB
build/block-library/blocks/navigation/style.css 2.2 kB
build/block-library/blocks/navigation/view-modal.min.js 2.81 kB
build/block-library/blocks/navigation/view.min.js 447 B
build/block-library/blocks/nextpage/editor-rtl.css 395 B
build/block-library/blocks/nextpage/editor.css 395 B
build/block-library/blocks/page-list/editor-rtl.css 376 B
build/block-library/blocks/page-list/editor.css 376 B
build/block-library/blocks/page-list/style-rtl.css 175 B
build/block-library/blocks/page-list/style.css 175 B
build/block-library/blocks/paragraph/editor-rtl.css 174 B
build/block-library/blocks/paragraph/editor.css 174 B
build/block-library/blocks/paragraph/style-rtl.css 279 B
build/block-library/blocks/paragraph/style.css 281 B
build/block-library/blocks/post-author/style-rtl.css 175 B
build/block-library/blocks/post-author/style.css 176 B
build/block-library/blocks/post-comments-form/editor-rtl.css 96 B
build/block-library/blocks/post-comments-form/editor.css 96 B
build/block-library/blocks/post-comments-form/style-rtl.css 501 B
build/block-library/blocks/post-comments-form/style.css 501 B
build/block-library/blocks/post-date/style-rtl.css 61 B
build/block-library/blocks/post-date/style.css 61 B
build/block-library/blocks/post-excerpt/editor-rtl.css 73 B
build/block-library/blocks/post-excerpt/editor.css 73 B
build/block-library/blocks/post-excerpt/style-rtl.css 134 B
build/block-library/blocks/post-excerpt/style.css 134 B
build/block-library/blocks/post-featured-image/editor-rtl.css 586 B
build/block-library/blocks/post-featured-image/editor.css 584 B
build/block-library/blocks/post-featured-image/style-rtl.css 318 B
build/block-library/blocks/post-featured-image/style.css 318 B
build/block-library/blocks/post-navigation-link/style-rtl.css 153 B
build/block-library/blocks/post-navigation-link/style.css 153 B
build/block-library/blocks/post-template/editor-rtl.css 99 B
build/block-library/blocks/post-template/editor.css 98 B
build/block-library/blocks/post-template/style-rtl.css 282 B
build/block-library/blocks/post-template/style.css 282 B
build/block-library/blocks/post-terms/style-rtl.css 96 B
build/block-library/blocks/post-terms/style.css 96 B
build/block-library/blocks/post-title/style-rtl.css 100 B
build/block-library/blocks/post-title/style.css 100 B
build/block-library/blocks/preformatted/style-rtl.css 103 B
build/block-library/blocks/preformatted/style.css 103 B
build/block-library/blocks/pullquote/editor-rtl.css 135 B
build/block-library/blocks/pullquote/editor.css 135 B
build/block-library/blocks/pullquote/style-rtl.css 326 B
build/block-library/blocks/pullquote/style.css 325 B
build/block-library/blocks/pullquote/theme-rtl.css 167 B
build/block-library/blocks/pullquote/theme.css 167 B
build/block-library/blocks/query-pagination-numbers/editor-rtl.css 122 B
build/block-library/blocks/query-pagination-numbers/editor.css 121 B
build/block-library/blocks/query-pagination/editor-rtl.css 221 B
build/block-library/blocks/query-pagination/editor.css 211 B
build/block-library/blocks/query-pagination/style-rtl.css 288 B
build/block-library/blocks/query-pagination/style.css 284 B
build/block-library/blocks/query-title/style-rtl.css 63 B
build/block-library/blocks/query-title/style.css 63 B
build/block-library/blocks/query/editor-rtl.css 458 B
build/block-library/blocks/query/editor.css 457 B
build/block-library/blocks/quote/style-rtl.css 213 B
build/block-library/blocks/quote/style.css 213 B
build/block-library/blocks/quote/theme-rtl.css 223 B
build/block-library/blocks/quote/theme.css 226 B
build/block-library/blocks/read-more/style-rtl.css 132 B
build/block-library/blocks/read-more/style.css 132 B
build/block-library/blocks/rss/editor-rtl.css 149 B
build/block-library/blocks/rss/editor.css 149 B
build/block-library/blocks/rss/style-rtl.css 289 B
build/block-library/blocks/rss/style.css 288 B
build/block-library/blocks/search/editor-rtl.css 165 B
build/block-library/blocks/search/editor.css 165 B
build/block-library/blocks/search/style-rtl.css 409 B
build/block-library/blocks/search/style.css 406 B
build/block-library/blocks/search/theme-rtl.css 114 B
build/block-library/blocks/search/theme.css 114 B
build/block-library/blocks/separator/editor-rtl.css 146 B
build/block-library/blocks/separator/editor.css 146 B
build/block-library/blocks/separator/style-rtl.css 234 B
build/block-library/blocks/separator/style.css 234 B
build/block-library/blocks/separator/theme-rtl.css 194 B
build/block-library/blocks/separator/theme.css 194 B
build/block-library/blocks/shortcode/editor-rtl.css 474 B
build/block-library/blocks/shortcode/editor.css 474 B
build/block-library/blocks/site-logo/editor-rtl.css 490 B
build/block-library/blocks/site-logo/editor.css 490 B
build/block-library/blocks/site-logo/style-rtl.css 203 B
build/block-library/blocks/site-logo/style.css 203 B
build/block-library/blocks/site-tagline/editor-rtl.css 86 B
build/block-library/blocks/site-tagline/editor.css 86 B
build/block-library/blocks/site-title/editor-rtl.css 116 B
build/block-library/blocks/site-title/editor.css 116 B
build/block-library/blocks/site-title/style-rtl.css 57 B
build/block-library/blocks/site-title/style.css 57 B
build/block-library/blocks/social-link/editor-rtl.css 184 B
build/block-library/blocks/social-link/editor.css 184 B
build/block-library/blocks/social-links/editor-rtl.css 674 B
build/block-library/blocks/social-links/editor.css 673 B
build/block-library/blocks/social-links/style-rtl.css 1.4 kB
build/block-library/blocks/social-links/style.css 1.39 kB
build/block-library/blocks/spacer/editor-rtl.css 332 B
build/block-library/blocks/spacer/editor.css 332 B
build/block-library/blocks/spacer/style-rtl.css 48 B
build/block-library/blocks/spacer/style.css 48 B
build/block-library/blocks/table/editor-rtl.css 433 B
build/block-library/blocks/table/editor.css 433 B
build/block-library/blocks/table/style-rtl.css 651 B
build/block-library/blocks/table/style.css 650 B
build/block-library/blocks/table/theme-rtl.css 157 B
build/block-library/blocks/table/theme.css 157 B
build/block-library/blocks/tag-cloud/style-rtl.css 251 B
build/block-library/blocks/tag-cloud/style.css 253 B
build/block-library/blocks/template-part/editor-rtl.css 404 B
build/block-library/blocks/template-part/editor.css 404 B
build/block-library/blocks/template-part/theme-rtl.css 101 B
build/block-library/blocks/template-part/theme.css 101 B
build/block-library/blocks/text-columns/editor-rtl.css 95 B
build/block-library/blocks/text-columns/editor.css 95 B
build/block-library/blocks/text-columns/style-rtl.css 166 B
build/block-library/blocks/text-columns/style.css 166 B
build/block-library/blocks/verse/style-rtl.css 99 B
build/block-library/blocks/verse/style.css 99 B
build/block-library/blocks/video/editor-rtl.css 552 B
build/block-library/blocks/video/editor.css 555 B
build/block-library/blocks/video/style-rtl.css 179 B
build/block-library/blocks/video/style.css 179 B
build/block-library/blocks/video/theme-rtl.css 139 B
build/block-library/blocks/video/theme.css 139 B
build/block-library/classic-rtl.css 162 B
build/block-library/classic.css 162 B
build/block-library/common-rtl.css 1.11 kB
build/block-library/common.css 1.11 kB
build/block-library/editor-elements-rtl.css 75 B
build/block-library/editor-elements.css 75 B
build/block-library/elements-rtl.css 54 B
build/block-library/elements.css 54 B
build/block-library/reset-rtl.css 478 B
build/block-library/reset.css 478 B
build/block-library/theme-rtl.css 698 B
build/block-library/theme.css 703 B
build/block-serialization-default-parser/index.min.js 1.13 kB
build/block-serialization-spec-parser/index.min.js 2.83 kB
build/blocks/index.min.js 51 kB
build/components/style-rtl.css 11.6 kB
build/components/style.css 11.7 kB
build/compose/index.min.js 12.3 kB
build/core-data/index.min.js 15.9 kB
build/customize-widgets/index.min.js 11.8 kB
build/customize-widgets/style-rtl.css 1.41 kB
build/customize-widgets/style.css 1.41 kB
build/data-controls/index.min.js 663 B
build/date/index.min.js 32.1 kB
build/deprecated/index.min.js 518 B
build/dom-ready/index.min.js 336 B
build/dom/index.min.js 4.71 kB
build/edit-post/classic-rtl.css 571 B
build/edit-post/classic.css 571 B
build/edit-post/index.min.js 34.5 kB
build/edit-post/style-rtl.css 7.5 kB
build/edit-post/style.css 7.5 kB
build/edit-widgets/index.min.js 16.9 kB
build/edit-widgets/style-rtl.css 4.52 kB
build/edit-widgets/style.css 4.52 kB
build/editor/style-rtl.css 3.58 kB
build/editor/style.css 3.57 kB
build/element/index.min.js 4.93 kB
build/escape-html/index.min.js 548 B
build/format-library/index.min.js 7.23 kB
build/format-library/style-rtl.css 598 B
build/format-library/style.css 597 B
build/hooks/index.min.js 1.66 kB
build/html-entities/index.min.js 454 B
build/i18n/index.min.js 3.79 kB
build/is-shallow-equal/index.min.js 535 B
build/keyboard-shortcuts/index.min.js 1.79 kB
build/keycodes/index.min.js 1.92 kB
build/list-reusable-blocks/index.min.js 2.14 kB
build/list-reusable-blocks/style-rtl.css 865 B
build/list-reusable-blocks/style.css 865 B
build/media-utils/index.min.js 2.99 kB
build/notices/index.min.js 977 B
build/plugins/index.min.js 1.95 kB
build/preferences-persistence/index.min.js 2.23 kB
build/preferences/index.min.js 1.35 kB
build/primitives/index.min.js 960 B
build/priority-queue/index.min.js 1.52 kB
build/react-i18n/index.min.js 702 B
build/react-refresh-entry/index.min.js 8.44 kB
build/react-refresh-runtime/index.min.js 7.31 kB
build/redux-routine/index.min.js 2.75 kB
build/reusable-blocks/index.min.js 2.26 kB
build/reusable-blocks/style-rtl.css 265 B
build/reusable-blocks/style.css 265 B
build/rich-text/index.min.js 10.8 kB
build/server-side-render/index.min.js 2.09 kB
build/shortcode/index.min.js 1.52 kB
build/style-engine/index.min.js 1.53 kB
build/token-list/index.min.js 650 B
build/url/index.min.js 3.69 kB
build/vendors/inert-polyfill.min.js 2.48 kB
build/vendors/react-dom.min.js 41.8 kB
build/vendors/react.min.js 4.02 kB
build/viewport/index.min.js 1.09 kB
build/warning/index.min.js 280 B
build/widgets/index.min.js 7.31 kB
build/widgets/style-rtl.css 1.18 kB
build/widgets/style.css 1.18 kB
build/wordcount/index.min.js 1.06 kB

compressed-size-action

@glendaviesnz
Copy link
Contributor

This is testing pretty well for me. Will have a think about alternatives to all those styles.

Do you think we need to worry about hover styles overriding the borders?

button-hover.mp4

the above is on the Blockbase theme

@aaronrobertshaw
Copy link
Contributor Author

Do you think we need to worry about hover styles overriding the borders?

Good catch, thanks. I'd say we do.

Regarding theme.json and Global Styles, we might be able to leverage feature-level selectors to enforce the user's border selection. I don't think we have a means of styling the hover state of individual buttons at this point, so the block might be restricted to at least maintaining the same border.

We could extend the new styles further to enforce the border on hover when a border color, style or width are set.

If we come up with a solution to more elegantly override the Element API provided button styles that might also provide a clue to handling pseudo selectors and hover/focus states set by theme.json.

@glendaviesnz
Copy link
Contributor

Without these tweaks the user has to set multiple fields before a border shows up

For me with all those styles removed it seems to work as expected as long as a border width is set, which makes sense to me, ie. I would not expect border color and border radius to be visible at all until a width is specified, ie. this flow makes sense to me:

border-color.mp4

@aaronrobertshaw
Copy link
Contributor Author

ie. this flow makes sense to me:

I'll have to dig it up from the original border support PRs but this flow wasn't desired. Too many clicks and users expect to see visual feedback when they change a control.

@glendaviesnz
Copy link
Contributor

glendaviesnz commented Sep 30, 2022

too many clicks and users expect to see visual feedback when they change a control.

In this instance this might be a general assumption ( which does generally make sense ) being applied to a specific context where it might not make sense, ie. a 0px border logically should not provide any visual feedback as it doesn't actually exist 😄

@aaronrobertshaw
Copy link
Contributor Author

There's a lot of history around the border support and controls but here's a couple of links to where the application of a style and width when the user initially selects a color were raised.

Another factor to consider here, if we remove the styles as suggested, is that this block now behaves differently from others with border support.

@glendaviesnz
Copy link
Contributor

Another factor to consider here, if we remove the styles as suggested, is that this block now behaves differently from others with border support.

yeh, if it has already been well discussed and decided on then this probably isn't the PR to be unwinding it all on

@aaronrobertshaw
Copy link
Contributor Author

Thanks for the sanity check though. It is good to challenge past decisions and see how strongly they hold. I agree this might not be the PR to wind it back.

I'll revisit this PR next week to see what we might be able to do about the pseudo-selector styles.

@aaronrobertshaw aaronrobertshaw force-pushed the add/button-border-support branch from a2564e7 to 0b3c58c Compare December 16, 2022 07:01
@glendaviesnz
Copy link
Contributor

What is the status of this PR - is it ready for another test after you additional border supports?

The border-width: 0; set by the button element through the Elements API interferes with the UX of adding a border. It means we are back to the situation needing to set multiple fields before a border shows if we don't override it.
@aaronrobertshaw aaronrobertshaw force-pushed the add/button-border-support branch from 0b3c58c to 58e488c Compare February 6, 2023 06:08
@aaronrobertshaw
Copy link
Contributor Author

What is the status of this PR

Thanks for checking in on this one @glendaviesnz 👍

I've rebased it and tested applying borders through theme.json or at the individual block level. It's all testing pretty well for me. If you could give it another once over, then I think we can land this pending your approval.

@github-actions
Copy link

github-actions bot commented Feb 6, 2023

Flaky tests detected in 4ca13f0.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4118026664
📝 Reported issues:

@glendaviesnz
Copy link
Contributor

glendaviesnz commented Feb 7, 2023

It looks some of the core defaults (hover, focus) are still taking priority over the theme.json setting, but not the user applied settings:

button-defaults.mp4

this is with the theme.json settings suggested in the description. I can take a closer look at this tomorrow if you don't have chance to look at it.

@aaronrobertshaw
Copy link
Contributor Author

Thanks for reviewing @glendaviesnz 👍

this is with the theme.json settings suggested in the description

I'm having trouble replicating the behaviour in your demo above. I'll continue to dig into it though. Using TwentyTwentyThree, here is what I see:

Screen.Recording.2023-02-07.at.2.43.38.pm.mp4

In the meantime, it might also be worth noting that we can probably use the elements pseudo selectors within theme.json to configure the :hover and :focus border styles. Something like the following might work (will need to test this):

	"core/button": {
		"border": {
			"radius": "0",
			"color": "fuchsia",
			"style": "solid",
			"width": "1em"
		},
		"elements": {
			"link": {
				":hover": {
					"border": {
						"color": "green"
					}
				}
			}
		}
	},

@aaronrobertshaw
Copy link
Contributor Author

I can take a closer look at this tomorrow if you don't have chance to look at it.

@glendaviesnz If you can take a further look into the behaviour you were seeing that would be great thanks.

I can't replicate any core styles overriding the button theme.json styles. At most, I noticed some themes that add their own styles with higher specificity than theme.json, overriding styles e.g. blockbase.

Apologies if I'm missing something obvious here 🤔

@glendaviesnz
Copy link
Contributor

@glendaviesnz If you can take a further look into the behaviour you were seeing that would be great thanks.

On further testing it was actually some higher specificity theme overrides causing this, which needs to be sorted at the theme end rather than in the block supports I think.

Copy link
Contributor

@glendaviesnz glendaviesnz left a comment

Choose a reason for hiding this comment

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

LGTM

@aaronrobertshaw aaronrobertshaw merged commit 3f7e7f9 into trunk Feb 7, 2023
@aaronrobertshaw aaronrobertshaw deleted the add/button-border-support branch February 7, 2023 23:27
@github-actions github-actions bot added this to the Gutenberg 15.2 milestone Feb 7, 2023
@femkreations femkreations added the Needs User Documentation Needs new user documentation label Jun 14, 2023
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 [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi Needs User Documentation Needs new user documentation [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants