[Slider] Major upgrade

[oliviertassinari]

This is a work in progress: 97% (44 hours): https://deploy-preview-15703--material-ui.netlify.com/components/slider/.

• Closes [Slider] Track border radius incorrect when moving the slider icon #15803 - Change the DOM & CSS structure.
• Closes [Slider] Add Range input #779 - Add support for range input.
• Closes [Slider] Normalize onChange event parameters #11990 - Use the onChange(event, value) signature. It's not the first time we use it. It's also the one suggested in [RFC] onChange -> onInput, and don't polyfill onInput for uncontrolled components facebook/react#9657.
• Closes [Slider] The sliders do not follow the Material Design Spec #12463 - I have done my best without making the implementation too complex.
• Closes [Lab][Slider] - Empower Multi-Touch-Behaviour by exposing an id to props #12807 - We already support multi-touch.
• Closes [Slider] Missing name property? #13485 - The component doesn't have a name as it's not a native element, people have to account for it when using react-final-form or formik. However, I have added a hidden input for people who want to submit an HTML form.
• Closes [Slider] Improve touch passive event handling #13623 - I had to use a non-passive listener.
• Closes Slider: Provide index/percent of item under mouse onMouseOver #14469 - The thumb label was added for this use case.
• Closes [Slider] The touch event is bubbled up to parent controls #14913 - This is an issue with react-swipeable-views. It could support a specific data- attribute for the problem. I don't think that the Slider should account for it.
• Closes [Slider] Slider gets blocked from invisible highlighting #15889 - Better handle the touch start event.
  - Closes Popper can't update position when content height changed #15802 - Expose a popperRef prop.
• Related to [RFC] Implement onInput and onChange for Slider #14674 - I'm not sure about this one, hence it's the last one listed. I would like you feedback on it @eps1lon. I was thinking of: we keep onChange for consistency with the other components. We introduce an onAfterChange prop for the case described:

How I understand the use cases of the onDrag* events is to start validation or some other expensive operation only after the user has ended the drag.

Then, we wait for facebook/react#9657 to migrate all our input components.

[eps1lon]

I wouldn't close #14674 at this point. As I understood React Fire onChange will be going away at which point this will make more sense. I would suggest onCommitChange however which is a more verbose name for how onChange actually works.

[ryancogswell]

@oliviertassinari This looks great! I actually just found out today about a client request where they want a slider for something in our app.

Are you planning on providing any support for Value entry field in the initial version?

[oliviertassinari]

@ryancogswell It probably still need a day of work. I don't see anything specific with the value entry case. I have the following so far: https://deploy-preview-15703--material-ui.netlify.com/components/slider/.

[ryancogswell]

I don't see anything specific with the value entry case.

@oliviertassinari Sorry, I'm not sure I understand what you're saying. Are you saying you aren't doing anything yet for the value entry case, or that you don't see anything in the spec about the value entry case (i.e. ability to do text entry of the value when the slider has focus), or that you don't think this case needs anything specific within Slider to support?

[oliviertassinari]

I don't think that it needs anything specific within the Slider support. What prevents you from having this use case with the existing lab component?

[ryancogswell]

What prevents you from having this use case with the existing lab component?

@oliviertassinari I'm sure it's possible, but there are enough layout/styling concerns and potential complexities with tapping into the onKeyDown of the Slider and determining when to register a change to the Slider and honoring the step of the Slider, that it would be valuable for the component to take care of all of that for the user. We'll probably want the text entry option in our app, so I'll eventually find out how much is involved; though, at least initially, I'll probably avoid the complexity of trying to have text entry work when focus is on the thumb.

[oliviertassinari]

OK, I will add a demo. It should be about 1. Using the Grid component, 2. Using a debounce on the input changes. 3. Using the step prop for the input number type.

[mui-pr-bot]

@material-ui/lab: parsed: +1.08% , gzip: +1.45%

Details of bundle changes.

Comparing: 07b9d6e...d2bc0dc

bundle	parsed diff	gzip diff	prev parsed	current parsed	prev gzip	current gzip
@material-ui/core	-0.00%	-0.09%	318,556	318,551	87,551	87,472
@material-ui/core/Paper	-0.00%	-0.03%	68,345	68,344	20,371	20,365
@material-ui/core/Paper.esm	0.00%	-0.16%	61,639	61,641	19,173	19,142
@material-ui/core/Popper	-0.00%	-0.07%	28,966	28,965	10,415	10,408
@material-ui/core/Textarea	-0.04%	+0.17% 🔺	5,513	5,511	2,370	2,374
@material-ui/core/TrapFocus	0.00%	-0.06%	3,755	3,755	1,581	1,580
@material-ui/core/styles/createMuiTheme	-0.02%	-0.52%	16,016	16,012	5,821	5,791
@material-ui/core/useMediaQuery	-0.08%	-0.09%	2,529	2,527	1,088	1,087
@material-ui/lab	+1.08% 🔺	+1.45% 🔺	138,981	140,477	42,767	43,388
@material-ui/styles	0.00%	+0.02% 🔺	51,765	51,766	15,346	15,349
@material-ui/system	-0.01%	+0.05% 🔺	14,825	14,823	4,241	4,243
Button	0.00%	-0.28%	84,274	84,274	25,679	25,608
Modal	0.00%	+0.10% 🔺	20,343	20,344	6,682	6,689
Slider	+Infinity% 🔺	+Infinity% 🔺	0	74,649	0	23,196
colorManipulator	0.00%	0.00%	3,904	3,904	1,544	1,544
docs.landing	0.00%	0.00%	55,232	55,232	13,947	13,947
docs.main	+0.01% 🔺	0.00%	652,812	652,885	206,694	206,701
packages/material-ui/build/umd/material-ui.production.min.js	0.00%	0.00%	291,991	291,991	83,425	83,425

