Skip to content

Conversation

@linhvovan29546
Copy link

Ref: #95

Changes in this PR

  • Handle Handle accessibility for crossing over thumbs

Screenshots

Screen.Recording.2024-05-04.at.10.52.22.AM.mov

@linhvovan29546
Copy link
Author

Hi @Sharcoux I've created this PR to address accessibility when crossingAllowed=true. Ref: #95. Could you please review it?

// Accessibility actions
const accessibilityActions = useEvent((event: RN.AccessibilityActionEvent) => {
const tenth = (maximumValue - minimumValue) / 10
const tenth = Math.round((maximumValue - minimumValue) / 10)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will break sliders relying on small values, like between 0 and 1.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to round the value, simply use "step". It was made for that.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

switch (event.nativeEvent.actionName) {
case 'increment':
updateValue(value + (step || tenth))
updateValue(+(step || tenth))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to make updateValue be a SetStateAction<number>, accepting both a number or a function.

Copy link
Author

@linhvovan29546 linhvovan29546 May 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Sharcoux I don't really understand SetStateAction<number>, and if I change that, I believe it will affect many old functions.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find this suspicious. What would have the focuse, then?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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' />
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you order them based on their current value, the thumb would not change, so you would not need to handle the focus switch.

@linhvovan29546
Copy link
Author

Hi @Sharcoux I've created this PR to address accessibility when crossingAllowed=true. Ref: #95. Could you please review it?
@Sharcoux Okay, thanks for the review. I will consider some of your approaches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants