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

ComboboxControl: Add lint rule for 40px size prop usage #64560

Merged
merged 4 commits into from
Aug 16, 2024
Merged

ComboboxControl: Add lint rule for 40px size prop usage #64560

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
88a9176
Fix in Author block
mirka Aug 15, 2024
b00f9a4
Fix in Avatar block
mirka Aug 15, 2024
42deaf2
Add lint rule
mirka Aug 15, 2024
563439d
Merge branch 'trunk' into combobox-40px
mirka Aug 16, 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
1 change: 1 addition & 0 deletions .eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,7 @@ module.exports = {
...[
'BorderBoxControl',
'BorderControl',
'ComboboxControl',
'DimensionControl',
'FontSizePicker',
'ToggleGroupControl',
Expand Down
1 change: 1 addition & 0 deletions packages/block-library/src/avatar/user-control.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ function UserControl( { value, onChange } ) {

return (
<ComboboxControl
__next40pxDefaultSize
Copy link
Member Author

Choose a reason for hiding this comment

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

Before After
Avatar block, before Avatar block, after

__nextHasNoMarginBottom
label={ __( 'User' ) }
help={ __(
Expand Down
1 change: 1 addition & 0 deletions packages/block-library/src/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
@import "./nextpage/editor.scss";
@import "./page-list/editor.scss";
@import "./paragraph/editor.scss";
@import "./post-author/editor.scss";
@import "./post-excerpt/editor.scss";
@import "./pullquote/editor.scss";
@import "./rss/editor.scss";
Expand Down
1 change: 1 addition & 0 deletions packages/block-library/src/post-author/block.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,5 +66,6 @@
"clientNavigation": true
}
},
"editorStyle": "wp-block-post-author-editor",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this change needed?

Copy link
Member

Choose a reason for hiding this comment

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

It's necessary because we're readding the editor style file and we want it to be taken into consideration when previewing the blocks in the editor.

"style": "wp-block-post-author"
}
125 changes: 67 additions & 58 deletions packages/block-library/src/post-author/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
PanelBody,
SelectControl,
ToggleControl,
__experimentalVStack as VStack,
} from '@wordpress/components';
import { useSelect, useDispatch } from '@wordpress/data';
import { __ } from '@wordpress/i18n';
Expand Down Expand Up @@ -100,74 +101,82 @@ function PostAuthorEdit( {
<>
<InspectorControls>
<PanelBody title={ __( 'Settings' ) }>
{ showAuthorControl &&
( ( showCombobox && (
<ComboboxControl
__nextHasNoMarginBottom
label={ __( 'Author' ) }
options={ authorOptions }
value={ authorId }
onChange={ handleSelect }
allowReset={ false }
/>
) ) || (
<VStack
spacing={ 4 }
className="wp-block-post-author__inspector-settings"
>
{ showAuthorControl &&
( ( showCombobox && (
<ComboboxControl
__next40pxDefaultSize
Copy link
Member Author

Choose a reason for hiding this comment

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

The spacing gets messed up when a ComboboxControl enters the mix, because it has an additional div wrapper outside the BaseControl. The auto margin added to BaseControls by the block inspector interprets this as a :last-child BaseControl, and adds a smaller margin.

So here we add a VStack to explicitly handle our spacing, with the correct metrics (16px gap), and additional CSS to counteract the auto margins. Painful 😭 FYI I am working on this in #64526.

Before After
Author block, before Author block, after

Looks like this when showCombobox === false:

Author block, after with select

Copy link
Contributor

Choose a reason for hiding this comment

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

Painful

:hug:

__nextHasNoMarginBottom
label={ __( 'Author' ) }
options={ authorOptions }
value={ authorId }
onChange={ handleSelect }
allowReset={ false }
/>
) ) || (
<SelectControl
__next40pxDefaultSize
__nextHasNoMarginBottom
label={ __( 'Author' ) }
value={ authorId }
options={ authorOptions }
onChange={ handleSelect }
/>
) ) }
<ToggleControl
__nextHasNoMarginBottom
label={ __( 'Show avatar' ) }
checked={ showAvatar }
onChange={ () =>
setAttributes( { showAvatar: ! showAvatar } )
}
/>
{ showAvatar && (
<SelectControl
__next40pxDefaultSize
__nextHasNoMarginBottom
label={ __( 'Author' ) }
value={ authorId }
options={ authorOptions }
onChange={ handleSelect }
label={ __( 'Avatar size' ) }
value={ attributes.avatarSize }
options={ avatarSizes }
onChange={ ( size ) => {
setAttributes( {
avatarSize: Number( size ),
} );
} }
/>
) ) }
<ToggleControl
__nextHasNoMarginBottom
label={ __( 'Show avatar' ) }
checked={ showAvatar }
onChange={ () =>
setAttributes( { showAvatar: ! showAvatar } )
}
/>
{ showAvatar && (
<SelectControl
__next40pxDefaultSize
) }
<ToggleControl
__nextHasNoMarginBottom
label={ __( 'Avatar size' ) }
value={ attributes.avatarSize }
options={ avatarSizes }
onChange={ ( size ) => {
setAttributes( {
avatarSize: Number( size ),
} );
} }
label={ __( 'Show bio' ) }
checked={ showBio }
onChange={ () =>
setAttributes( { showBio: ! showBio } )
}
/>
) }
<ToggleControl
__nextHasNoMarginBottom
label={ __( 'Show bio' ) }
checked={ showBio }
onChange={ () =>
setAttributes( { showBio: ! showBio } )
}
/>
<ToggleControl
__nextHasNoMarginBottom
label={ __( 'Link author name to author page' ) }
checked={ isLink }
onChange={ () => setAttributes( { isLink: ! isLink } ) }
/>
{ isLink && (
<ToggleControl
__nextHasNoMarginBottom
label={ __( 'Open in new tab' ) }
onChange={ ( value ) =>
setAttributes( {
linkTarget: value ? '_blank' : '_self',
} )
label={ __( 'Link author name to author page' ) }
checked={ isLink }
onChange={ () =>
setAttributes( { isLink: ! isLink } )
}
checked={ linkTarget === '_blank' }
/>
) }
{ isLink && (
<ToggleControl
__nextHasNoMarginBottom
label={ __( 'Open in new tab' ) }
onChange={ ( value ) =>
setAttributes( {
linkTarget: value ? '_blank' : '_self',
} )
}
checked={ linkTarget === '_blank' }
/>
) }
</VStack>
</PanelBody>
</InspectorControls>

Expand Down
7 changes: 7 additions & 0 deletions packages/block-library/src/post-author/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
.wp-block-post-author__inspector-settings {
// Counteract the margin added by the block inspector.
.components-base-control,
.components-base-control:last-child {
margin-bottom: 0;
}
}
Loading