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 3] #65309

Merged

mirka merged 5 commits into WordPress:trunk from hbhalodia:fix/issue-65018-7

Sep 25, 2024

Contributor

hbhalodia commented Sep 13, 2024

Part of - #65018

What?

Issue - Button: Replace remaining 40px default size violations #65018, To use default to 40px for the button.
This would fix in edit-site components, such as page-patterns, save-panel, sidebar-buttons, sidebar-daatviews, and site-hub.

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.

hbhalodia commented

View reviewed changes

packages/edit-site/src/components/page-patterns/fields.js

@@ @@ -133,8 +133,7 @@ function TitleField( { item } ) { @@
               					title
               				) : (
               					<Button
-              						// TODO: Switch to `true` (40px size) if possible
-              						__next40pxDefaultSize={ false }
+              						__next40pxDefaultSize

Contributor Author

hbhalodia Sep 16, 2024

This can be seen when you visit, site-editor --> check for patterns and the pattern title.

Note: It has a link variant, so there is an override style present, so before and after is same.

Before	After

hbhalodia commented

View reviewed changes

packages/edit-site/src/components/save-panel/index.js

@@ @@ -150,8 +150,7 @@ export default function SavePanel() { @@
               				} ) }
               			>
               				<Button
-              					// TODO: Switch to `true` (40px size) if possible
-              					__next40pxDefaultSize={ false }
+              					__next40pxDefaultSize

Contributor Author

hbhalodia Sep 16, 2024

This can be seen, when you open the save panel. It has style for screen-reader-text, hence need to remove that style to see the visual change.

Before	After

hbhalodia commented

View reviewed changes

packages/edit-site/src/components/sidebar-button/index.js

