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: Introduce size prop #51842

Merged
merged 12 commits into from
Jun 23, 2023
Merged

Button: Introduce size prop #51842

merged 12 commits into from
Jun 23, 2023

Conversation

mirka
Copy link
Member

@mirka mirka commented Jun 23, 2023

Closes #51708
Part of #46741

What?

Introduces a new size prop to the Button component where:

  • small = 24px
  • compact = 32px
  • default = 40px when __next40pxDefaultSize = true, otherwise 36px

Why?

After some investigation and discussion in #51708, it became clear that Button still needs three size variants, at least for the time being.

How?

  • Reverts the changes in Button: Add opt-in prop for larger isSmall size #51012 (basically retracts the __next32pxSmallSize prop that I added there)
  • NumberControl: Fixes a prop name conflict by renaming (no visual changes)
  • FontSizePicker: Fixes a prop name conflict by simplifying the reset button

Testing Instructions

  1. npm run storybook:dev
  2. Check the size variants in the Button story. The __next40pxDefaultSize prop should only affect the size = 'default' case. The isSmall prop should still work.

Screenshots or screencast

Default

Buttons at default size

Default with __next40pxDefaultSize

Buttons at default size (40px)

Compact

Buttons at compact size

Small

Buttons at small size

mirka added 7 commits June 22, 2023 22:45
This reverts commit 19bcabf.

# Conflicts:
#	packages/components/CHANGELOG.md
Also fixes type duplicate prop name issues in NumberControl and FontSizePicker
@mirka mirka self-assigned this Jun 23, 2023
@github-actions
Copy link

github-actions bot commented Jun 23, 2023

Size Change: +3.05 kB (0%)

Total Size: 1.44 MB

