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.

Sign up for GitHub

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

Jump to bottom

Makes focusPath prop optional #6

Open
celiolatorraca opened this issue Jan 22, 2018 · 0 comments
Open

Makes focusPath prop optional #6

celiolatorraca opened this issue Jan 22, 2018 · 0 comments
Labels
discussion enhancement New feature or request

Comments

@celiolatorraca
Copy link
Contributor

celiolatorraca commented Jan 22, 2018 •
edited
Loading

For now, the focusPath is required. What would be the best approach to change it?

What is the problem?

As a react-based component, we lay on react lifecycle...

  • After a component mounts, we register it to focus manager and if there is no focusPath provided, the manager generates a new one.
  • When this component is re-rendered, as a normal react component, we unregister this component from the focus manager while unmounting it, then register again after mounting the new one.

But, if we are generating a new focusPath for components that didn't provide one, after this cycle we would have a currentFocusPath (injected by withNavigation) pointing to an element that was already unmounted...

What do you think we can do to avoid this?
@celiolatorraca celiolatorraca added the enhancement New feature or request label Jan 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
discussion enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@celiolatorraca @raphamorim