@@ @@ -11,8 +11,7 @@ import { Button } from '@wordpress/components'; @@
               export default function SidebarButton( props ) {
               	return (
               		<Button
-              			// TODO: Switch to `true` (40px size) if possible
-              			__next40pxDefaultSize={ false }
+              			__next40pxDefaultSize

Contributor Author

hbhalodia Sep 16, 2024

This can be seen when you visit, site-editor --> and check for icon button in sidebar panel.

Before	After

hbhalodia commented

View reviewed changes

packages/edit-site/src/components/sidebar-dataviews/add-new-view.js

@@ @@ -88,8 +88,7 @@ function AddNewItemModalContent( { type, setIsAdding } ) { @@
               				/>
               				<HStack justify="right">
               					<Button
-              						// TODO: Switch to `true` (40px size) if possible
-              						__next40pxDefaultSize={ false }
+              						__next40pxDefaultSize

Contributor Author

hbhalodia Sep 16, 2024

This can be seen when you add a custom data view and click on add new.

Before	After

hbhalodia commented

View reviewed changes

packages/edit-site/src/components/sidebar-dataviews/add-new-view.js

@@ @@ -99,8 +98,7 @@ function AddNewItemModalContent( { type, setIsAdding } ) { @@
               					</Button>
               					<Button
-              						// TODO: Switch to `true` (40px size) if possible
-              						__next40pxDefaultSize={ false }
+              						__next40pxDefaultSize

Contributor Author

hbhalodia Sep 16, 2024

This can be seen when you add a custom data view and click on add new.

Before	After

hbhalodia commented

View reviewed changes

packages/edit-site/src/components/site-hub/index.js

@@ @@ -62,8 +62,7 @@ const SiteHub = memo( @@
               						) }
               					>
               						<Button
-              							// TODO: Switch to `true` (40px size) if possible
-              							__next40pxDefaultSize={ false }
+              							__next40pxDefaultSize

Contributor Author

hbhalodia Sep 16, 2024 •

edited

Loading

This can be seen when you visit and check for go back button on the top left corner in site editor mode.

Note: It has fixed height, so changing this does not change before and after images.

Before	After

hbhalodia commented

View reviewed changes

packages/edit-site/src/components/site-hub/index.js

@@ @@ -80,8 +79,7 @@ const SiteHub = memo( @@
               					<HStack>
               						<div className="edit-site-site-hub__title">
               							<Button
-              								// TODO: Switch to `true` (40px size) if possible
-              								__next40pxDefaultSize={ false }
+              								__next40pxDefaultSize

Contributor Author

hbhalodia Sep 16, 2024

This can be seen when you visit and check for site title in site editor mode.

Note: It has link variant, hence 40px size does not affect the button.

Before	After

hbhalodia commented

View reviewed changes

packages/edit-site/src/components/site-hub/index.js

@@ @@ -101,8 +99,7 @@ const SiteHub = memo( @@
               							className="edit-site-site-hub__actions"
               						>
               							<Button
-              								// TODO: Switch to `true` (40px size) if possible
-              								__next40pxDefaultSize={ false }
+              								__next40pxDefaultSize

Contributor Author

hbhalodia Sep 16, 2024 •

edited

Loading

This can be seen when you visit and check for search icon on the site editor mode.

Before	After

hbhalodia commented

View reviewed changes

packages/edit-site/src/components/site-hub/index.js

@@ @@ -149,8 +146,7 @@ export const SiteHubMobile = memo( @@
               						) }
               					>
               						<Button
-              							// TODO: Switch to `true` (40px size) if possible
-              							__next40pxDefaultSize={ false }
+              							__next40pxDefaultSize

Contributor Author

hbhalodia Sep 16, 2024

This can be seen when you visit and check for go to back button on top left corver in site editor mode for mobile mode.

Note: It has fixed height, so changing this does not change before and after images.

Before	After

hbhalodia commented

View reviewed changes

packages/edit-site/src/components/site-hub/index.js

@@ @@ -186,8 +181,7 @@ export const SiteHubMobile = memo( @@
               							className="edit-site-site-hub__actions"
               						>
               							<Button
-              								// TODO: Switch to `true` (40px size) if possible
-              								__next40pxDefaultSize={ false }
+              								__next40pxDefaultSize

Contributor Author

hbhalodia Sep 16, 2024

This can be seen when you visit and check for search icon on the site editor mode on mobile.

Before	After

hbhalodia commented

View reviewed changes

packages/edit-site/src/components/site-hub/index.js

@@ @@ -170,8 +166,7 @@ export const SiteHubMobile = memo( @@
               					<HStack>
               						<div className="edit-site-site-hub__title">
               							<Button
-              								// TODO: Switch to `true` (40px size) if possible
-              								__next40pxDefaultSize={ false }
+              								__next40pxDefaultSize

Contributor Author

hbhalodia Sep 16, 2024

This can be seen when you visit and check for site title in site editor mode for mobile.

Note: It has link variant, hence 40px size does not affect the button.

Before	After

hbhalodia marked this pull request as ready for review

September 16, 2024 04:55

github-actions bot commented Sep 16, 2024 •

edited

Loading

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>
Co-authored-by: mirka <0mirka00@git.wordpress.org>
Co-authored-by: tyxla <tyxla@git.wordpress.org>

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

hbhalodia mentioned this pull request

Button: Replace remaining 40px default size violations #65018

Closed

21 tasks

mirka assigned hbhalodia

mirka added the [Type] Code Quality label

mirka requested a review from a team

September 18, 2024 14:15

mirka approved these changes

View reviewed changes

Member

mirka left a comment

Looks good! 🚀

mirka enabled auto-merge (squash)

September 19, 2024 09:35

tyxla disabled auto-merge

September 19, 2024 11:55

Member

tyxla commented Sep 19, 2024

It appears like there are some consistently failing e2e tests. Could they be related? I'd be surprised if they are, but let's try rebasing the branch, maybe that will resolve the failures.

mirka enabled auto-merge (squash)

September 24, 2024 06:42

hbhalodia added 5 commits

September 26, 2024 02:53


          Fix patterns link button title to use 40px default size

a9b6c3f


          Fix open save panel button default size to 40px

744b937


          Fix editor sidebar buttons to use 40px defualt size

752d3a2


          Fix data views create new views buttons to use 40px default size

2cc7851


          Fix site hub buttons to use 40px default button size

b9ab536

mirka force-pushed the fix/issue-65018-7 branch from dfed4f4 to b9ab536 Compare

September 25, 2024 17:55

Member

mirka commented Sep 25, 2024

Something is cursed about this branch, we just cannot get it to pass CI checks 😭 The failures are varied and don't seem to be related to the changes in this PR. I'm trying a clean rebase as a last resort.

mirka merged commit 4a625e5 into WordPress:trunk

58 of 59 checks passed

github-actions bot added this to the Gutenberg 19.4 milestone

Member

mirka commented Sep 25, 2024

Yay, it worked! 🎉

mirka added the Backport to WP 6.7 Beta/RC label

noisysocks added Backported to WP Core and removed Backport to WP 6.7 Beta/RC labels

noisysocks pushed a commit that referenced this pull request


          Fix: Button: Replace remaining 40px default size violation [Edit Site…

4914c6d

… 3] (#65309)

* Fix patterns link button title to use 40px default size

* Fix open save panel button default size to 40px

* Fix editor sidebar buttons to use 40px defualt size

* Fix data views create new views buttons to use 40px default size

* Fix site hub buttons to use 40px default button size

---------

Co-authored-by: hbhalodia <hbhalodia@git.wordpress.org>
Co-authored-by: mirka <0mirka00@git.wordpress.org>
Co-authored-by: tyxla <tyxla@git.wordpress.org>

noisysocks mentioned this pull request

Update packages for 6.7 Beta 2 WordPress/wordpress-develop#7515

Closed

draganescu mentioned this pull request

Manual cherry pick for #65627 #66119

Merged

sirreal mentioned this pull request

test/pr automation workflow #66457

Closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Backported to WP Core [Type] Code Quality