Filename Size Change
build/block-editor/index.min.js 208 kB -30 B (0%)
build/block-editor/style-rtl.css 14.7 kB +16 B (0%)
build/block-editor/style.css 14.7 kB +17 B (0%)
build/block-library/index.min.js 201 kB +290 B (0%)
build/components/index.min.js 240 kB +32 B (0%)
build/components/style-rtl.css 11.8 kB -9 B (0%)
build/components/style.css 11.8 kB -6 B (0%)
build/edit-post/index.min.js 33.9 kB +11 B (0%)
build/edit-site/index.min.js 82.8 kB +2.25 kB (+3%)
build/edit-site/style-rtl.css 12.5 kB +246 B (+2%)
build/edit-site/style.css 12.5 kB +242 B (+2%)
build/edit-widgets/index.min.js 16.8 kB +6 B (0%)
build/editor/index.min.js 45.5 kB +3 B (0%)
build/format-library/index.min.js 7.62 kB -17 B (0%)
ℹ️ View Unchanged
Filename Size
build/a11y/index.min.js 955 B
build/annotations/index.min.js 2.69 kB
build/api-fetch/index.min.js 2.28 kB
build/autop/index.min.js 2.1 kB
build/blob/index.min.js 451 B
build/block-directory/index.min.js 6.99 kB
build/block-directory/style-rtl.css 1.02 kB
build/block-directory/style.css 1.02 kB
build/block-editor/content-rtl.css 4.22 kB
build/block-editor/content.css 4.22 kB
build/block-editor/default-editor-styles-rtl.css 381 B
build/block-editor/default-editor-styles.css 381 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 126 B
build/block-library/blocks/audio/theme.css 126 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 104 B
build/block-library/blocks/avatar/style.css 104 B
build/block-library/blocks/block/editor-rtl.css 305 B
build/block-library/blocks/block/editor.css 305 B
build/block-library/blocks/button/editor-rtl.css 584 B
build/block-library/blocks/button/editor.css 582 B
build/block-library/blocks/button/style-rtl.css 624 B
build/block-library/blocks/button/style.css 623 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 113 B
build/block-library/blocks/categories/editor.css 112 B
build/block-library/blocks/categories/style-rtl.css 124 B
build/block-library/blocks/categories/style.css 124 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 409 B
build/block-library/blocks/columns/style.css 409 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 647 B
build/block-library/blocks/cover/editor.css 650 B
build/block-library/blocks/cover/style-rtl.css 1.61 kB
build/block-library/blocks/cover/style.css 1.6 kB
build/block-library/blocks/details/editor-rtl.css 65 B
build/block-library/blocks/details/editor.css 65 B
build/block-library/blocks/details/style-rtl.css 159 B
build/block-library/blocks/details/style.css 159 B
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 126 B
build/block-library/blocks/embed/theme.css 126 B
build/block-library/blocks/file/editor-rtl.css 316 B
build/block-library/blocks/file/editor.css 316 B
build/block-library/blocks/file/interactivity.min.js 395 B
build/block-library/blocks/file/style-rtl.css 269 B
build/block-library/blocks/file/style.css 270 B
build/block-library/blocks/file/view.min.js 375 B
build/block-library/blocks/footnotes/style-rtl.css 183 B
build/block-library/blocks/footnotes/style.css 182 B
build/block-library/blocks/freeform/editor-rtl.css 2.58 kB
build/block-library/blocks/freeform/editor.css 2.58 kB
build/block-library/blocks/gallery/editor-rtl.css 947 B
build/block-library/blocks/gallery/editor.css 952 B
build/block-library/blocks/gallery/style-rtl.css 1.53 kB
build/block-library/blocks/gallery/style.css 1.53 kB
build/block-library/blocks/gallery/theme-rtl.css 108 B
build/block-library/blocks/gallery/theme.css 108 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 336 B
build/block-library/blocks/html/editor.css 337 B
build/block-library/blocks/image/editor-rtl.css 834 B
build/block-library/blocks/image/editor.css 833 B
build/block-library/blocks/image/interactivity.min.js 1.34 kB
build/block-library/blocks/image/style-rtl.css 1.34 kB
build/block-library/blocks/image/style.css 1.34 kB
build/block-library/blocks/image/theme-rtl.css 126 B
build/block-library/blocks/image/theme.css 126 B
build/block-library/blocks/latest-comments/style-rtl.css 357 B
build/block-library/blocks/latest-comments/style.css 357 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 712 B
build/block-library/blocks/navigation-link/editor.css 711 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 296 B
build/block-library/blocks/navigation-submenu/editor.css 295 B
build/block-library/blocks/navigation/editor-rtl.css 2.26 kB
build/block-library/blocks/navigation/editor.css 2.26 kB
build/block-library/blocks/navigation/interactivity.min.js 978 B
build/block-library/blocks/navigation/style-rtl.css 2.21 kB
build/block-library/blocks/navigation/style.css 2.2 kB
build/block-library/blocks/navigation/view-modal.min.js 2.78 kB
build/block-library/blocks/navigation/view.min.js 438 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 401 B
build/block-library/blocks/page-list/editor.css 401 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 508 B
build/block-library/blocks/post-comments-form/style.css 508 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 71 B
build/block-library/blocks/post-excerpt/editor.css 71 B
build/block-library/blocks/post-excerpt/style-rtl.css 141 B
build/block-library/blocks/post-excerpt/style.css 141 B
build/block-library/blocks/post-featured-image/editor-rtl.css 588 B
build/block-library/blocks/post-featured-image/editor.css 586 B
build/block-library/blocks/post-featured-image/style-rtl.css 319 B
build/block-library/blocks/post-featured-image/style.css 319 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 314 B
build/block-library/blocks/post-template/style.css 314 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-time-to-read/style-rtl.css 69 B
build/block-library/blocks/post-time-to-read/style.css 69 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 335 B
build/block-library/blocks/pullquote/style.css 335 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 450 B
build/block-library/blocks/query/editor.css 449 B
build/block-library/blocks/quote/style-rtl.css 222 B
build/block-library/blocks/quote/style.css 222 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 178 B
build/block-library/blocks/search/editor.css 178 B
build/block-library/blocks/search/style-rtl.css 587 B
build/block-library/blocks/search/style.css 584 B
build/block-library/blocks/search/theme-rtl.css 114 B
build/block-library/blocks/search/theme.css 114 B
build/block-library/blocks/search/view.min.js 531 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 323 B
build/block-library/blocks/shortcode/editor.css 323 B
build/block-library/blocks/site-logo/editor-rtl.css 754 B
build/block-library/blocks/site-logo/editor.css 754 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.43 kB
build/block-library/blocks/social-links/style.css 1.42 kB
build/block-library/blocks/spacer/editor-rtl.css 348 B
build/block-library/blocks/spacer/editor.css 348 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 645 B
build/block-library/blocks/table/style.css 644 B
build/block-library/blocks/table/theme-rtl.css 146 B
build/block-library/blocks/table/theme.css 146 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 403 B
build/block-library/blocks/template-part/editor.css 403 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/term-description/style-rtl.css 111 B
build/block-library/blocks/term-description/style.css 111 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 174 B
build/block-library/blocks/video/style.css 174 B
build/block-library/blocks/video/theme-rtl.css 126 B
build/block-library/blocks/video/theme.css 126 B
build/block-library/classic-rtl.css 179 B
build/block-library/classic.css 179 B
build/block-library/common-rtl.css 1.1 kB
build/block-library/common.css 1.1 kB
build/block-library/editor-elements-rtl.css 75 B
build/block-library/editor-elements.css 75 B
build/block-library/editor-rtl.css 12.1 kB
build/block-library/editor.css 12.1 kB
build/block-library/elements-rtl.css 54 B
build/block-library/elements.css 54 B
build/block-library/interactivity/runtime.min.js 2.69 kB
build/block-library/interactivity/vendors.min.js 8.2 kB
build/block-library/reset-rtl.css 478 B
build/block-library/reset.css 478 B
build/block-library/style-rtl.css 13.6 kB
build/block-library/style.css 13.6 kB
build/block-library/theme-rtl.css 686 B
build/block-library/theme.css 691 B
build/block-serialization-default-parser/index.min.js 1.12 kB
build/block-serialization-spec-parser/index.min.js 2.87 kB
build/blocks/index.min.js 50.9 kB
build/commands/index.min.js 14.9 kB
build/commands/style-rtl.css 827 B
build/commands/style.css 827 B
build/compose/index.min.js 12 kB
build/core-commands/index.min.js 2.12 kB
build/core-data/index.min.js 16.1 kB
build/customize-widgets/index.min.js 11.9 kB
build/customize-widgets/style-rtl.css 1.46 kB
build/customize-widgets/style.css 1.45 kB
build/data-controls/index.min.js 640 B
build/data/index.min.js 8.25 kB
build/date/index.min.js 40.4 kB
build/deprecated/index.min.js 451 B
build/dom-ready/index.min.js 324 B
build/dom/index.min.js 4.63 kB
build/edit-post/classic-rtl.css 544 B
build/edit-post/classic.css 545 B
build/edit-post/style-rtl.css 7.58 kB
build/edit-post/style.css 7.57 kB
build/edit-widgets/style-rtl.css 4.53 kB
build/edit-widgets/style.css 4.53 kB
build/editor/style-rtl.css 3.58 kB
build/editor/style.css 3.58 kB
build/element/index.min.js 4.8 kB
build/escape-html/index.min.js 537 B
build/format-library/style-rtl.css 554 B
build/format-library/style.css 553 B
build/hooks/index.min.js 1.55 kB
build/html-entities/index.min.js 448 B
build/i18n/index.min.js 3.58 kB
build/is-shallow-equal/index.min.js 527 B
build/keyboard-shortcuts/index.min.js 1.64 kB
build/keycodes/index.min.js 1.84 kB
build/list-reusable-blocks/index.min.js 2.13 kB
build/list-reusable-blocks/style-rtl.css 836 B
build/list-reusable-blocks/style.css 836 B
build/media-utils/index.min.js 2.9 kB
build/notices/index.min.js 948 B
build/plugins/index.min.js 1.77 kB
build/preferences-persistence/index.min.js 1.84 kB
build/preferences/index.min.js 1.24 kB
build/primitives/index.min.js 943 B
build/priority-queue/index.min.js 1.52 kB
build/private-apis/index.min.js 943 B
build/react-i18n/index.min.js 615 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.7 kB
build/reusable-blocks/index.min.js 2.38 kB
build/reusable-blocks/style-rtl.css 243 B
build/reusable-blocks/style.css 243 B
build/rich-text/index.min.js 10.9 kB
build/router/index.min.js 1.77 kB
build/server-side-render/index.min.js 1.94 kB
build/shortcode/index.min.js 1.39 kB
build/style-engine/index.min.js 1.81 kB
build/token-list/index.min.js 582 B
build/url/index.min.js 3.57 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 958 B
build/warning/index.min.js 268 B
build/widgets/index.min.js 7.16 kB
build/widgets/style-rtl.css 1.15 kB
build/widgets/style.css 1.16 kB
build/wordcount/index.min.js 1.02 kB

