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 2] #65258

Merged
merged 10 commits into from
Sep 16, 2024
Merged

Fix: Button: Replace remaining 40px default size violation [Edit Site 2] #65258

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
c05547a
Fix font library modal fonts to use 40px default button size
hbhalodia Sep 12, 2024
26fbffc
Fix install font button to use default 40px size
hbhalodia Sep 12, 2024
1d4a850
Fix the installed fonts button size to use 40px default
hbhalodia Sep 12, 2024
d0b232a
Fix upload font button size to use 40px default size
hbhalodia Sep 12, 2024
ca97bb2
Fix add the default button 40px size for randomize color pallete button
hbhalodia Sep 12, 2024
a06490c
Fix edit site style revision button to use 40px default size
hbhalodia Sep 12, 2024
911cefc
Fix edit site shadow components buttons to use default 40px size
hbhalodia Sep 12, 2024
0d0ae24
Merge branch 'trunk' into fix/issue-65018-6
hbhalodia Sep 15, 2024
61c6627
Fix the style for the button height on the font library modal
hbhalodia Sep 15, 2024
b933d43
Add !important to override the button size for 40px
hbhalodia Sep 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions packages/edit-site/src/components/global-styles/font-library-modal/font-card.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,7 @@ function FontCard( { font, onClick, variantsText, navigatorPath } ) {

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 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

Before After
Font-library-modal-before font-librayr-modal-after

Copy link
Member

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 the height: auto?

Copy link
Member

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)

Copy link
Contributor Author

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 the height: auto?

Sure, Would do this.

onClick={ () => {
onClick();
if ( navigatorPath ) {
Expand Down
3 changes: 1 addition & 2 deletions packages/edit-site/src/components/global-styles/font-library-modal/font-collection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -472,8 +472,7 @@ function FontCollection( { slug } ) {
className="font-library-modal__footer"
>
<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 visit, site-editor --> styles --> typogrpahy --> open font library modal and install fonts tab.

Before After
install-fonts-before install-fonts-after

variant="primary"
onClick={ handleInstall }
isBusy={ isInstalling }
Expand Down
6 changes: 2 additions & 4 deletions packages/edit-site/src/components/global-styles/font-library-modal/installed-fonts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,8 +437,7 @@ function InstalledFonts() {
{ isInstalling && <ProgressBar /> }
{ shouldDisplayDeleteButton && (
<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 visit, site-editor --> styles --> typogrpahy --> open font library modal and choose font to edit.

Before After
installed-fonts-delete-before Screenshot 2024-09-12 at 11 04 50 AM

isDestructive
variant="tertiary"
onClick={ handleUninstallClick }
Expand All @@ -447,8 +446,7 @@ function InstalledFonts() {
</Button>
) }
<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 visit, site-editor --> styles --> typogrpahy --> open font library modal and choose font

Before After
installed-fonts-update-before installed-fonts-update-after

variant="primary"
onClick={ handleUpdate }
disabled={ ! fontFamiliesHasChanges }
Expand Down
3 changes: 1 addition & 2 deletions packages/edit-site/src/components/global-styles/font-library-modal/upload-fonts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,7 @@ function UploadFonts() {
onChange={ onFilesUpload }
render={ ( { openFileDialog } ) => (
<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 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

Before After
upload-font-before Upload-font-after

Copy link
Member

Choose a reason for hiding this comment

The 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 !important to the height.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 !important to the height.

Sure 👍.

className="font-library-modal__upload-area"
onClick={ openFileDialog }
>
Expand Down
3 changes: 1 addition & 2 deletions packages/edit-site/src/components/global-styles/palette.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,7 @@ function Palette( { name } ) {
{ window.__experimentalEnableColorRandomizer &&
themeColors?.length > 0 && (
<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 visit, site-editor --> styles --> color --> pallete, option for randmoize colors.

Before After
radomize-colors-before randomize-colors-after

variant="secondary"
icon={ shuffle }
onClick={ randomizeThemeColors }
Expand Down
3 changes: 1 addition & 2 deletions packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,7 @@ function RevisionsButtons( {
aria-current={ isSelected }
>
<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 visit, site-editor --> styles --> revisions icon --> check for individual revisions button.

Note: This has some overridden styles, so no difference in before and after.

Before After
revisions-button-before revisions-button-after

className="edit-site-global-styles-screen-revisions__revision-button"
accessibleWhenDisabled
disabled={ isSelected }
Expand Down
12 changes: 4 additions & 8 deletions packages/edit-site/src/components/global-styles/shadows-edit-panel.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,7 @@ export default function ShadowsEditPanel() {
>
<FlexItem>
<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 visit, site-editor --> styles --> shadows --> add shadow and rename it, a modal would be opened and check for cancel button.

Before After
shadow-rename-cancel-before shadow-rename-cancel-after

variant="tertiary"
onClick={ () =>
setIsRenameModalVisible( false )
Expand All @@ -246,8 +245,7 @@ export default function ShadowsEditPanel() {
</FlexItem>
<FlexItem>
<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 visit, site-editor --> styles --> shadows --> add shadow and rename it, a modal would be opened and check for save button.

Before After
shadow-rename-save-before shadow-rename-save-after

variant="primary"
type="submit"
>
Expand Down Expand Up @@ -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
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 visit, site-editor --> styles --> shadows --> add shadow. Open shadow and check for drop shadow button.

Note: It already has a 40px size as height is auto.

Before After
drop-showdow-button-before drop-shadow-button-aafter

icon={ shadowIcon }
{ ...toggleProps }
>
Expand All @@ -394,8 +391,7 @@ function ShadowItem( { shadow, onChange, canRemove, onRemove } ) {
{ canRemove && (
<FlexItem>
<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 visit, site-editor --> styles --> shadows --> add shadow. Open shadow and check for drop shadow remove icon button.

Note: It already has a 40px size as height is auto.

Before After
drop-shadow-remove-before drop-shadow-remove-after

icon={ reset }
{ ...removeButtonProps }
/>
Expand Down
Loading