-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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 violation [Block library 3] #65110
Changes from all commits
ff5e4d5
cfd8c15
3c7bd33
87a37b7
f5ddf47
0d3029d
0e297d9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,16 +32,14 @@ export function ConvertToLinksModal( { onClick, onClose, disabled } ) { | |
</p> | ||
<div className="wp-block-page-list-modal-buttons"> | ||
<Button | ||
// TODO: Switch to `true` (40px size) if possible | ||
__next40pxDefaultSize={ false } | ||
__next40pxDefaultSize | ||
variant="tertiary" | ||
onClick={ onClose } | ||
> | ||
{ __( 'Cancel' ) } | ||
</Button> | ||
<Button | ||
// TODO: Switch to `true` (40px size) if possible | ||
__next40pxDefaultSize={ false } | ||
__next40pxDefaultSize | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
variant="primary" | ||
accessibleWhenDisabled | ||
disabled={ disabled } | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -342,8 +342,7 @@ export default function PageListEdit( { | |
<PanelBody title={ __( 'Edit this menu' ) }> | ||
<p>{ convertDescription }</p> | ||
<Button | ||
// TODO: Switch to `true` (40px size) if possible | ||
__next40pxDefaultSize={ false } | ||
__next40pxDefaultSize | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
variant="primary" | ||
accessibleWhenDisabled | ||
disabled={ ! hasResolvedPages } | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -84,8 +84,7 @@ const CommentsForm = ( { postId, postType } ) => { | |
if ( 'closed' === commentStatus ) { | ||
const actions = [ | ||
<Button | ||
// TODO: Switch to `true` (40px size) if possible | ||
__next40pxDefaultSize={ false } | ||
__next40pxDefaultSize | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
key="enableComments" | ||
onClick={ () => setCommentStatus( 'open' ) } | ||
variant="primary" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -308,8 +308,7 @@ export default function PostFeaturedImageEdit( { | |
mediaLibraryButton={ ( { open } ) => { | ||
return ( | ||
<Button | ||
// TODO: Switch to `true` (40px size) if possible | ||
__next40pxDefaultSize={ false } | ||
__next40pxDefaultSize | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As @hbhalodia said, noting that the These style overrides should be removed, if possible — but it's not in the scope of this PR. We can keep changes as they are. |
||
icon={ upload } | ||
variant="primary" | ||
label={ label } | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -627,8 +627,7 @@ export default function LogoEdit( { | |||||
render={ ( { open } ) => ( | ||||||
<div className="block-library-site-logo__inspector-upload-container"> | ||||||
<Button | ||||||
// TODO: Switch to `true` (40px size) if possible | ||||||
__next40pxDefaultSize={ false } | ||||||
__next40pxDefaultSize | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be seen, while you add Note: This has some styles that have higher specificity and specific to this button on this block, hence before and after is same, but we can see our change is applied in inspector controls.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have style overrides in this case as well. For the context of this PR, I think the current code changes are enough. Still, I'd love to remove all of those overrides if possible, and just use a standard
Therefore, we'd need to make sure that we could offer a good, consistent UI in all three cases even without those overrides. Something we could do in a follow-up (cc @WordPress/gutenberg-design )/ Also, I noticed that the dropdown comes from the |
||||||
onClick={ open } | ||||||
variant="secondary" | ||||||
> | ||||||
|
@@ -674,8 +673,7 @@ export default function LogoEdit( { | |||||
mediaLibraryButton={ ( { open } ) => { | ||||||
return ( | ||||||
<Button | ||||||
// TODO: Switch to `true` (40px size) if possible | ||||||
__next40pxDefaultSize={ false } | ||||||
__next40pxDefaultSize | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above. There are again a lot of style overrides, which ideally shouldn't be there at all. The code change here is good for the scope of this PR |
||||||
icon={ upload } | ||||||
variant="primary" | ||||||
label={ __( 'Choose logo' ) } | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be seen, while you add page-list block and try to convert page-list to navigationLinks