-
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 [Edit Site 2] #65258
Changes from all commits
c05547a
26fbffc
1d4a850
d0b232a
ca97bb2
a06490c
911cefc
0d0ae24
61c6627
b933d43
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 |
---|---|---|
|
@@ -472,8 +472,7 @@ function FontCollection( { slug } ) { | |
className="font-library-modal__footer" | ||
> | ||
<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" | ||
onClick={ handleInstall } | ||
isBusy={ isInstalling } | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -437,8 +437,7 @@ function InstalledFonts() { | |
{ isInstalling && <ProgressBar /> } | ||
{ shouldDisplayDeleteButton && ( | ||
<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. |
||
isDestructive | ||
variant="tertiary" | ||
onClick={ handleUninstallClick } | ||
|
@@ -447,8 +446,7 @@ function InstalledFonts() { | |
</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" | ||
onClick={ handleUpdate } | ||
disabled={ ! fontFamiliesHasChanges } | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -230,8 +230,7 @@ function UploadFonts() { | |||||
onChange={ onFilesUpload } | ||||||
render={ ( { openFileDialog } ) => ( | ||||||
<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 when you visit, site-editor --> styles --> typography --> open font library modal --> upload font tab. Note: Due to higher specificity of button class, it has changed the size of upload button to 40px which is not looking nice. Should we revert this change or add some style change by adding !importnant or higher specificity? Cc: @mirka @ciampo
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. I raised this as a separate issue at #65333, but for the scope of this PR let's just add an 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.
Sure 👍. |
||||||
className="font-library-modal__upload-area" | ||||||
onClick={ openFileDialog } | ||||||
> | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -87,8 +87,7 @@ function Palette( { name } ) { | |
{ window.__experimentalEnableColorRandomizer && | ||
themeColors?.length > 0 && ( | ||
<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="secondary" | ||
icon={ shuffle } | ||
onClick={ randomizeThemeColors } | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -162,8 +162,7 @@ function RevisionsButtons( { | |
aria-current={ isSelected } | ||
> | ||
<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. |
||
className="edit-site-global-styles-screen-revisions__revision-button" | ||
accessibleWhenDisabled | ||
disabled={ isSelected } | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -234,8 +234,7 @@ export default function ShadowsEditPanel() { | |
> | ||
<FlexItem> | ||
<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="tertiary" | ||
onClick={ () => | ||
setIsRenameModalVisible( false ) | ||
|
@@ -246,8 +245,7 @@ export default function ShadowsEditPanel() { | |
</FlexItem> | ||
<FlexItem> | ||
<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" | ||
type="submit" | ||
> | ||
|
@@ -381,8 +379,7 @@ function ShadowItem( { shadow, onChange, canRemove, onRemove } ) { | |
<HStack align="center" justify="flex-start" spacing={ 0 }> | ||
<FlexItem style={ { flexGrow: 1 } }> | ||
<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. |
||
icon={ shadowIcon } | ||
{ ...toggleProps } | ||
> | ||
|
@@ -394,8 +391,7 @@ function ShadowItem( { shadow, onChange, canRemove, onRemove } ) { | |
{ canRemove && ( | ||
<FlexItem> | ||
<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. |
||
icon={ reset } | ||
{ ...removeButtonProps } | ||
/> | ||
|
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 when you visit, site-editor --> styles --> typogrpahy --> open font library modal.
Note: This has changed some styles, but that is not looking good, should we add some higher specificity or !important? as we did previously with block library? @mirka @ciampo
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.
Yes, I guess we can go with a
!important
on theheight: auto
?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.
(FYI, unrelated to this but the tabs styling was broken. Fix proposed in #65330)
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.
Sure, Would do this.