Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Button Replace remaining 40px default size violations [Edit widgets] #65367

Merged
merged 1 commit into from
Sep 17, 2024

Conversation

dhruvang21
Copy link
Contributor

Part of - #65018

What?

Issue - #65018, To use default to 40px for the button.

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

Screenshot is added for individual changed files.

Copy link

github-actions bot commented Sep 16, 2024

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: dhruvang21 <dhruvang21@git.wordpress.org>
Co-authored-by: draganescu <andraganescu@git.wordpress.org>

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

Copy link

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @dhruvang21! In case you missed it, we'd love to have you join us in our Slack community.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Sep 16, 2024
@@ -44,8 +44,7 @@ export default function InserterSidebar() {
>
<TagName className="edit-widgets-layout__inserter-panel-header">
<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.

Before
before-widget-insert

After
after-widget-insert

@@ -51,8 +51,7 @@ export default function ListViewSidebar() {
<div className="edit-widgets-editor__list-view-panel-header">
<strong>{ __( 'List View' ) }</strong>
<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.

Before
before-list-view
After
after-list-view

@@ -66,8 +66,7 @@ export default function WidgetAreas( { selectedWidgetAreaId } ) {
) }
{ ! selectedWidgetArea && (
<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.

Before
before-widget-area

After
after-widget-area

Copy link
Contributor

@draganescu draganescu left a comment

Choose a reason for hiding this comment

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

This looked all right to me.

Unrelated I found that the focus ring of the editor buttons in the header is cut off in the widgets editor

Screenshot 2024-09-16 at 19 24 45

but this looks unrelated to this PR as it was the same in trunk for me.

@dhruvang21
Copy link
Contributor Author

Hi @draganescu,

I looked into the issue and found that it’s caused by the overflow: hidden; CSS property in the .edit-widgets-header__navigable-toolbar-wrapper class. I’ve attached a screenshot showing the resolved issue along with a patch for it.

Would it be okay to resolve this issue in the current PR, or would you prefer to create a new issue and assign it to me? I’d be happy to create a PR for it.

Thanks!

outline-issue

PATCH

diff --git a/packages/edit-widgets/src/components/header/style.scss b/packages/edit-widgets/src/components/header/style.scss
index 6e5d8de814..f3ee8f6e93 100644
--- a/packages/edit-widgets/src/components/header/style.scss
+++ b/packages/edit-widgets/src/components/header/style.scss
@@ -81,7 +81,6 @@
flex-shrink: 2;
padding-right: $grid-unit-10;
padding-left: $grid-unit-20;

  • overflow: hidden;
    }

.edit-widgets-header__title {

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.

Before
before-copy-error

After
after-copy-error

@draganescu draganescu added [Type] Bug An existing feature does not function as intended [Feature] UI Components Impacts or related to the UI component system [Type] Enhancement A suggestion for improvement. and removed [Type] Bug An existing feature does not function as intended labels Sep 17, 2024
@draganescu
Copy link
Contributor

@dhruvang21 let's fix the focus cutoff in another issue/PR. Great work here 👏🏻 thank you!

@draganescu draganescu merged commit 524516d into WordPress:trunk Sep 17, 2024
74 of 78 checks passed
@github-actions github-actions bot added this to the Gutenberg 19.3 milestone Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] UI Components Impacts or related to the UI component system First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants