diff --git a/apps/pigment-css-next-app/src/app/material-ui/react-slider/page.tsx b/apps/pigment-css-next-app/src/app/material-ui/react-slider/page.tsx index 97fb8108c1cb09..19c93224b7a523 100644 --- a/apps/pigment-css-next-app/src/app/material-ui/react-slider/page.tsx +++ b/apps/pigment-css-next-app/src/app/material-ui/react-slider/page.tsx @@ -17,7 +17,6 @@ import RangeSlider from '../../../../../../docs/data/material/components/slider/ import SliderSizes from '../../../../../../docs/data/material/components/slider/SliderSizes'; import TrackFalseSlider from '../../../../../../docs/data/material/components/slider/TrackFalseSlider'; import TrackInvertedSlider from '../../../../../../docs/data/material/components/slider/TrackInvertedSlider'; -import VerticalAccessibleSlider from '../../../../../../docs/data/material/components/slider/VerticalAccessibleSlider'; import VerticalSlider from '../../../../../../docs/data/material/components/slider/VerticalSlider'; export default function Slider() { @@ -125,12 +124,6 @@ export default function Slider() { -
-

Vertical Accessible Slider

-
- -
-

Vertical Slider

diff --git a/apps/pigment-css-vite-app/src/pages/material-ui/react-slider.tsx b/apps/pigment-css-vite-app/src/pages/material-ui/react-slider.tsx index 963612339e2faf..a3bfac23c4c197 100644 --- a/apps/pigment-css-vite-app/src/pages/material-ui/react-slider.tsx +++ b/apps/pigment-css-vite-app/src/pages/material-ui/react-slider.tsx @@ -17,7 +17,6 @@ import RangeSlider from '../../../../../docs/data/material/components/slider/Ran import SliderSizes from '../../../../../docs/data/material/components/slider/SliderSizes.tsx'; import TrackFalseSlider from '../../../../../docs/data/material/components/slider/TrackFalseSlider.tsx'; import TrackInvertedSlider from '../../../../../docs/data/material/components/slider/TrackInvertedSlider.tsx'; -import VerticalAccessibleSlider from '../../../../../docs/data/material/components/slider/VerticalAccessibleSlider.tsx'; import VerticalSlider from '../../../../../docs/data/material/components/slider/VerticalSlider.tsx'; export default function Slider() { @@ -126,12 +125,6 @@ export default function Slider() {
-
-

Vertical Accessible Slider

-
- -
-

Vertical Slider

