Skip to content

Adjust the custom range steps to match the units chosen #44959

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

Merged
glendaviesnz merged 3 commits into from
Oct 18, 2022
Merged

Adjust the custom range steps to match the units chosen #44959

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e0c71be
Adjust the custom range steps to match the units chosen
glendaviesnz Oct 17, 2022
1d9c272
Add max values specific to units selected
glendaviesnz Oct 17, 2022
37cf855
Extract custom value settings into config object
glendaviesnz Oct 18, 2022
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
15 changes: 13 additions & 2 deletions packages/block-editor/src/components/spacing-sizes-control/spacing-input-control.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ export default function SpacingInputControl( {
const customTooltipContent = ( newValue ) =>
value === undefined ? undefined : spacingSizes[ newValue ]?.name;

const customRangeValue = parseInt( currentValue, 10 );
const customRangeValue = parseFloat( currentValue, 10 );

const getNewCustomValue = ( newSize ) => {
const isNumeric = ! isNaN( parseFloat( newSize ) );
Expand Down Expand Up @@ -163,6 +163,8 @@ export default function SpacingInputControl( {
! showCustomValueControl &&
currentValueHint !== undefined;

const maxCustomValues = { px: 300, '%': 100, vw: 100, vh: 100 };
Copy link
Contributor

@talldan talldan Oct 18, 2022

Choose a reason for hiding this comment

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

This could be moved to file scope and named MAX_CUSTOM_VALUES to avoid the little bit of overhead of redefining it on every render.


return (
<>
{ side !== 'all' && (
Expand Down Expand Up @@ -227,7 +229,16 @@ export default function SpacingInputControl( {
<RangeControl
value={ customRangeValue }
min={ 0 }
max={ 300 }
max={
maxCustomValues[ selectedUnit ]
? maxCustomValues[ selectedUnit ]
: 10
Copy link
Contributor

@talldan talldan Oct 18, 2022

Choose a reason for hiding this comment

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

Could possibly change this to maxCustomValues[ selectedUnit ] ?? 10, but I also wonder why em and rem are excluded from maxCustomValues?

}
step={
[ 'px', '%', 'vh', 'vw' ].includes( selectedUnit )
? 1
: 0.1
}
Copy link
Contributor

@talldan talldan Oct 18, 2022

Choose a reason for hiding this comment

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

Might be good to do this the same way maxCustomValues works, extract it out to a variable.

withInputField={ false }
onChange={ handleCustomValueSliderChange }
className="components-spacing-sizes-control__custom-value-range"
Expand Down