base-ui
base-ui copied to clipboard
[RFC] Implement onInput and onChange for Slider
We currently have separate onDragStart, onDragEnd and onChange events. Combined with the native onClick these cause a somewhat unintuitive call order.
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.
I think a simpler solution would be to actually implement onInput and onChange listeners. This means that no onChange is fired during drag anymore. We would however fire onInput throughout this process.
An example of a user interface involving both interactive manipulation and a commit action would be a Range controls that use a slider, when manipulated using a pointing device. While the user is dragging the control's knob, input events would fire whenever the position changed, whereas the change event would only fire when the user let go of the knob, committing to a specific value. -- https://html.spec.whatwg.org/multipage/input.html#common-input-element-events
We could get rid of some bad ergonomics with onDrag* handlers as well as enabling cross-browser behavior for onInput and onChange.
Other resources:
- https://stackoverflow.com/a/19067260/3406963
- https://www.w3.org/Bugs/Public/show_bug.cgi?id=21377
Search keywords:
@eps1lon How does it fit into https://github.com/facebook/react/issues/9657?
Slider is a controlled component. This would mean that we should only allow onInput but warn against onChange. That means we just drop onDrag*.
I think this is what people mean by "react doesn't care about the spec". While this is hyperbolic it's true for this case. The spec is very clear about the onChange onInput difference. If react doesn't want to enable cross-browser compliance, fine. Changing the behavior, not so fine.
I don't think it's good to warn for onChange behavior because type="range" has a very clear use case for this type of event. Considering we acknowledge the onInput vs. onChange necessity with the onDrag handlers it might be a good idea to follow the spec here and document the rationale.
Maybe we need to fundamentally decide what we view as our spec: The html spec or the spec that is derived from react-dom implementation.
This would solve mui/material-ui#13132 for me. As a library user, I always assume the DOM spec first before checking how react does things.
Another thing to consider is keyboard interaction – it should trigger both an "input" and "change" event.
See for yourself: https://jsfiddle.net/pon4uvjw/
@dpren
As a library user, I always assume the DOM spec first before checking how react does things.
Preferably but with React Fire we won't have onChange anyway. I do however recognize the need for onInput for sliders so I'd like to properly polyfill it (in favor of non-standard onDrag* we have).
Another thing to consider is keyboard interaction – it should trigger both an "input" and "change" event.
Thanks, haven't really thought about it. The important distinction here is that continuous keydown will fire both input and change while mousedown + move will only fire input. This is specifically mentioned in the spec:
when manipulated using a pointing device
Any other change will have an input immediately before it.
I am using onInput={(e) => { e.target.value = e.target.value.replace(/[^0-9]/g, '') }} /> But I need 2 decimal places
This will be decided by https://github.com/mui/base-ui, so I'm closing this. Material UI will follow the API from Base UI.
This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue. Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.
I'm reopening, closed issues should either be:
- solved with a PR/Link that proves it, or a justification to why it's not something that should be solved.
- have them shifted somewhere else, with a link to that new ongoing work. e.g. downstream issue in Chromium
This looks more like case 2. but there are no issues to carry this from, so this needs to stay open. We could transfer this to https://github.com/mui/base-ui, this could work.
@siriwatknp How about we transfer this to the Base UI repository?
@oliviertassinari sounds good to me.
The Base UI slider has onValueChange and onValueCommitted. While they're not the same as proposed props, it seems to me that they serve the same purpose.