Skip to content

Support landmark navigation and toasts across iframes#3892

Merged
devongovett merged 31 commits into
mainfrom
landmark-iframe
Jan 12, 2023
Merged

Support landmark navigation and toasts across iframes#3892
devongovett merged 31 commits into
mainfrom
landmark-iframe

Conversation

@devongovett
Copy link
Copy Markdown
Member

Depends on #3868

This adds the necessary APIs to implement landmark navigation across iframe boundaries. In addition, this enables toasts to pop out of iframes and for the user to navigate between the toasts and iframe content.

It works by adding a custom DOM event named react-aria-landmark-navigation, which can be canceled by calling preventDefault to override the default behavior. This event is triggered during landmark navigation when we reach the end of the landmark sequence in either direction. By default, we wrap around to the first/last landmark again, but if preventDefault is called on this event we do not. This enables the iframe page to fire a message out of the iframe, where the F6 key is re-dispatched so that the landmark navigation on the outer page is applied. The iframe itself is made a landmark as well, and an additional focus option is supported so that when landmark navigation reaches the iframe itself, focus can be redirected back inside. This makes it behave as is there were a single landmark navigation sequence across the whole page.

This PR also fixes a couple bugs:

  1. Landmarks inside aria-hidden trees were not skipped.
  2. When adding an element while ariaHideOutside is active, it could hide data-react-aria-top-layer elements (toasts). Now we apply the tree walker to added elements to ensure this is not the case.

snowystinger
snowystinger previously approved these changes Jan 10, 2023

let ref = useRef(null);
let {landmarkProps} = useLandmark({
role: 'main',
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The story we load in already has a main, I think there can only be one main per application. Right now Option+CMD+F6 for go to main is a little strange because it'll move through 3 different elements.

It's not pressing, just if we want to make the example more like how we'd expect people to implement it. Not that we can really do anything about enforcing it with iframes. Same thing with id's inside the iframe.

Maybe nothing to do for code, just need to make sure we state these things in the documentation.

@LFDanLu
Copy link
Copy Markdown
Member

LFDanLu commented Jan 11, 2023

Retested toasts in Android Talkback and Windows FF NVDA. Confirmed that Talkback properly announces the Toasts as they appear but Windows FF NVDA still has intermittent behavior where it will suddenly stop announcing the Toasts. Doesn't seem like the ariaHideOutside is blocking it there.

Still reviewing the code changes

@devongovett
Copy link
Copy Markdown
Member Author

I can look into the NVDA thing separately.

LFDanLu
LFDanLu previously approved these changes Jan 11, 2023
Copy link
Copy Markdown
Member

@LFDanLu LFDanLu left a comment

Choose a reason for hiding this comment

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

LGTM for the most part, verified that aria-hide outside, landmarks, and toast seems to be working as expected. Just one thing I noticed with the iframe story, but that is story implementation specific and be looked at later with the shell work

Comment thread packages/@react-spectrum/toast/stories/Toast.stories.tsx
reidbarber
reidbarber previously approved these changes Jan 11, 2023
Comment thread packages/@react-aria/landmark/test/useLandmark.test.tsx Outdated
devongovett and others added 3 commits January 11, 2023 16:08
Co-authored-by: Reid Barber <reid@reidbarber.com>
Co-authored-by: Daniel Lu <dl1644@gmail.com>
Base automatically changed from toast to main January 12, 2023 18:41
@devongovett devongovett dismissed stale reviews from reidbarber, LFDanLu, and snowystinger via 6ce0e63 January 12, 2023 18:41
…iframe

# Conflicts:
#	packages/@react-aria/overlays/src/ariaHideOutside.ts
#	packages/@react-spectrum/toast/stories/Toast.stories.tsx
@devongovett devongovett merged commit 68d9c9e into main Jan 12, 2023
@devongovett devongovett deleted the landmark-iframe branch January 12, 2023 18:54
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.

4 participants