react-native-web icon indicating copy to clipboard operation
react-native-web copied to clipboard

Setting a Pressable as "disabled" from enter key press locks its internal `focused` state as `true`

Open AlanSl opened this issue 3 years ago • 4 comments
trafficstars

Is there an existing issue for this?

  • [X] I have searched the existing issues

Might be a side effect of the fix for https://github.com/necolas/react-native-web/issues/2127

Describe the issue

If a Pressable's onPress function causes it to become disabled, and this is triggered by tabbing in with the keyboard and triggering the onPress function from the keyboard using the Enter key, then the Pressable's internal focused state becomes locked as true even after the actual HTML element has blurred.

For example, here, the first Pressable was disabled and blurred using the Enter key and the second Pressable is the focused element, but the first Pressable thinks it is focused still.

image

Expected behavior

A Pressable's focused state should not be true if it is not focused, regardless of how it came to be not focused, and it should never be both disabled and focused.

Steps to reproduce

  • Create a Pressable that shows its internal focused state visibly
  • Set its disabled state to true in an onPress handler
  • Select it using the keyboard tab key and trigger the onPress handler with the Enter key

Test case

https://codesandbox.io/s/quirky-williamson-v3r94g https://snack.expo.dev/@alansl/amused-cheese

Additional comments

There's also a possibly-related issue described in comments in the linked example, where it's possible to programmatically focus a disabled Pressable just before disabled is removed, and then when disabled is removed, the element is focused but the focused state is stuck as false. Which issue occurs depends on whether mouse or keyboard events are used and timing of when the focus switch happens. Suggests the issue hinges on the timing of rerenders triggered by different event types.

In the provided example, it's almost possible to work around this issue with careful timing - programmatically blurring the element after setting disabled to false in the target but before setting it to true in the trigger element. But this creates some strange timing issues when mouse clicks are used. Also, in real-life examples, this may not be an option: for example if the elements' disabled state is controlled by one selected state, it may not be possible to set a moment when both are enabled.

Suggested fix

It looks like all focus / blur handling is suppressed as soon as disabled is applied, and the timing of this relative to event handler timing varies by event type. Suggestion:

  • When disabled flips from false to true, if focused is still true, it shouldn't be and the element is losing focus; so call onBlur (including setting internal state) while it still can.
  • When disabled flips from true to false, if the element has become actually focused in the DOM but focused is false, it is legitimately gaining focus but the event handlers were skipped due to event / re-render timing; so call onFocus (including setting internal state)

AlanSl avatar Aug 08 '22 10:08 AlanSl

Best workaround for my particular case so far seems to be to control an isFocused state in the Pressable's parent using onBlur / onFocus and not allow disabled to be true if isFocused is true; so, regardless of the timing around the triggering event, if an action causes the parent's disabled prop to be true on next render and moves focus away, it will not apply disabled as true until the re-render caused by the onBlur handler setting isFocused to false.

AlanSl avatar Aug 08 '22 11:08 AlanSl

Test case should be a codesandbox (as asked for in the template) using the latest published version

necolas avatar Aug 10 '22 17:08 necolas

https://codesandbox.io/s/quirky-williamson-v3r94g

AlanSl avatar Aug 11 '22 14:08 AlanSl

If there's a mandatory format and template, may I suggest linking to it in the issue template

Screenshot_20220811-081834_Chrome

necolas avatar Aug 11 '22 15:08 necolas

try this patch https://github.com/necolas/react-native-web/pull/2374

necolas avatar Aug 24 '22 21:08 necolas