[Slider] Set CSS writing-mode and update vertical slider docs
#44537
+71
−140
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mj12albert
added
the
component: slider
mj12albert
changed the title
writing-mode and update vertical slider docs
mj12albert
force-pushed
the
fix/vertical-slider-a11y
branch
from
4b5574e
to
5c830c3
Compare
Netlify deploy previewBundle size reportDetails of bundle changes (Toolpad) |
mj12albert
force-pushed
the
fix/vertical-slider-a11y
branch
3 times, most recently
from
e3164ed
to
0c864c7
Compare
Comment on lines
+690
to
+695
| const cssWritingMode = React.useMemo(() => { | ||
| if (orientation === 'vertical') { | ||
| return isRtl ? 'vertical-rl' : 'vertical-lr'; | ||
| } | ||
| return undefined; | ||
| }, [isRtl, orientation]); |
There was a problem hiding this comment.
Do we really need React.useMemo here? It's handling just an if statement and useMemo itself probably does more computation than this.
| 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 { |
There was a problem hiding this comment.
Would .MuiSlider-thumb input be a better selector? e.g. if the internal DOM structure changes and input is not a direct child in the future, this would break. And since afaik there can only be one input it's safe to assume that it will always target the input.
| **WARNING**: Chrome, Safari and newer Edge versions that is any browser based on WebKit exposes `<Slider orientation="vertical" />` 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 implements `aria-orientation` incorrrectly for vertical sliders and exposes them as `'horizontal'` in the accessibility tree. ([Chromium issue #40736841](https://issues.chromium.org/issues/40736841)) |
There was a problem hiding this comment.
Suggested change
| Chrome versions below 124 implements `aria-orientation` incorrrectly for vertical sliders and exposes them as `'horizontal'` in the accessibility tree. ([Chromium issue #40736841](https://issues.chromium.org/issues/40736841)) | |
| Chrome versions below 124 implements `aria-orientation` incorrectly for vertical sliders and exposes them as `'horizontal'` in the accessibility tree. ([Chromium issue #40736841](https://issues.chromium.org/issues/40736841)) |
| **WARNING**: Chrome, Safari and newer Edge versions that is any browser based on WebKit exposes `<Slider orientation="vertical" />` 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 implements `aria-orientation` incorrrectly for vertical sliders and exposes them as `'horizontal'` in the accessibility tree. ([Chromium issue #40736841](https://issues.chromium.org/issues/40736841)) |
There was a problem hiding this comment.
Do you know why we actively support Chrome 109? Why the jump from 109 to 119 in https://github.com/mui/material-ui/blob/v6.1.8/.browserslistrc#L28-L29? I wonder if this warning makes sense now that Chrome is already on v131.
mj12albert
force-pushed
the
fix/vertical-slider-a11y
branch
from
0c864c7
to
fe585a2
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #44237