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.

Sign up for GitHub

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

Jump to bottom

Fix: Button: Replace remaining 40px default size violation [Edit Site 1] #65226

Merged
merged 5 commits into from
Sep 19, 2024
Merged

Fix: Button: Replace remaining 40px default size violation [Edit Site 1] #65226

merged 5 commits into from
Sep 19, 2024

Conversation

hbhalodia
Copy link
Contributor

Part of - #65018

What?

Why?

  • To make the consistent button across Gutenberg, and we would have a lint rule added once fixed, all the button usage.

How?

  • Change from __next40pxDefaultSize={ false } to __next40pxDefaultSize on component.

Testing Instructions

  • Add the blocks on the page/post and check for the buttons defined.
  • Screenshot is added for individual changed files.

@tyxla tyxla mentioned this pull request Sep 11, 2024
Closed
21 tasks
@hbhalodia hbhalodia marked this pull request as ready for review September 12, 2024 04:30
@github-actions GitHub Actions
Copy link

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: hbhalodia <hbhalodia@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

hbhalodia
hbhalodia commented Sep 12, 2024
View reviewed changes
packages/edit-site/src/components/add-new-template/add-custom-template-modal-content.js
@@ -36,8 +36,7 @@ function SuggestionListItem( {
<Composite.Item
render={
<Button
// TODO: Switch to `true` (40px size) if possible
__next40pxDefaultSize={ false }
__next40pxDefaultSize
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This can be seen when you go to site editor and go to template, add new and select any template.

Note: This would have no impact as it has some styles which are overriden.

Before After
custom-template-before custom-template-after

hbhalodia
hbhalodia commented Sep 12, 2024
View reviewed changes
packages/edit-site/src/components/add-new-template/index.js
@@ -107,8 +107,7 @@ function TemplateListItem( {
} ) {
return (
<Button
// TODO: Switch to `true` (40px size) if possible
__next40pxDefaultSize={ false }
__next40pxDefaultSize
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This can be seen when you go to site editor and go to template, add new and check the modal for the same.

Note: This would have no impact as it has some styles which are overridden.

Before After
custom-template-index-before Custom-template-index-after

hbhalodia
hbhalodia commented Sep 12, 2024
View reviewed changes
packages/edit-site/src/components/editor-canvas-container/index.js
@@ -136,8 +136,7 @@ function EditorCanvasContainer( {
>
{ shouldShowCloseButton && (
<Button
// TODO: Switch to `true` (40px size) if possible
__next40pxDefaultSize={ false }
__next40pxDefaultSize
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This can be seen when you go to site editor --> styles --> revisions, there is close button.

Before After
editor-canvas-close-before Editor-anvas-close-after

hbhalodia
hbhalodia commented Sep 12, 2024
View reviewed changes
packages/edit-site/src/components/editor/index.js
@@ -258,8 +258,7 @@ export default function EditSiteEditor( { isPostsList = false } ) {
whileTap="tap"
>
<Button
// TODO: Switch to `true` (40px size) if possible
__next40pxDefaultSize={ false }
__next40pxDefaultSize
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This can be seen when you go to site editor --> styles, check for the left top icon to go back to dashboard or other page.

Note: This does not have impact because of some overridden styles.

Before After
Editor-close-button-before Editor-close-button-after

hbhalodia
hbhalodia commented Sep 12, 2024
View reviewed changes
packages/edit-site/src/components/error-boundary/warning.js
variant="secondary"
ref={ ref }
>
<Button __next40pxDefaultSize variant="secondary" ref={ ref }>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This can be seen when you have some unexpected error on editor and it breaks.

Note: It has some overridden style as height auto, so overall button size would get increased.

Before After
error-boundry-before error-boundry-after

@akasunil akasunil added the [Type] Code Quality Issues or PRs that relate to code quality label Sep 12, 2024
@mirka mirka requested a review from a team September 18, 2024 14:14
mirka
mirka approved these changes Sep 19, 2024
View reviewed changes
Copy link
Member

@mirka mirka left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@mirka mirka merged commit eb715c2 into WordPress:trunk Sep 19, 2024
72 of 74 checks passed
@github-actions github-actions bot added this to the Gutenberg 19.4 milestone Sep 19, 2024
@noisysocks noisysocks added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Sep 23, 2024
noisysocks pushed a commit that referenced this pull request Sep 23, 2024
@hbhalodia @mirka @noisysocks
Fix: Button: Replace remaining 40px default size violation [Edit Site…
eae0c80 
… 1] (#65226)

* Fix custom template modal button suggestion list to use default 40px button height

* Fix the add new template modal item button to use 40px default size

* Fix editor canvas close button to use 40px default button size

* Fix add the editor index back button for edit site to use 40px default size

* Fix the copy error button size for error boundry warning to use 40px default size

Co-authored-by: hbhalodia <hbhalodia@git.wordpress.org>
Co-authored-by: mirka <0mirka00@git.wordpress.org>
@noisysocks noisysocks added Backported to WP Core Pull request that has been successfully merged into WP Core and 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 labels Sep 23, 2024
@noisysocks noisysocks mentioned this pull request Sep 23, 2024
Closed
@draganescu draganescu mentioned this pull request Oct 15, 2024
Merged
@sirreal sirreal mentioned this pull request Oct 25, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mirka mirka mirka approved these changes

Assignees

@hbhalodia hbhalodia

Labels
Backported to WP Core Pull request that has been successfully merged into WP Core [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Milestone
Gutenberg 19.4
Development

Successfully merging this pull request may close these issues.
4 participants
@hbhalodia @mirka @noisysocks @akasunil