fix(#3047): Interactive elements hidden by the VisuallyHidden component don't receive Talkback virtual clicks on certain devices by majornista · Pull Request #3048 · adobe/react-spectrum

majornista · 2022-04-20T14:48:00Z

🐛 Bug Report
Elements like a Menu's DismissButton and the ListView's drag handle don't receive a click event when focused and triggered by Android Talkback. This seems to be device specific since it works on my Samsung Galaxy S9+ but not on other Android devices that have the same Talkback and Chrome version (e.g. Android 7.1.1 on HTC Nexus 9 using Talkback 12.1 with Chrome 100.0.4896.88). The root cause seems to be due to clip-path and/or the size of the element itself.

Base reproduction with a possible fix: https://codepen.io/majornista/pen/Exorwzq?editors=1111
bug filed: https://issuetracker.google.com/issues/229646660

🤔 Expected Behavior
Talkback should be able to trigger a click on a visually hidden element

😯 Current Behavior
Talkback isn't able to trigger a click on a visually hidden element on certain devices

💁 Possible Solution
Increasing the height/width of the visually hidden element seems to help, see codepen example.

Closes #3047

✅ Pull Request Checklist:

Included link to corresponding React Spectrum GitHub Issue: Interactive elements hidden by the VisuallyHidden component don't receive Talkback virtual clicks on certain devices #3047 .
Added/updated unit tests and storybook for this change (for new code or code which already has tests).
Filled out test instructions.
Updated documentation (if it already exists for this component).
Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

Open https://reactspectrum.blob.core.windows.net/reactspectrum/957a3ebc3e5bf9f2ece221a38e354b485b3f0585/storybook/index.html?path=/story/menutrigger--default-menu-static on an Android device with Talkback running. Note that the bug did not reproduce on a Samsung phone, but was able to be reproduced on several other Android devices.
Swipe to focus, and double-tap to activate the Menu Button to expand the menu.
Swipe left to move Talkback focus to the visually hidden Dismiss Button immediately preceding the menu within the popover.
Double tap to activate the Dismiss Button
The menu should close.

🧢 Your Project:

Adobe/Accessibility

…nt don't receive Talkback virtual clicks on certain devices 🐛 Bug Report Elements like a Menu's DismissButton and the ListView's drag handle don't receive a click event when focused and triggered by Android Talkback. This seems to be device specific since it works on my Samsung Galaxy S9+ but not on other Android devices that have the same Talkback and Chrome version (e.g. Android 7.1.1 on HTC Nexus 9 using Talkback 12.1 with Chrome 100.0.4896.88). The root cause seems to be due to clip-path and/or the size of the element itself. Base reproduction with a possible fix: https://codepen.io/majornista/pen/Exorwzq?editors=1111 bug filed: https://issuetracker.google.com/issues/229646660 🤔 Expected Behavior Talkback should be able to trigger a click on a visually hidden element 😯 Current Behavior Talkback isn't able to trigger a click on a visually hidden element on certain devices 💁 Possible Solution Increasing the height/width of the visually hidden element seems to help, see code pen 🧢 Your Company/Team Adobe/Accessibility

LFDanLu · 2022-04-20T18:51:30Z

Note for testing: will need to make sure that the hidden element can't be accidentally pressed now that it is 10px by 10px.. Hopefully it doesn't block presses meant for the menu/row if the user presses on the visually hidden element.

LFDanLu · 2022-05-06T22:26:11Z

packages/@react-aria/visually-hidden/src/VisuallyHidden.tsx

-  height: 1,
-  margin: '0 -1px -1px 0',
+  clip: 'rect(0 10px 10px 0)',
+  clipPath: 'inset(40%)',


Unfortunately due to this clipPath no longer being inset(50%) I am able to click in the top left corner of the datepicker modal and close it: https://reactspectrum.blob.core.windows.net/reactspectrum/868e245a81fbaea15da6756323a666be62a5460b/storybook/index.html?path=/story/date-and-time-datepicker--default

@majornista I think I remember you saying that this clipPath change was necessary to get your Android devices to properly click the visually dismiss button via TalkBack? If so we'll have to discuss if we think this is a big enough problem to ditch this approach. Maybe we could absolutely position the visually hidden dismiss button just outside of the Dialog, not sure...

~~We may need to ditch the approach or at least modify it. I've oftentimes thought the dismiss button should just be a big hit target placed over the visible viewport dimensions, underneath the overlay.~~

I think we can add style={{marginBlockStart: '-10px'}} to the DismissButton within Overlays.

…rlay

LFDanLu

Confirmed that I'm still able to use the DismissButton via TalkBack and VO. Open to other people's opinion on the marginBlockStart style applied to DismissButton

LFDanLu · 2022-05-10T16:48:53Z

packages/@react-aria/overlays/src/DismissButton.tsx


  return (
-    <VisuallyHidden>
+    <VisuallyHidden style={{marginBlockStart: '-10px'}}>


Open to opinions: Maybe we should open up the api so user provided styles to the DismissButton are passed to the VisuallyHidden component? Just a bit concerned that the -10px is making an assumption about the positioning of

@LFDanLu Yeah, the -10px is based on the height of the hit target for the VisuallyHidden element, so if that were to change...

my concern there is that no one would know what to base the value on
might it be better if we allow people to pass a ref to the target? we could watch for size changes on it and adjust accordingly

One could pass transform: translate(0, -100%) as needed.

See 9ee38ed

majornista · 2022-05-17T13:46:52Z

@LFDanLu Would it make sense to only change the VisuallyHidden css on Android?

…roid

LFDanLu · 2022-05-20T20:59:39Z

@LFDanLu Would it make sense to only change the VisuallyHidden css on Android?

@majornista Hm, I think that would be fair, we have precedence for platform specific fixes in other places.

LFDanLu

Confirmed it is working in TalkBack and VO for me. Now that DismissButton has the style prop, perhaps we should document this bug in the DismissButton docs but that can/should be followup since we need to wait for this change to be released before updating the docs.

The other docs changes should also be removed for the mean time for the above reason

packages/@react-aria/visually-hidden/src/VisuallyHidden.tsx

packages/@react-aria/autocomplete/docs/useSearchAutocomplete.mdx

Per #3048 (comment)

LFDanLu

LGTM

adobe-bot · 2022-06-04T00:06:02Z

Build successful! 🎉

devongovett · 2022-06-20T17:12:31Z

packages/@react-aria/menu/stories/useMenu.stories.tsx

    <FocusScope restoreFocus>
      <div {...overlayProps} ref={overlayRef}>
-        <DismissButton onDismiss={props.onClose} />
+        <DismissButton onDismiss={props.onClose} style={{transform: 'translate(0, -100%)'}} />


Should these styles be built into VisuallyHidden? Requiring this addition is a breaking change for anyone that previously copied this example into their own component.

It's not really a problem with VisuallyHidden. It's a problem with positioning the DismissButton so that it does not overlap behind a MenuItem in the overlay, and it depends on where the DismissButton is rendered in the DOM, before or after the Menu.

Related comment thread: #3048 (comment)

See similar changes in React-Spectrum components: https://github.com/adobe/react-spectrum/pull/3048/files#diff-fdb3e8acd93715a7b806be804886c9f064fc17a049f57b4cc90f690d731ad105L87-R89

adobe-bot · 2022-06-27T14:49:17Z

Build successful! 🎉

adobe-bot · 2022-07-11T19:40:50Z

Build successful! 🎉

adobe-bot · 2022-07-15T14:50:27Z

Build successful! 🎉

adobe-bot · 2022-07-22T16:27:50Z

Build successful! 🎉

dannify · 2022-08-03T23:54:46Z

The original problem has since been testing on a couple of Android devices, none of which have this problem. The combination of the low percentage of users (only our team has reported it so far), an existing bug filed with google, and the downsides of this suggested solution is enough to just close this issue as "won't fix". We can revisit if reported in the future.

Michael Jordan added 2 commits April 20, 2022 10:45

fix(#3047): fix failing tests

ddcba3e

majornista requested review from LFDanLu and devongovett April 20, 2022 15:02

This comment was marked as outdated.

Sign in to view

dannify added the ListView label Apr 20, 2022

majornista added the accessibility label Apr 20, 2022

Merge branch 'main' into Issue-3047

868e245

This comment was marked as outdated.

Sign in to view

LFDanLu reviewed May 6, 2022

View reviewed changes

fix(#3047): Adjust top margin of VisuallyHidden Dismiss button in ove…

a7dae7b

…rlay

This comment was marked as outdated.

Sign in to view

majornista requested a review from LFDanLu May 9, 2022 17:19

Merge branch 'main' into Issue-3047

94471f7

This comment was marked as outdated.

Sign in to view

LFDanLu reviewed May 10, 2022

View reviewed changes

fix(#3047): DismissButton: accept styles to adjust layout positioning

9ee38ed

This comment was marked as outdated.

Sign in to view

majornista requested review from LFDanLu and snowystinger May 16, 2022 19:59

fix(#3047): VisuallyHidden adjust dimensions and clipPath only on And…

f307837

…roid

This comment was marked as outdated.

Sign in to view

Merge branch 'main' into Issue-3047

996fb7e

This comment was marked as outdated.

Sign in to view

LFDanLu requested changes May 20, 2022

View reviewed changes

packages/@react-aria/visually-hidden/src/VisuallyHidden.tsx Outdated Show resolved Hide resolved

packages/@react-aria/autocomplete/docs/useSearchAutocomplete.mdx Outdated Show resolved Hide resolved

Michael Jordan added 2 commits May 24, 2022 15:24

fix(#3047): Remove docs changes

193226a

Per #3048 (comment)

fix(#3407): make lineHeight conditional for Talkback as well

5f8f5e4

Per #3048 (comment)

majornista requested a review from LFDanLu May 24, 2022 19:26

This comment was marked as outdated.

Sign in to view

Merge branch 'main' into Issue-3047

eb37239

This comment was marked as outdated.

Sign in to view

Merge branch 'main' into Issue-3047

b553d71

This comment was marked as outdated.

Sign in to view

Merge branch 'main' into Issue-3047

eb8c70c

This comment was marked as outdated.

Sign in to view

LFDanLu previously approved these changes Jun 3, 2022

View reviewed changes

Merge branch 'main' into Issue-3047

9b68cf3

devongovett reviewed Jun 20, 2022

View reviewed changes

Merge branch 'main' into Issue-3047

957a3eb

Merge branch 'main' into Issue-3047

0c53576

majornista dismissed LFDanLu’s stale review via 0c53576 July 11, 2022 19:32

Merge branch 'main' into Issue-3047

b32093a

Michael Jordan added 2 commits July 22, 2022 12:07

Merge branch 'main' into Issue-3047

c5841c8

fix(#34047): fix order of imports lint error

a43a579

dannify closed this Aug 3, 2022

LFDanLu deleted the Issue-3047 branch January 11, 2023 01:36

Conversation

majornista commented Apr 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Pull Request Checklist:

📝 Test Instructions:

🧢 Your Project:

Uh oh!

This comment was marked as outdated.

LFDanLu commented Apr 20, 2022

Uh oh!

This comment was marked as outdated.

Choose a reason for hiding this comment

Uh oh!

majornista May 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

LFDanLu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

majornista commented May 17, 2022

Uh oh!

This comment was marked as outdated.

LFDanLu commented May 20, 2022

Uh oh!

This comment was marked as outdated.

LFDanLu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

LFDanLu left a comment

Choose a reason for hiding this comment

Uh oh!

adobe-bot commented Jun 4, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adobe-bot commented Jun 27, 2022

Uh oh!

adobe-bot commented Jul 11, 2022

Uh oh!

adobe-bot commented Jul 15, 2022

Uh oh!

adobe-bot commented Jul 22, 2022

Uh oh!

dannify commented Aug 3, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

majornista commented Apr 20, 2022 •

edited

Loading

majornista May 9, 2022 •

edited

Loading