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

Customize widgets, edit post: refactor Button to new sizes #65807

Merged
merged 4 commits into from
Oct 2, 2024
Merged

Customize widgets, edit post: refactor Button to new sizes #65807

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
825f107
Customize widgets, edit post: refactor Button usages to new component…
ciampo Oct 1, 2024
e247866
Update snapshots
ciampo Oct 1, 2024
6094ebf
Switch to compact size in the customize widgets welcome guide
ciampo Oct 2, 2024
ff87070
Remove unused classnames, update snapshots
ciampo Oct 2, 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
7 changes: 1 addition & 6 deletions packages/customize-widgets/src/components/error-boundary/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,7 @@ import { doAction } from '@wordpress/hooks';
function CopyButton( { text, children } ) {
const ref = useCopyToClipboard( text );
return (
<Button
// TODO: Switch to `true` (40px size) if possible
__next40pxDefaultSize={ false }
variant="secondary"
ref={ ref }
>
<Button size="compact" 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.

  • Active an non-blocks based theme
  • Artificially throw somewhere in the package (
  • Visit customizer, then widgets

Screenshot 2024-10-01 at 22 09 32

Note: this button inherits some very generic style overrides, although this code is so legacy that we can probably keep it as-is without worrying too much.

Copy link
Member

Choose a reason for hiding this comment

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

I'm okay with the style overrides FWIW, but I think this is a perfect fit for the size="compact", given the size of the neighbor buttons that are likely to not change in the future.

Copy link
Contributor Author

@ciampo ciampo Oct 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Makes sense — so, just to make sure, no changes needed for this instance since size="compact" is already being applied?

The height doesn't look like the compact size because of the mentioned style overrides, which change (among other things) the box-sizing CSS property, causing the height of the button to be different than intended.

Copy link
Member

Choose a reason for hiding this comment

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

That's correct 👍

And whether we want to play with the style overrides in this PR in order to make the button height match better, I'll leave this to your judgment.

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'd leave it since it's a legacy part of the codebase, and this error screen should (hopefully) not be seen often by users 🤞

{ children }
</Button>
);
Expand Down
4 changes: 1 addition & 3 deletions packages/customize-widgets/src/components/inserter/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,7 @@ function Inserter( { setIsOpened } ) {
{ __( 'Add a block' ) }
</h2>
<Button
// TODO: Switch to `true` (40px size) if possible
__next40pxDefaultSize={ false }
className="customize-widgets-layout__inserter-panel-header-close-button"
size="small"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • Active an non-blocks based theme
  • Visit customizer, then widgets, then click on the "+" icon button

Screenshot 2024-10-01 at 19 09 56

I picked size="small" to be coherent with the other "X" button (which is defined somewhere else in the codebase)

icon={ closeSmall }
onClick={ () => setIsOpened( false ) }
aria-label={ __( 'Close inserter' ) }
Expand Down
4 changes: 1 addition & 3 deletions packages/customize-widgets/src/components/welcome-guide/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,7 @@ export default function WelcomeGuide( { sidebar } ) {
) }
</p>
<Button
// TODO: Switch to `true` (40px size) if possible
__next40pxDefaultSize={ false }
className="customize-widgets-welcome-guide__button"
size="compact"
variant="primary"
onClick={ () =>
toggle( 'core/customize-widgets', 'welcomeGuide' )
Expand Down
3 changes: 1 addition & 2 deletions packages/edit-post/src/components/back-button/fullscreen-mode-close.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,7 @@ function FullscreenModeClose( { showTooltip, icon, href, initialPost } ) {
return (
<motion.div whileHover="expand">
<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.

  • Edit a post

Screenshot 2024-10-01 at 21 57 17

This change doesn't really matter because of these style overrides.

className={ classes }
href={ buttonHref }
label={ buttonLabel }
Expand Down
2 changes: 1 addition & 1 deletion ...ges/edit-post/src/components/back-button/test/__snapshots__/fullscreen-mode-close.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ exports[`FullscreenModeClose when in full screen mode should display a default s
<div>
<a
aria-label="Back"
class="components-button edit-post-fullscreen-mode-close"
class="components-button edit-post-fullscreen-mode-close is-next-40px-default-size"
href="edit.php?"
>
<svg
Expand Down
3 changes: 1 addition & 2 deletions packages/edit-post/src/components/init-pattern-modal/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,7 @@ export default function InitPatternModal() {
/>
<HStack justify="right">
<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.

Visit /wp-admin/post-new.php?post_type=wp_block

Screenshot 2024-10-01 at 21 59 52

variant="primary"
type="submit"
disabled={ ! title }
Expand Down
4 changes: 1 addition & 3 deletions packages/edit-post/src/components/preferences-modal/enable-custom-fields.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,7 @@ export function CustomFieldsConfirmation( { willEnable } ) {
) }
</p>
<Button
// TODO: Switch to `true` (40px size) if possible
__next40pxDefaultSize={ false }
className="edit-post-preferences-modal__custom-fields-confirmation-button"
__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.

  • Edit a post
  • Open the preferences modal
  • Toggle the "Custom fields" option in the "Advanced" section

Screenshot 2024-10-01 at 22 03 00

variant="secondary"
isBusy={ isReloading }
accessibleWhenDisabled
Expand Down
4 changes: 2 additions & 2 deletions ...dit-post/src/components/preferences-modal/test/__snapshots__/enable-custom-fields.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ exports[`EnableCustomFieldsOption renders a checked checkbox and a confirmation
A page reload is required for this change. Make sure your content is saved before reloading.
</p>
<button
class="components-button edit-post-preferences-modal__custom-fields-confirmation-button is-secondary"
class="components-button is-next-40px-default-size is-secondary"
type="button"
>
Show & Reload Page
Expand Down Expand Up @@ -300,7 +300,7 @@ exports[`EnableCustomFieldsOption renders an unchecked checkbox and a confirmati
A page reload is required for this change. Make sure your content is saved before reloading.
</p>
<button
class="components-button edit-post-preferences-modal__custom-fields-confirmation-button is-secondary"
class="components-button is-next-40px-default-size is-secondary"
type="button"
>
Hide & Reload Page
Expand Down
Loading