Generated by 🚫 dangerJS against d2bc0dc

Updated

[eps1lon]

After clicking it still responds to keyboard input. This is a regression from the previous version and doesn't follow wai-aria authoring practices.

[oliviertassinari]

After clicking it still responds to keyboard input. This is a regression from the previous version and doesn't follow wai-aria authoring practices.

@eps1lon I don't understand, can you clarify? In the following. The value starts around 0, I click around 80%. I press the up key command, then the down key command two times.

• <input type="range" />
  

• <Slider />
  

[eps1lon]

Yeah you understood it. Native browser implementation is not following wai-aria as well. If there's a W3 spec for that behavior we can keep it (but need to display label again). So far browser implementations are most often not following specs or are different between vendors. They shouldn't be the most important reference.

[ryancogswell]

After clicking it still responds to keyboard input.

@eps1lon Can you clarify what the desired behavior is and which wai-aria authoring practices you are referring to? The wai-aria example (https://www.w3.org/TR/wai-aria-practices/examples/slider/slider-2.html) seems to behave the same way unless I'm misunderstanding what aspect you are focusing on.

[mbrookes]

Re the review request - nothing further from me at this point.

[ryancogswell]

Can you clarify what the desired behavior

@eps1lon I just answered my own question with a little experimenting.

To clarify for others, in the wai-aria example, if you click on the Slider thumb then keyboard focus is placed on the Slider (same as in this branch), but if you click elsewhere on the Slider it just changes the Slider position without giving it focus.

When I was first trying to understand your point, I was clicking on the thumb and dragging it to change the value and then using the keyboard to change it further. In that case, keyboard focus behaves the same for the wai-aria example and this branch.

[joshwooding]

The signature looks to be missing here

[oliviertassinari]

Yes, it's an issue with the markdown genetation tool.

[eps1lon]

There's no visual indication that the thumb is focused (i.e. no focus-visible) until a key is pressed after clicking on the rail. You can see this in Oliviers video.

Basically clicking the rail should/did not focus the thumb. If that behavior is described in the W3C spec we should stick to it but make the focus-visible styling consistent. But so far this is a regression from previous behavior and wai-aria authoring practices.

[ryancogswell]

There's no visual indication that the thumb is focused (i.e. no focus-visible) until a key is pressed after clicking on the rail.

@eps1lon I more fully understand what you're getting at now, but it is still unclear to me whether there is a problem with the new behavior.

There are two main questions to answer:

1. Should the focus-visible styling be in effect when receiving focus via a mouse click?

   The answer to this for buttons has been "no". Clicking a button gives it keyboard focus, but does not give it focus-visible styling; whereas tabbing to a button gives it focus-visible styling. The current behavior in this branch is consistent with that approach and it makes sense to me that the focus-visible styling would then turn on when you do further interaction with the slider via the keyboard.

2. Should clicking the rail give focus to the slider?

   The wai-aria example only gives focus to the slider when clicking on the thumb and not when clicking on the rail, but the design guidelines don't say anything about this aspect. Either behavior (giving focus only when clicking on the thumb vs. giving focus when clicking on the thumb or the rail) seems reasonable to me.

   I like having a click on the rail give focus since it allows for clicking in the approximate location and then fine-tuning using the arrow keys.

[oliviertassinari]

I support @ryancogswell points here. In the light of contradictions between different sources, I think that we should follow them in this order:

1. The wai-aria written recommendations.
2. The browser behavior, what I have shown with macOS and Chrome can be reproduced with Windows 10 and Edge. Two very different environments. What I haven't tried but would love to is: old school heavy Windows application.
3. The wai-aria examples' behavior.
4. Other libraries' behavior.

[eps1lon]

I propose that we discuss this in a separate issue. I would put the W3 spec above all the points listed above and move browser implementation to the bottom (again those have proven to be a bad reference for normalized behavior) but we can compare this at length later.

[oliviertassinari]

Should we raise the problem in https://github.com/w3c/aria-practices/issues?

[oliviertassinari]

This is one step further. We are not done yet!

[Astrantia]

@oliviertassinari @eps1lon where did jumped pseudoselector go?

[oliviertassinari]

@Astrantia This jumped custom pseudo-class was removed. This was an attempt to simplify the component. Could you describe your use case for using it?