compressed-size-action

@mirka mirka added the [Package] Components /packages/components label Jun 23, 2023
@@ -74,8 +65,13 @@ type BaseButtonProps = {
* Renders a pressed button style.
*/
isPressed?: boolean;
// TODO: Deprecate officially (add console warning and move to DeprecatedButtonProps).
Copy link
Member Author

Choose a reason for hiding this comment

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

This can only be done after all the usages in the Gutenberg app are migrated.

Comment on lines -51 to -54
&&& {
height: ${ ( props ) =>
props.size === '__unstable-large' ? '40px' : '30px' };
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure when it regressed, but these height overrides weren't currently working. The reset button was rendered as 24px no matter what.

In this PR, I went ahead and simplified it a bit so that the reset button is 40px in the __unstable-large case, and 24px otherwise. This should be ok because we'll be phasing out the non-large variants anyway.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good. I wonder if compact would be a better fallback than small here, though? Although it doesn't seem to matter much, since we're phasing it out anyway, as you mentioned

Copy link
Member Author

Choose a reason for hiding this comment

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

I definitely considered it, but a 2px difference between the text field and the button would irritate a designer more than a 6px difference would 😆

@@ -244,7 +244,7 @@ function UnforwardedNumberControl(
onClick={ buildSpinButtonClickHandler(
'up'
) }
size={ size }
spinButtonSize={ size }
Copy link
Member Author

Choose a reason for hiding this comment

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

Renamed to avoid prop name collision (no visual changes).

Comment on lines 225 to 227
- `'default'`: For normal text-label buttons, unless it is a toggle button.
- `'compact'`: For toggle buttons and icon buttons.
- `'small'`: For icon buttons associated with more advanced or auxiliary features.
Copy link
Member Author

Choose a reason for hiding this comment

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

@jasmussen Do these guidelines look ok?

Copy link
Contributor

Choose a reason for hiding this comment

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

This part of the sheet visually explains the difference between 40 and 32:

Screenshot 2023-06-23 at 14 42 39

Specifically, 40px is the default to use.

32px, and compact is a good word for it, is the one to use in certain situations, notably in the case of mixing toggle buttons with buttons.

So, we could tweak "compact":

  • 'compact': For toggle buttons, icon buttons, and buttons when used in context of either.

What do you think?

@mirka mirka marked this pull request as ready for review June 23, 2023 12:40
@mirka mirka requested a review from ajitbohra as a code owner June 23, 2023 12:40
@mirka mirka requested review from ciampo, jasmussen and chad1008 June 23, 2023 12:40
Copy link
Contributor

@jasmussen jasmussen left a comment

Choose a reason for hiding this comment

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

Yep, this seems like a good pragmatic solution. I think there's value to all three when building out an interface, even though it would be nice to have been able to settle for two sizes.

Copy link
Contributor

@ciampo ciampo left a comment

Choose a reason for hiding this comment

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

Apart from unit tests failures, I left a couple of comments, but nothing blocking — LGTM 🚀

Also, should we remove the isSmall prop from the README and Storybook, or should that be done once the prop is officially deprecated?

* - `'compact'`: For toggle buttons and icon buttons.
* - `'small'`: For icon buttons associated with more advanced or auxiliary features.
*
* @default 'default'
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably add a note about the size prop having priority over the isSmall prop, when defined. That should probably be added to the description of both props.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes! d03ba97

@@ -402,6 +402,11 @@ describe( 'Button', () => {
);
expect( console ).toHaveWarned();
} );

it( 'should not break when the legacy isSmall prop is passed', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also add a test about the interaction between isSmall and size, to make sure that they behave as expected in terms of which prop has priority?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea 9ec488a

Comment on lines -51 to -54
&&& {
height: ${ ( props ) =>
props.size === '__unstable-large' ? '40px' : '30px' };
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good. I wonder if compact would be a better fallback than small here, though? Although it doesn't seem to matter much, since we're phasing it out anyway, as you mentioned

@ciampo ciampo added [Type] Enhancement A suggestion for improvement. Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta labels Jun 23, 2023
@mirka
Copy link
Member Author

mirka commented Jun 23, 2023

Thank you for the reviews!

✅ I fixed the issue that was causing the unit test failures 590964c

should we remove the isSmall prop from the README and Storybook, or should that be done once the prop is officially deprecated?

I guess we should keep it around until the official deprecation, since it's still very widely used 👍

@mirka mirka enabled auto-merge (squash) June 23, 2023 19:50
@mirka mirka merged commit a3af2c7 into trunk Jun 23, 2023
@mirka mirka deleted the button-size branch June 23, 2023 20:14
@github-actions github-actions bot added this to the Gutenberg 16.2 milestone Jun 23, 2023
@ramonjd ramonjd added the Backport to Gutenberg RC Pull request that needs to be backported to a Gutenberg release candidate (RC) label Jun 26, 2023
tellthemachines pushed a commit that referenced this pull request Jun 27, 2023
* Revert "Button: Add opt-in prop for larger `isSmall` size (#51012)"

This reverts commit 19bcabf.

# Conflicts:
#	packages/components/CHANGELOG.md

* Add docs for `size` prop

Also fixes type duplicate prop name issues in NumberControl and FontSizePicker

* Add CSS

* Fixup

* Add TODO for deprecation

* Add test for back compat

* Fixup

* Add changelog

* Tweak description for "compact"

* Note that the `size` prop takes precedence

* Add test for prop priority

* Stop leaking `spinButtonSize` prop for styling
@tellthemachines
Copy link
Contributor

I just cherry-picked this PR to the release/16.1 branch to get it included in the next release: a7c4b8f

@tellthemachines tellthemachines removed Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta Backport to Gutenberg RC Pull request that needs to be backported to a Gutenberg release candidate (RC) labels Jun 27, 2023
tellthemachines added a commit that referenced this pull request Jun 27, 2023
* Site Editor: Disable the revision button if there is no clickable menu (#51851)

* Improve LinkControl Edit UI (#51712)

* Move text above link

* Change "URL" label to "Link"

* Style tweaks

* Add chevron based advanced settings button

* Adapt logic for rendering actions and settings

* Tweaks

* Add proper i18n

Co-authored-by: Ben Dwyer <[email protected]>

* Remove commented out style

Co-authored-by: Ben Dwyer <[email protected]>

* Use $button-size-next-default-40px

* Add showSettings, combine with new logic

* Add additional translation context to advanced

* Update toggle drawer name in tests

* Standardise query for settings toggle

* Update test to check for absence of cancel button during link creation

* Fix cancellation tests

* Ensure label is always “Link” but remains hidden when it’s the only visible control

* Update tests to look for “Link” instead of “URL” name for input

* Update empty value UI tests to only run for editing as opposed to creating links

* Fix e2e test tabbing order

* Use updated terms

* Select settings toggle by text not aria label

* Fix another tabbing order bug

* Fix one more tabbing issue in e2e tests

* Fix final tab ordering e2e test

* Decouple conditions for showing action buttons from settings

Settings may not be provided but action buttons are always needed

* Tweak styling to account for action buttons when there are no settings provided

* Fix test

* Fix e2e test

* Update name of the combobox

* Fix test expecting Submit button on creation

* Fix test by testing under edit rather than creation conditions

* Rename URL to Link and avoid triggering command centre

* move test earlier

---------

Co-authored-by: Ben Dwyer <[email protected]>
Co-authored-by: Dave Smith <[email protected]>

* Update colors (#51856)

* Library: Fix misalignment of description in custom template parts (#51868)

* Backport adding the distraction free mode to the site editor (#51173) (#51932)

* Fix toolbar overlap in site editor (#51810)

* expand fixed toolbar to cover document title control

* adds the z-index code back

* Page Content Focus: Switch to Page panel when deselecting a block (#51881)

* Don't show 'Back to page' notification when navigating away from page (#51880)

* Add top margin to page details (#51858)

* Keep framer-motion from updating minor version (#51894)

* Keep framer-motion from updating minor version

* Revert unnecessary package-lock changes

* useBlockSync(): Reset inner blocks when component unmounts (#51783)

* BlockLockModal: restore focus on fallback toolbar button when original button is not rendered (#51666)

* useFocusReturn: pass focus restoration default target to the onFocusReturn callback

* Modal: pass onFocusReturn callback

* BlockLockModal: restore focus to first focusable item when unlocking block from toolbar button

* Add comments

* Revert changes to `useFocusReturn` and `Modal` component, just add logic to the BlockLockToolbar instead

* Comment

* Fix missing MenuGroup in header more menu (#51860)

* Add `manage all custom patterns` command (#51845)

* Add manage all custom patterns command

* reorganise with useAdminNavigationCommands

* Command center: Add another batch of commands to the site editor (#51832)

Co-authored-by: Nik Tsekouras <[email protected]>

* Fix delete shortcut incorrectly bound to non-user patterns (#51830)

* `ConfirmDialog`: Fix affirmative action being triggered an extra time when selecting a button via keyboard (#51730)

* Ensure the confirm dialog cannnot be submitted using enter when the cancel button is focused

* Add test case

* Add CHANGELOG entry

* Add PR number to changelog

* Also prevent double submission of Confirm button

* Use actions in storybook example rather than outputting to a heading

* Ensure there is always a Navigation available in the browse mode sidebar via fallback algorithm (#50321)

* Normalize menu used in sidebar with fallback algorithm

* Make fallback retrieval invalidate query cache for Navigation entities

* Conditionally trigger fallback creation if no menus are found

* Make code self documenting

* Add image block aspect ratio control (#51545)

* Simplify ImageSizeControl by using Auto as a placeholder

* Rename imageWidth and imageHeight props to naturalWidth and naturalHeight

* Convert NumberControl onChange values to Numbers

* Simplify LatestPostsEdit to use updated ImageSizeControl

* Add JSDoc types for debugging

* Remove unnecessary noop

* Fix possible undefined values in NumberControl onChange

* Fix onChangeImage param type which may be undefined

* Rename OnChange callback prop

* Inline JSDoc props instead of new object

* Simplify handing undefined and NaN in onChange

* Revert prop name change since this isn't a private API

* Add a privateApis export for experimental ImageSizeControl

* Use the privateApis version of ImageSizeControl

* Add deprecation notice to the original component

* Revert image-size-control and create image-dimensions-control instead

* Re-add deprecation notice to image-size-control

* Try making a whole new component

* Revert changes to image, latest-posts, and media-text blocks

* Organize and update the dimensions tool panel item

* Reword size help text

* Reorganize into reusable components

* Add stories for other individual tools

* Update stories path

* Remove SelectControl __next prop

* Pass through isShownByDefault to ResolutionTool

* Remove unused scss

* Deprecate experimental ImageSizeControl

* Simplify ScaleTool onChange

* Add better defaults for value and onChange

* Fix circular dependency

* Update comment about auto and custom aspect ratios

* Add JSDoc types for ScaleTool

* Add JSDoc types for WidthHeightTool

* Add default value and onChange for WidthHeightTool

* Remove unused import

* Add aspectRatio to image block attributes

* Add scale to image block attributes

* Update JSDoc comment

* Add dimensions tool to image block

* Rename naturalAspectRatio for clarity

* Fix aspect-ratio-tool lint

* Fix scale-tool lint

* Fix width-height-tool lint

* Fix dimensions-tool lint

* Fix resolution-tool lint

* Add @emption/styled to block-editor

* Fix image block lint

* Update components changelog

* Fix AspectRatioTool reference

* Support 'auto' in width-height-tool

* Make null/undefined values mean 'auto' instead of defaultValue in aspectRatioTool

* Add deprecation for image block

* Fix ResizableBox interactions

* Add comments for default values

* Fix ResizableBox with auto w/h

* Clear aspect-ratio on resize

* Add TODO comment for ResolutionTool defaultValue

* Move the scale hide/show into dimensions controls

* Add first test

* Fix scale being set after it was deleted

* WIP writing tests

* Update test

* UI tweaks

* Move alt text as ToolsPanelItem

* Tweak default scale option help text

* Only use contain and cover for image scale options

* Update test

* Test the remaining callback values

* Add comment about toStrictEqual

* Add test for setting custom aspect ratio and then resetting

* Move custom scaleOptions to the image block

* Remember last aspect ratio so it can be restored when with/height are unset then set

* Remove unused import

* Format code

* Remove image w/h reset when a new image is added

* Use UnitControl's default units instead of spacing.units

* Provide the complete set of object-fit options by default

* Update TODO that will be committed

* Clean up evalAspectRatio and add docs

* Someone can file a bug report if offsetWidth/offsetHeight causes issues

* I couldn't figure out why height depended on having a custom border, but things seem to work without that

* Update docs for image block

* Update comment about default value

* Fix redundant wording

* I think the img width and height attributes can be removed if they're specified in the style attribute

* Update package-lock.json with @emotion/styled dependency

* Update mock calls for test example

* Simplify test values

* Consolidate mock calls expect

* Require defaultScale and defaultAspectRatio for DimensionsTool

* Add DimensionsTool tests for all custom transitions

* Remove comment about matching aspect ratio options

* Remove redundant check in tests

* Add comments to defaultAspectRatio and defaultScale

* Organize tests by which field is being updated

* Fix type conversion

* Add state diagram for last two tests

* Refactor and fix some tests

* Fix and simplify WidthHeightTool onChange

* Remove default scale option in image block.json

* Simplify DimensionsTool onChange logic

* Update block deprecations with width and height

* Revert image block width and height attributes to numbers since we only support px units for now

* Revert "Update block deprecations with width and height"

This reverts commit 941a81149ed4bc344ac2c0e183624069e33d75ad.

* Prevent NaN width/height

* Fix DimensionTool width/height units

* Fix JSDoc Dimenstions width/height types

* No default needed for ResolutionTool

* Fix drag handle aspect ratio reset

* Simplify null checks

* Stop using pxWidth and pxHeight

* Remove e2e tests that reference the scale button that was removed

* Fix image scaling for small images

* Try fixing aspectRatio only images

* Update test to respect the new aspect ratio behavior

---------

Co-authored-by: Alex Lende <[email protected]>
Co-authored-by: Rich Tabor <[email protected]>
Co-authored-by: Jerry Jones <[email protected]>

* Site Editor: Make string to add Template parts & Patterns consistent and translatable (#51743)

* Site Editor: Make Template Parts & Patterns Management dropdown translatable

* Make strings consistent

* Site Editor Sidebar: improvements to buttons (#51762)

* Do not show tooltip from all "back" buttons

* Avoid double button rendering in the patterns screen

* Use as prop instead of classname

* Add translation to strings

* Fix more icons for high resolution devices (#51768)

* Site tagline icon

* Update align-none.js

* Update position-left.js

* Update position-right.js

* Update position-center.js

* Update button.js

* Update buttons.js

* Update media-and-text.js

* Update spacer block icon

* Update separator.js

* Update stretch-full-width.js

* Update stretch-wide.js

* Update resize-corner-n-e.js

* Update justify-center.js

* Update align-left.js

* Update align-center.js

* Update align-right.js

* Update snapshots

* Hide block toolbar using CSS when it is empty (#51779)

* Update the add template modal design (#51806)

* Add icons

* alignment

* Custom descriptions

* justify content

* Style custom template button

* Remove min-height

* Don't display description when there isn't one

* Reduce space between template + description

* Style icon

* Style custom template

* increase button size

* Add prompt

* Update template icons

* Make year dynamic

* Remove short descriptions

* Revert "Remove short descriptions"

This reverts commit 7eb06e8ab845b9cda3975989456614df5b221c29.

* re-instate descriptions but only show as a tooltip

* simplify a bit

---------

Co-authored-by: ntsekouras <[email protected]>

* Buttons Block: Fix for orientation-based block movers (#51831)

* Button: Introduce `size` prop (#51842)

* Revert "Button: Add opt-in prop for larger `isSmall` size (#51012)"

This reverts commit 19bcabf.

# Conflicts:
#	packages/components/CHANGELOG.md

* Add docs for `size` prop

Also fixes type duplicate prop name issues in NumberControl and FontSizePicker

* Add CSS

* Fixup

* Add TODO for deprecation

* Add test for back compat

* Fixup

* Add changelog

* Tweak description for "compact"

* Note that the `size` prop takes precedence

* Add test for prop priority

* Stop leaking `spinButtonSize` prop for styling

* Color (#51847)

* Only show Page Content Focus commands when in edit mode (#51888)

* Add UI commands to the post editor (#51900)

Co-authored-by: ntsekouras <[email protected]>

* ZStack: fix component bounding box to match children (#51836)

* ZStack: rewrite using CSS grid

* Use first-of-type instead of fist-child

* CHANGELOG

* Improve comment

* Apply styles once in the parent wrapper

* Avoid each child view from expanding to all available space

* Remove unnecessary wrapeprs in storybook exmaple

* Add view patterns plural label. (#51850)

* Fix css styles in block.jsons. (#51866)

* Update active item appearance in Library (#51848)

* Color

* Use aria-current

* Fix Rename in Navigation on Browse Mode (#51791)

* Ensure edits are passed to save

* Ensure empty strings are invalid

* Force break of long strings in menu titles

* Fix ability to click through to Template Parts in Library (#51838)

* ItemGroup: Update button focus styles to use `:focus-visible` (#51787)

* Use focus-visible rather than focus on ItemGroup buttons

* Update snapshot

* Update Changelog

* Update package-lock

---------

Co-authored-by: Aki Hamano <[email protected]>
Co-authored-by: Rich Tabor <[email protected]>
Co-authored-by: Ben Dwyer <[email protected]>
Co-authored-by: Dave Smith <[email protected]>
Co-authored-by: James Koster <[email protected]>
Co-authored-by: Héctor <[email protected]>
Co-authored-by: Andrei Draganescu <[email protected]>
Co-authored-by: Robert Anderson <[email protected]>
Co-authored-by: Marco Ciampini <[email protected]>
Co-authored-by: Nik Tsekouras <[email protected]>
Co-authored-by: Riad Benguella <[email protected]>
Co-authored-by: Daniel Richards <[email protected]>
Co-authored-by: Alex Lende <[email protected]>
Co-authored-by: Alex Lende <[email protected]>
Co-authored-by: Jerry Jones <[email protected]>
Co-authored-by: Lena Morita <[email protected]>
Co-authored-by: Andrea Fercia <[email protected]>
Co-authored-by: Jonny Harris <[email protected]>
Co-authored-by: Sarah Norris <[email protected]>
@bph bph modified the milestones: Gutenberg 16.2, Gutenberg 16.1 Jun 27, 2023
sethrubenstein pushed a commit to pewresearch/gutenberg that referenced this pull request Jul 13, 2023
* Revert "Button: Add opt-in prop for larger `isSmall` size (WordPress#51012)"

This reverts commit 19bcabf.

# Conflicts:
#	packages/components/CHANGELOG.md

* Add docs for `size` prop

Also fixes type duplicate prop name issues in NumberControl and FontSizePicker

* Add CSS

* Fixup

* Add TODO for deprecation

* Add test for back compat

* Fixup

* Add changelog

* Tweak description for "compact"

* Note that the `size` prop takes precedence

* Add test for prop priority

* Stop leaking `spinButtonSize` prop for styling
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Button: Audit 24px isSmall size
6 participants