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.

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

add width control to separator block #16925

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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: 3 additions & 0 deletions packages/block-library/src/separator/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
},
"customColor": {
"type": "string"
},
"width": {
"type": "number"
}
}
}
19 changes: 17 additions & 2 deletions packages/block-library/src/separator/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,15 @@ import classnames from 'classnames';
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { HorizontalRule } from '@wordpress/components';
import { HorizontalRule, RangeControl, PanelBody } from '@wordpress/components';
import {
InspectorControls,
withColors,
PanelColorSettings,
} from '@wordpress/block-editor';

function SeparatorEdit( { color, setColor, className } ) {
function SeparatorEdit( { color, setColor, className, attributes, setAttributes } ) {
const { width = 25 } = attributes;
Copy link
Member

Choose a reason for hiding this comment

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

Should we create a constant at the top of the file for the default width (25)?

return (
<>
<HorizontalRule
Expand All @@ -27,9 +28,23 @@ function SeparatorEdit( { color, setColor, className } ) {
style={ {
backgroundColor: color.color,
color: color.color,
width: width + '%',
} }
/>
<InspectorControls>
<PanelBody title={ __( 'Separator Settings' ) }>
<RangeControl
label={ __( 'Percentage width' ) }
Copy link
Member

Choose a reason for hiding this comment

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

Range control has a initialPosition property https://github.com/WordPress/gutenberg//blob/1e40b281d1db725e512e383b5e00f3b59e333d5a/packages/components/src/range-control/README.md. when the value is unset the property allows the range to be in a give position e.g: 25, but the input is empty to allow the user to differentiate between the state the user set the width to 25 (inline style added) the user did not set any width but currently the default is 25 (no inline style is added).
Would it make sense to use this mechanism here?

value={ width || '' }
onChange={ ( nextWidth ) => {
setAttributes( { width: nextWidth } );
} }
min={ 1 }
max={ 100 }
required
allowReset
/>
</PanelBody>
<PanelColorSettings
title={ __( 'Color Settings' ) }
colorSettings={ [
Expand Down
2 changes: 2 additions & 0 deletions packages/block-library/src/separator/save.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export default function separatorSave( { attributes } ) {
const {
color,
customColor,
width,
} = attributes;

// the hr support changing color using border-color, since border-color
Expand All @@ -32,6 +33,7 @@ export default function separatorSave( { attributes } ) {
const separatorStyle = {
backgroundColor: backgroundClass ? undefined : customColor,
color: colorClass ? undefined : customColor,
width: width ? width + '%' : undefined,
};

return ( <hr
Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/separator/theme.scss
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

// Default, thin style
&:not(.is-style-wide):not(.is-style-dots) {
max-width: 100px;
width: 100px;
}

&.has-background:not(.is-style-dots) {
Expand Down