Feat: Handle accessibility for crossing over thumbs#97
Feat: Handle accessibility for crossing over thumbs#97linhvovan29546 wants to merge 2 commits intoSharcoux:masterfrom
Conversation
src/components/Thumb.tsx
Outdated
| // Accessibility actions | ||
| const accessibilityActions = useEvent((event: RN.AccessibilityActionEvent) => { | ||
| const tenth = (maximumValue - minimumValue) / 10 | ||
| const tenth = Math.round((maximumValue - minimumValue) / 10) |
There was a problem hiding this comment.
This will break sliders relying on small values, like between 0 and 1.
There was a problem hiding this comment.
@Sharcoux In some cases, I need to round that value, and I believe others may have the same requirement. Therefore, my approach is to add a new prop function (e.g., formatAccessibilityValue) to format it accordingly. What are your thoughts on this?
There was a problem hiding this comment.
If you want to round the value, simply use "step". It was made for that.
There was a problem hiding this comment.
In my case, if my step is 1, and my range is large, from 0 to 999. I don't think increasing/decreasing the value by 1 is ideal.
There was a problem hiding this comment.
In that case, you can use the method return by useRounding. That method takes into account the min and max and step to decide the best rounding to apply.
src/components/Thumb.tsx
Outdated
| switch (event.nativeEvent.actionName) { | ||
| case 'increment': | ||
| updateValue(value + (step || tenth)) | ||
| updateValue(+(step || tenth)) |
There was a problem hiding this comment.
It would be better to make updateValue be a SetStateAction<number>, accepting both a number or a function.
There was a problem hiding this comment.
@Sharcoux I don't really understand SetStateAction<number>, and if I change that, I believe it will affect many old functions.
There was a problem hiding this comment.
SetStateAction is the type of the function returned by React.useState. For instance:
const [value, setValue] = React.useState(0)
Here, setValue is a SetStateAction that can accept setValue(1) as well as setValue(oldValue => oldValue + 1)
| onPress={onPress} onMove={onMove} onRelease={onRelease} | ||
| onPress={(value) => { | ||
| // We need to blur the min/max thumb if it is focused when the user interacts with the slider | ||
| blurThumbs() |
There was a problem hiding this comment.
I find this suspicious. What would have the focuse, then?
There was a problem hiding this comment.
@Sharcoux In the example, if I focus on the min thumb, and then drag this thumb over the max thumb, you will see that the min thumb remains focused while I'm interacting with the max thumb.
Screen.Recording.2024-05-23.at.5.08.06.PM.mov
| > | ||
| <Track color={outboundColor} style={minStyle} length={minTrackPct * 100} vertical={vertical} thickness={trackHeight} /> | ||
| <Thumb key='min' {...thumbProps} updateValue={updateMinValue} value={min} thumb='min' /> | ||
| <Thumb key='min' {...thumbProps} thumbRef={minThumbRef} updateValue={(value) => crossingAllowed ? updateAccessibilityMinValue(value) : updateMinValue(min + value)} value={min} thumb='min' /> |
There was a problem hiding this comment.
I think that a better approach would be to uniquely identify each thumb with their "key", and order them based on their current value. This would require accepting the range to be reverted internally: [3, 1] would be a valid range internally/
There was a problem hiding this comment.
@Sharcoux If I order them based on their current value with their 'key', how can I focus on the max/min thumbs if I increase/decrease the value beyond them?
There was a problem hiding this comment.
If you order them based on their current value, the thumb would not change, so you would not need to handle the focus switch.
2 participants
Ref: #95
Changes in this PR
Screenshots
Screen.Recording.2024-05-04.at.10.52.22.AM.mov