diff --git a/docs/data/material/components/slider/VerticalAccessibleSlider.js b/docs/data/material/components/slider/VerticalAccessibleSlider.js deleted file mode 100644 index e9892bcc838cdd..00000000000000 --- a/docs/data/material/components/slider/VerticalAccessibleSlider.js +++ /dev/null @@ -1,28 +0,0 @@ -import * as React from 'react'; -import Box from '@mui/material/Box'; -import Slider from '@mui/material/Slider'; - -export default function VerticalAccessibleSlider() { - function preventHorizontalKeyboardNavigation(event) { - if (event.key === 'ArrowLeft' || event.key === 'ArrowRight') { - event.preventDefault(); - } - } - - return ( - - - - ); -} diff --git a/docs/data/material/components/slider/VerticalAccessibleSlider.tsx b/docs/data/material/components/slider/VerticalAccessibleSlider.tsx deleted file mode 100644 index bc66892e7f20b3..00000000000000 --- a/docs/data/material/components/slider/VerticalAccessibleSlider.tsx +++ /dev/null @@ -1,28 +0,0 @@ -import * as React from 'react'; -import Box from '@mui/material/Box'; -import Slider from '@mui/material/Slider'; - -export default function VerticalAccessibleSlider() { - function preventHorizontalKeyboardNavigation(event: React.KeyboardEvent) { - if (event.key === 'ArrowLeft' || event.key === 'ArrowRight') { - event.preventDefault(); - } - } - - return ( - - - - ); -} diff --git a/docs/data/material/components/slider/VerticalAccessibleSlider.tsx.preview b/docs/data/material/components/slider/VerticalAccessibleSlider.tsx.preview deleted file mode 100644 index 2f23a0596d7eee..00000000000000 --- a/docs/data/material/components/slider/VerticalAccessibleSlider.tsx.preview +++ /dev/null @@ -1,12 +0,0 @@ - \ No newline at end of file diff --git a/docs/data/material/components/slider/VerticalSlider.js b/docs/data/material/components/slider/VerticalSlider.js index 502b71bdb8bb65..5ae94100d85d93 100644 --- a/docs/data/material/components/slider/VerticalSlider.js +++ b/docs/data/material/components/slider/VerticalSlider.js @@ -2,36 +2,13 @@ import * as React from 'react'; import Stack from '@mui/material/Stack'; import Slider from '@mui/material/Slider'; -function valuetext(value) { - return `${value}°C`; -} - -const marks = [ - { - value: 0, - label: '0°C', - }, - { - value: 20, - label: '20°C', - }, - { - value: 37, - label: '37°C', - }, - { - value: 100, - label: '100°C', - }, -]; - export default function VerticalSlider() { return ( @@ -45,7 +22,7 @@ export default function VerticalSlider() { 'Temperature'} orientation="vertical" - getAriaValueText={valuetext} + getAriaValueText={getAriaValueText} defaultValue={[20, 37]} valueLabelDisplay="auto" marks={marks} @@ -53,3 +30,26 @@ export default function VerticalSlider() { ); } + +function getAriaValueText(value) { + return `${value}°C`; +} + +const marks = [ + { + value: 0, + label: '0°C', + }, + { + value: 20, + label: '20°C', + }, + { + value: 37, + label: '37°C', + }, + { + value: 100, + label: '100°C', + }, +]; diff --git a/docs/data/material/components/slider/VerticalSlider.tsx b/docs/data/material/components/slider/VerticalSlider.tsx index 2f4e4ffa167ca8..743b7103b07703 100644 --- a/docs/data/material/components/slider/VerticalSlider.tsx +++ b/docs/data/material/components/slider/VerticalSlider.tsx @@ -2,36 +2,13 @@ import * as React from 'react'; import Stack from '@mui/material/Stack'; import Slider from '@mui/material/Slider'; -function valuetext(value: number) { - return `${value}°C`; -} - -const marks = [ - { - value: 0, - label: '0°C', - }, - { - value: 20, - label: '20°C', - }, - { - value: 37, - label: '37°C', - }, - { - value: 100, - label: '100°C', - }, -]; - export default function VerticalSlider() { return ( @@ -45,7 +22,7 @@ export default function VerticalSlider() { 'Temperature'} orientation="vertical" - getAriaValueText={valuetext} + getAriaValueText={getAriaValueText} defaultValue={[20, 37]} valueLabelDisplay="auto" marks={marks} @@ -53,3 +30,26 @@ export default function VerticalSlider() { ); } + +function getAriaValueText(value: number) { + return `${value}°C`; +} + +const marks = [ + { + value: 0, + label: '0°C', + }, + { + value: 20, + label: '20°C', + }, + { + value: 37, + label: '37°C', + }, + { + value: 100, + label: '100°C', + }, +]; diff --git a/docs/data/material/components/slider/slider.md b/docs/data/material/components/slider/slider.md index aa6afc620b1e17..13dffd9f9b337e 100644 --- a/docs/data/material/components/slider/slider.md +++ b/docs/data/material/components/slider/slider.md @@ -98,17 +98,22 @@ You can learn more about this in the [overrides documentation page](/material-ui ## Vertical sliders +Set the `orientation` prop to `"vertical"` to create vertical sliders. The thumb will track vertical movement instead of horizontal movement. + {{"demo": "VerticalSlider.js"}} -**WARNING**: Chrome, Safari and newer Edge versions that is any browser based on WebKit exposes `` as horizontal ([chromium issue #40736841](https://issues.chromium.org/issues/40736841)). -By applying `-webkit-appearance: slider-vertical;` the slider is exposed as vertical. +:::warning +Chrome versions below 124 implement `aria-orientation` incorrectly for vertical sliders and expose them as `'horizontal'` in the accessibility tree. ([Chromium issue #40736841](https://issues.chromium.org/issues/40736841)) + +The `-webkit-appearance: slider-vertical` CSS property can be used to correct this for these older versions, with the trade-off of causing a console warning in newer Chrome versions: -However, by applying `-webkit-appearance: slider-vertical;` keyboard navigation for horizontal keys (Arrow Left, Arrow Right) is reversed ([chromium issue #40739626](https://issues.chromium.org/issues/40739626)). -Usually, up and right should increase and left and down should decrease the value. -If you apply `-webkit-appearance` you could prevent keyboard navigation for horizontal arrow keys for a truly vertical slider. -This might be less confusing to users compared to a change in direction. +```css +.MuiSlider-thumb input { + -webkit-appearance: slider-vertical; +} +``` -{{"demo": "VerticalAccessibleSlider.js"}} +::: ## Marks placement diff --git a/packages/mui-material/src/Slider/Slider.test.js b/packages/mui-material/src/Slider/Slider.test.js index 8b8d868434dbbd..a8c35939986ffd 100644 --- a/packages/mui-material/src/Slider/Slider.test.js +++ b/packages/mui-material/src/Slider/Slider.test.js @@ -421,41 +421,43 @@ describe('', () => { }); describe('prop: step', () => { - it('should handle a null step', () => { - const { getByRole, container } = render( - , - ); - stub(container.firstChild, 'getBoundingClientRect').callsFake(() => ({ - width: 100, - height: 10, - bottom: 10, - left: 0, - })); - const slider = getByRole('slider'); - - fireEvent.touchStart( - container.firstChild, - createTouches([{ identifier: 1, clientX: 21, clientY: 0 }]), - ); - expect(slider).to.have.attribute('aria-valuenow', '20'); + describe('when step is `null`', () => { + it('values are defined by mark values', () => { + const { getByRole, container } = render( + , + ); + stub(container.firstChild, 'getBoundingClientRect').callsFake(() => ({ + width: 100, + height: 10, + bottom: 10, + left: 0, + })); + const slider = getByRole('slider'); + + fireEvent.touchStart( + container.firstChild, + createTouches([{ identifier: 1, clientX: 21, clientY: 0 }]), + ); + expect(slider).to.have.attribute('aria-valuenow', '20'); - fireEvent.change(slider, { - target: { - value: 21, - }, - }); - expect(slider).to.have.attribute('aria-valuenow', '30'); + fireEvent.change(slider, { + target: { + value: 21, + }, + }); + expect(slider).to.have.attribute('aria-valuenow', '30'); - fireEvent.change(slider, { - target: { - value: 29, - }, + fireEvent.change(slider, { + target: { + value: 29, + }, + }); + expect(slider).to.have.attribute('aria-valuenow', '20'); }); - expect(slider).to.have.attribute('aria-valuenow', '20'); }); it('change events with non integer numbers should work', () => { @@ -988,6 +990,213 @@ describe('', () => { }); }); + describe('keyboard interactions', () => { + [ + ['ltr', 'horizontal', ['ArrowLeft', 'ArrowDown'], ['ArrowRight', 'ArrowUp']], + ['ltr', 'vertical', ['ArrowLeft', 'ArrowDown'], ['ArrowRight', 'ArrowUp']], + ['rtl', 'horizontal', ['ArrowRight', 'ArrowDown'], ['ArrowLeft', 'ArrowUp']], + ['rtl', 'vertical', ['ArrowRight', 'ArrowDown'], ['ArrowLeft', 'ArrowUp']], + ].forEach((entry) => { + const [direction, orientation, decrementKeys, incrementKeys] = entry; + + describe(direction, () => { + describe(`orientation: ${orientation}`, () => { + decrementKeys.forEach((key) => { + it(`key: ${key} decrements the value`, () => { + const { getByRole } = render( + + + , + ); + + const slider = getByRole('slider'); + expect(slider).to.have.attribute('aria-valuenow', '50'); + + act(() => { + slider.focus(); + }); + + fireEvent.keyDown(slider, { key }); + expect(slider).to.have.attribute('aria-valuenow', '49'); + + fireEvent.keyDown(slider, { key }); + expect(slider).to.have.attribute('aria-valuenow', '48'); + }); + }); + + incrementKeys.forEach((key) => { + it(`key: ${key} increments the value`, () => { + const { getByRole } = render( + + + , + ); + + const slider = getByRole('slider'); + expect(slider).to.have.attribute('aria-valuenow', '50'); + + act(() => { + slider.focus(); + }); + + fireEvent.keyDown(slider, { key }); + expect(slider).to.have.attribute('aria-valuenow', '51'); + + fireEvent.keyDown(slider, { key }); + expect(slider).to.have.attribute('aria-valuenow', '52'); + }); + }); + + it('key: PageUp and key: PageDown change the value based on `shiftStep`', () => { + const { getByRole } = render( + + + , + ); + + const slider = getByRole('slider'); + expect(slider).to.have.attribute('aria-valuenow', '50'); + + act(() => { + slider.focus(); + }); + + fireEvent.keyDown(slider, { key: 'PageUp' }); + expect(slider).to.have.attribute('aria-valuenow', '55'); + + fireEvent.keyDown(slider, { key: 'PageDown' }); + expect(slider).to.have.attribute('aria-valuenow', '50'); + + fireEvent.keyDown(slider, { key: 'PageDown' }); + expect(slider).to.have.attribute('aria-valuenow', '45'); + }); + + it('key: End sets the value to max', () => { + const { getByRole } = render( + + + , + ); + + const slider = getByRole('slider'); + expect(slider).to.have.attribute('aria-valuenow', '50'); + + act(() => { + slider.focus(); + }); + + fireEvent.keyDown(slider, { key: 'End' }); + expect(slider).to.have.attribute('aria-valuenow', '99'); + }); + + it('key: Home sets the value to min', () => { + const { getByRole } = render( + + + , + ); + + const slider = getByRole('slider'); + expect(slider).to.have.attribute('aria-valuenow', '50'); + + act(() => { + slider.focus(); + }); + + fireEvent.keyDown(slider, { key: 'Home' }); + expect(slider).to.have.attribute('aria-valuenow', '1'); + }); + + describe('when `step` is `null` and values are restricted by `marks`', () => { + decrementKeys.forEach((key) => { + it(`key: ${key} decrements the value`, () => { + const { getByRole } = render( + + + , + ); + + const slider = getByRole('slider'); + expect(slider).to.have.attribute('aria-valuenow', '79'); + + act(() => { + slider.focus(); + }); + + fireEvent.keyDown(slider, { key }); + expect(slider).to.have.attribute('aria-valuenow', '29'); + + fireEvent.keyDown(slider, { key }); + expect(slider).to.have.attribute('aria-valuenow', '19'); + }); + }); + + incrementKeys.forEach((key) => { + it(`key: ${key} increments the value`, () => { + const { getByRole } = render( + + + , + ); + + const slider = getByRole('slider'); + expect(slider).to.have.attribute('aria-valuenow', '9'); + + act(() => { + slider.focus(); + }); + + fireEvent.keyDown(slider, { key }); + expect(slider).to.have.attribute('aria-valuenow', '19'); + + fireEvent.keyDown(slider, { key }); + expect(slider).to.have.attribute('aria-valuenow', '29'); + }); + }); + }); + }); + }); + }); + }); + describe('warnings', () => { beforeEach(() => { PropTypes.resetWarningCache(); diff --git a/packages/mui-material/src/Slider/useSlider.ts b/packages/mui-material/src/Slider/useSlider.ts index f3ef2bfeeeafc7..ba951a2b9f3395 100644 --- a/packages/mui-material/src/Slider/useSlider.ts +++ b/packages/mui-material/src/Slider/useSlider.ts @@ -24,6 +24,16 @@ import areArraysEqual from '../utils/areArraysEqual'; const INTENTIONAL_DRAG_COUNT_THRESHOLD = 2; +function getNewValue( + currentValue: number, + step: number, + direction: 1 | -1, + min: number, + max: number, +): number { + return direction === 1 ? Math.min(currentValue + step, max) : Math.max(currentValue - step, min); +} + function asc(a: number, b: number) { return a - b; } @@ -294,8 +304,8 @@ export function useSlider(parameters: UseSliderParameters): UseSliderReturnValue const index = Number(event.currentTarget.getAttribute('data-index')); const value = values[index]; const marksIndex = marksValues.indexOf(value); - let newValue: number | number[] = valueInput; + let newValue: number | number[] = valueInput; if (marks && step == null) { const maxMarksValue = marksValues[marksValues.length - 1]; if (newValue > maxMarksValue) { @@ -345,29 +355,85 @@ export function useSlider(parameters: UseSliderParameters): UseSliderReturnValue }; const createHandleHiddenInputKeyDown = - (otherHandlers: EventHandlers) => (event: React.KeyboardEvent) => { - // The Shift + Up/Down keyboard shortcuts for moving the slider makes sense to be supported - // only if the step is defined. If the step is null, this means tha the marks are used for specifying the valid values. - if (step !== null) { + (otherHandlers: EventHandlers) => (event: React.KeyboardEvent) => { + if ( + [ + 'ArrowUp', + 'ArrowDown', + 'ArrowLeft', + 'ArrowRight', + 'PageUp', + 'PageDown', + 'Home', + 'End', + ].includes(event.key) + ) { + event.preventDefault(); const index = Number(event.currentTarget.getAttribute('data-index')); const value = values[index]; - let newValue = null; - if ( - ((event.key === 'ArrowLeft' || event.key === 'ArrowDown') && event.shiftKey) || - event.key === 'PageDown' - ) { - newValue = Math.max(value - shiftStep, min); - } else if ( - ((event.key === 'ArrowRight' || event.key === 'ArrowUp') && event.shiftKey) || - event.key === 'PageUp' - ) { - newValue = Math.min(value + shiftStep, max); + // Keys actions that change the value by more than the most granular `step` + // value are only applied if the step not `null`. + // When step is `null`, the `marks` prop is used instead to define valid values. + if (step != null) { + const stepSize = event.shiftKey ? shiftStep : step; + switch (event.key) { + case 'ArrowUp': + newValue = getNewValue(value, stepSize, 1, min, max); + break; + case 'ArrowRight': + newValue = getNewValue(value, stepSize, isRtl ? -1 : 1, min, max); + break; + case 'ArrowDown': + newValue = getNewValue(value, stepSize, -1, min, max); + break; + case 'ArrowLeft': + newValue = getNewValue(value, stepSize, isRtl ? 1 : -1, min, max); + break; + case 'PageUp': + newValue = getNewValue(value, shiftStep, 1, min, max); + break; + case 'PageDown': + newValue = getNewValue(value, shiftStep, -1, min, max); + break; + case 'Home': + newValue = min; + break; + case 'End': + newValue = max; + break; + default: + break; + } + } else if (marks) { + const maxMarksValue = marksValues[marksValues.length - 1]; + const currentMarkIndex = marksValues.indexOf(value); + + const decrementKeys = [ + isRtl ? 'ArrowRight' : 'ArrowLeft', + 'ArrowDown', + 'PageDown', + 'Home', + ]; + const incrementKeys = [isRtl ? 'ArrowLeft' : 'ArrowRight', 'ArrowUp', 'PageUp', 'End']; + + if (decrementKeys.includes(event.key)) { + if (currentMarkIndex === 0) { + newValue = marksValues[0]; + } else { + newValue = marksValues[currentMarkIndex - 1]; + } + } else if (incrementKeys.includes(event.key)) { + if (currentMarkIndex === marksValues.length - 1) { + newValue = maxMarksValue; + } else { + newValue = marksValues[currentMarkIndex + 1]; + } + } } - if (newValue !== null) { + if (newValue != null) { changeValue(event, newValue); - event.preventDefault(); } } @@ -394,6 +460,7 @@ export function useSlider(parameters: UseSliderParameters): UseSliderReturnValue const createHandleHiddenInputChange = (otherHandlers: EventHandlers) => (event: React.ChangeEvent) => { otherHandlers.onChange?.(event); + // this handles value change by Pointer or Touch events // @ts-ignore changeValue(event, event.target.valueAsNumber); }; @@ -687,6 +754,11 @@ export function useSlider(parameters: UseSliderParameters): UseSliderReturnValue }; }; + let cssWritingMode: 'vertical-rl' | 'vertical-lr' | undefined; + if (orientation === 'vertical') { + cssWritingMode = isRtl ? 'vertical-rl' : 'vertical-lr'; + } + const getHiddenInputProps = = {}>( externalProps: ExternalProps = {} as ExternalProps, ): UseSliderHiddenInputProps => { @@ -724,6 +796,7 @@ export function useSlider(parameters: UseSliderParameters): UseSliderReturnValue // So that VoiceOver's focus indicator matches the thumb's dimensions width: '100%', height: '100%', + writingMode: cssWritingMode, }, }; }; diff --git a/test/regressions/index.js b/test/regressions/index.js index 1f069dc1bd703f..316406a9456dc9 100644 --- a/test/regressions/index.js +++ b/test/regressions/index.js @@ -178,7 +178,6 @@ const blacklist = [ 'docs-components-skeleton/Facebook.png', // Flaky image loading 'docs-components-skeleton/SkeletonChildren.png', // flaky image loading 'docs-components-skeleton/YouTube.png', // Flaky image loading - 'docs-components-slider/VerticalAccessibleSlider.png', // Redundant 'docs-components-snackbars/ConsecutiveSnackbars.png', // Needs interaction 'docs-components-snackbars/CustomizedSnackbars.png', // Redundant 'docs-components-snackbars/DirectionSnackbar.png', // Needs interaction