Skip to content

Removing pageSize from sliderProps #2917

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

Merged
merged 10 commits into from
Mar 4, 2022
21 changes: 0 additions & 21 deletions packages/@react-aria/slider/stories/Slider.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,6 @@ storiesOf('Slider (hooks)', module)
'single with aria label',
() => <StorySlider aria-label="Size" onChange={action('onChange')} onChangeEnd={action('onChangeEnd')} showTip />
)
.add(
'single with pageSize',
() => <StorySlider label="Degrees" onChange={action('onChange')} onChangeEnd={action('onChangeEnd')} minValue={0} maxValue={360} pageSize={15} formatOptions={{style: 'unit', unit: 'degree', unitDisplay: 'narrow'}} showTip />
)
.add(
'range',
() => (<StoryRangeSlider
Expand Down Expand Up @@ -60,23 +56,6 @@ storiesOf('Slider (hooks)', module)
unitDisplay: 'narrow'
} as any} />)
)
.add(
'range with pageSize',
() => (<StoryRangeSlider
label="Arc"
defaultValue={[45, 135]}
minValue={0}
maxValue={360}
pageSize={15}
onChange={action('onChange')}
onChangeEnd={action('onChangeEnd')}
showTip
formatOptions={{
style: 'unit',
unit: 'degree',
unitDisplay: 'narrow'
} as any} />)
)
.add(
'3 thumbs',
() => (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,6 @@ storiesOf('Slider/RangeSlider', module)
.add(
'min/max',
() => render({label: 'Label', minValue: 30, maxValue: 70})
)
.add(
'pageSize',
() => render({label: 'Label', minValue: 0, maxValue: 360, pageSize: 15, formatOptions: {style: 'unit', unit: 'degree', unitDisplay: 'narrow'}})
);

function render(props: SpectrumRangeSliderProps = {}) {
Expand Down
4 changes: 0 additions & 4 deletions packages/@react-spectrum/slider/stories/Slider.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,6 @@ storiesOf('Slider', module)
'step',
() => render({label: 'Label', minValue: 0, maxValue: 100, step: 5})
)
.add(
'pageSize',
() => render({label: 'Label', minValue: 0, maxValue: 360, pageSize: 15, formatOptions: {style: 'unit', unit: 'degree', unitDisplay: 'narrow'}})
)
.add(
'isFilled: true',
() => render({label: 'Label', isFilled: true})
Expand Down
43 changes: 39 additions & 4 deletions packages/@react-spectrum/slider/test/Slider.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -267,16 +267,51 @@ describe('Slider', function () {

it.each`
Name | props | commands
${'(left/right arrows, ltr)'} | ${{locale: 'de-DE'}} | ${[{left: press.ArrowRight, result: +1}, {left: press.ArrowLeft, result: -1}]}
${'(left/right arrows, rtl)'} | ${{locale: 'ar-AE'}} | ${[{left: press.ArrowRight, result: -1}, {left: press.ArrowLeft, result: +1}]}
${'(left/right arrows, ltr)'} | ${{locale: 'de-DE'}} | ${[{left: press.ArrowRight, result: +10}, {left: press.ArrowLeft, result: -10}]}
${'(left/right arrows, rtl)'} | ${{locale: 'ar-AE'}} | ${[{left: press.ArrowRight, result: -10}, {left: press.ArrowLeft, result: +10}]}
${'(page up/down, ltr)'} | ${{locale: 'de-DE'}} | ${[{left: press.PageUp, result: +20}, {left: press.PageDown, result: -20}]}
${'(page up/down, rtl)'} | ${{locale: 'ar-AE'}} | ${[{left: press.PageUp, result: +20}, {left: press.PageDown, result: -20}]}
`('$Name respects the page size', function ({props, commands}) {
`('$Name sets page size to a multiple of step', function ({props, commands}) {
let tree = render(
<Provider theme={theme} {...props}>
<Slider label="Label" pageSize={20} defaultValue={50} />
<Slider label="Label" minValue={0} maxValue={230} defaultValue={50} step={10} />
</Provider>
);
// The slider page size should be initially calulated as 230/10 = 23 and then snapped to 20 so it is a multiple of the step
let slider = tree.getByRole('slider');
testKeypresses([slider, slider], commands);
});

it.each`
Name | props | commands
${'(left/right arrows, ltr)'} | ${{locale: 'de-DE'}} | ${[{left: press.ArrowRight, result: +2}, {left: press.ArrowLeft, result: -2}]}
${'(left/right arrows, rtl)'} | ${{locale: 'ar-AE'}} | ${[{left: press.ArrowRight, result: -2}, {left: press.ArrowLeft, result: +2}]}
${'(page up/down, ltr)'} | ${{locale: 'de-DE'}} | ${[{left: press.PageUp, result: +2}, {left: press.PageDown, result: -2}]}
${'(page up/down, rtl)'} | ${{locale: 'ar-AE'}} | ${[{left: press.PageUp, result: +2}, {left: press.PageDown, result: -2}]}
`('$Name sets page size to a multiple of step (scenario: step is less than min)', function ({props, commands}) {
let tree = render(
<Provider theme={theme} {...props}>
<Slider label="Label" minValue={50} maxValue={75} defaultValue={60} step={2} />
</Provider>
);
// The slider page size should be initially calulated as 25/10 = 2.5, snaps to 2
let slider = tree.getByRole('slider');
testKeypresses([slider, slider], commands);
});

it.each`
Name | props | commands
${'(left/right arrows, ltr)'} | ${{locale: 'de-DE'}} | ${[{left: press.ArrowRight, result: +2}, {left: press.ArrowLeft, result: -2}]}
${'(left/right arrows, rtl)'} | ${{locale: 'ar-AE'}} | ${[{left: press.ArrowRight, result: -2}, {left: press.ArrowLeft, result: +2}]}
${'(page up/down, ltr)'} | ${{locale: 'de-DE'}} | ${[{left: press.PageUp, result: +4}, {left: press.PageDown, result: -4}]}
${'(page up/down, rtl)'} | ${{locale: 'ar-AE'}} | ${[{left: press.PageUp, result: +4}, {left: press.PageDown, result: -4}]}
`('$Name sets page size to a multiple of step (scenario: step is greater than max)', function ({props, commands}) {
let tree = render(
<Provider theme={theme} {...props}>
<Slider label="Label" minValue={-50} maxValue={-15} defaultValue={-40} step={2} />
</Provider>
);
// The slider page size should be initially calulated as 35/10 = 3.5, snaps to 4
let slider = tree.getByRole('slider');
testKeypresses([slider, slider], commands);
});
Expand Down
5 changes: 4 additions & 1 deletion packages/@react-stately/color/src/useColorSliderState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export function useColorSliderState(props: ColorSliderStateOptions): ColorSlider
}
});

let {step, pageSize} = color.getChannelRange(channel);
return {
...sliderState,
value: color,
Expand All @@ -84,6 +85,8 @@ export function useColorSliderState(props: ColorSliderStateOptions): ColorSlider
},
getThumbValueLabel() {
return color.formatChannelValue(channel, locale);
}
},
step,
pageSize
};
}
12 changes: 9 additions & 3 deletions packages/@react-stately/slider/src/useSliderState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import {clamp, snapValueToStep} from '@react-aria/utils';
import {SliderProps} from '@react-types/slider';
import {useControlledState} from '@react-stately/utils';
import {useRef, useState} from 'react';
import {useMemo, useRef, useState} from 'react';

export interface SliderState {
/**
Expand Down Expand Up @@ -159,10 +159,16 @@ export function useSliderState(props: SliderStateOptions): SliderState {
minValue = DEFAULT_MIN_VALUE,
maxValue = DEFAULT_MAX_VALUE,
numberFormatter: formatter,
step = DEFAULT_STEP_VALUE,
pageSize = Math.max((maxValue - minValue) / 10, step)
step = DEFAULT_STEP_VALUE
} = props;

// Page step should be at least equal to step and always a multiple of the step.
let pageSize = useMemo(() => {
let calcPageSize = (maxValue - minValue) / 10;
calcPageSize = snapValueToStep(calcPageSize, 0, calcPageSize + step, step);
return Math.max(calcPageSize, step);
}, [step, maxValue, minValue]);

const [values, setValues] = useControlledState<number[]>(
props.value as any,
props.defaultValue ?? [minValue] as any,
Expand Down
7 changes: 1 addition & 6 deletions packages/@react-types/slider/src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,7 @@ export interface SliderProps<T = number[]> extends RangeInputBase<number>, Value
* The slider's step value.
* @default 1
*/
step?: number,
/**
* The slider's page step value, used when incrementing and decrementing.
* @default 1
*/
pageSize?: number
step?: number
}

export interface SliderThumbProps extends FocusableProps, Validation, LabelableProps {
Expand Down