Skip to content

[Toast] Manually reverse tab order #1469

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 16, 2022
Merged

[Toast] Manually reverse tab order #1469

merged 5 commits into from
Jun 16, 2022

Conversation

andy-hook
Copy link
Contributor

@andy-hook andy-hook commented Jun 13, 2022

fixes #1322
builds on #1468

Most recent should be tabbed to first and unfortunately, most recent will be at the bottom of the viewport DOM.

As described in #1322 – We use portals to move Toasts into the Viewport, out of the box this causes issues with keyboard accessibility as the append only nature of portals means our source order places most recent toasts last. We were previously using mutation observer to re-order the DOM and solve for it, unfortunately this causes issues with animation libraries which depend on a stable DOM order.

After much investigation there appears to be no fool proof way to re-order DOM without causing issues downstream. Instead this change manages focus programatically so that the tab order reflects newest -> oldest toast without touching the DOM structure.

@andy-hook andy-hook force-pushed the 1322-animation-compat branch 8 times, most recently from 1aef6fb to 4dd564b Compare June 14, 2022 20:35
@andy-hook andy-hook marked this pull request as ready for review June 14, 2022 21:02
@andy-hook andy-hook requested a review from benoitgrelard as a code owner June 14, 2022 21:02
@andy-hook andy-hook force-pushed the 1322-animation-compat branch 2 times, most recently from 3384853 to 907477d Compare June 14, 2022 21:17
@benoitgrelard
Copy link
Contributor

Looks like there's a regression in the toast order between foreground/background caught by Chromatic.

@andy-hook
Copy link
Contributor Author

Looks like there's a regression in the toast order between foreground/background caught by Chromatic.

That’s expected right? we are choosing not to reorder which is why this is a breaking change.

@benoitgrelard
Copy link
Contributor

haha of course! 🤦‍♂️ I thought somehow foreground/background had a particular ordering thing, but they don't really right? It's more about how they get announced/prioritised?

@andy-hook
Copy link
Contributor Author

I thought somehow foreground/background had a particular ordering thing, but they don't really right

Not in the DOM sense no, they dictate the nature of the announcement. I assume that chromatic story is intended to validate DOM order though so perhaps it could be updated to make that clearer.

@benoitgrelard
Copy link
Contributor

Yeah I think that's what confused me.

Base automatically changed from 1211-animation-compat to main June 16, 2022 14:28
@andy-hook andy-hook force-pushed the 1322-animation-compat branch from f617db2 to 0540626 Compare June 16, 2022 14:34
@andy-hook andy-hook merged commit e41331d into main Jun 16, 2022
@andy-hook andy-hook deleted the 1322-animation-compat branch June 16, 2022 14:53
luisorbaiceta pushed a commit to luisorbaiceta/primitives that referenced this pull request Jul 21, 2022
* Manually reverse tab order

* Feedback

* Versions

* Fix rebase clobber

* Typo
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.

[Toast] Re-ordering DOM is making JS animation difficult
2 participants