Skip to content
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

Focus route if active element is not changed by app #58

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pdhoopr
Copy link
Contributor

@pdhoopr pdhoopr commented Jun 8, 2018

⚠️ Work in progress

Fixes #39.

When transitioning between routes, the new route will no longer be focused if the app has already changed the active element elsewhere (for example, a modal).

@pdhoopr
Copy link
Contributor Author

pdhoopr commented Jun 8, 2018

@ryanflorence So I've added a check in FocusHandlerImpl.focus to see if the app has updated the active element; hopefully that looks okay. I'm having a bit of a struggle coming up with the best place to save off the last active element for the comparison, though. At one point I had added to the LocationProvider's context, which worked but then it was propagated around to places it wasn't relevant (basically anywhere besides the FocusHandlerImpl). Do you happen to have an idea about how you want to store and update that information?

@pdhoopr pdhoopr force-pushed the focus-if-app-doesnt branch from 9d1a247 to ee6d374 Compare June 8, 2018 19:51
@pdhoopr pdhoopr force-pushed the focus-if-app-doesnt branch from ee6d374 to e99039f Compare June 8, 2018 20:00
@martinmckenna
Copy link

has this been fixed? I'm running into this issue currently with reach router@latest

@ryanflorence ryanflorence added the bug Something isn't working with a test case on codesandbox or an actual test. label Sep 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working with a test case on codesandbox or an actual test.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Advice for managing focus with modal
3 participants