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

Layer: Fix FocusTrapZone detection of Layered children #6489

Merged
merged 2 commits into from
Sep 27, 2018
Merged

Layer: Fix FocusTrapZone detection of Layered children #6489

merged 2 commits into from
Sep 27, 2018

Conversation

JasonGore
Copy link
Member

@JasonGore JasonGore commented Sep 27, 2018

Pull request checklist

Description of changes

Layer (before Portals) did not render content until after the virtual parent was set (after the first render pass in componentDidUpdate.) When Portals was introduced in #6211, the Layer content is rendered immediately in the first render pass, before virtual parent can be properly set. This is the root cause for #6484.

This PR prevents Layer content from rendering (and thus triggering focus events) until the virtual parent can be properly set.

Microsoft Reviewers: Open in CodeFlow

@cschleiden
Copy link
Contributor

image

@JasonGore
Copy link
Member Author

JasonGore commented Sep 27, 2018

Yeah, David and I talked about the additional render pass. This is pretty similar to pre-Portals behavior, though. If you can think of any methods that can render the Layer content AND set virtual parent before the rendered content triggers the window focus handler in FTZ, I'm all ears.

My initial hope was that the rootElement wrapper would be able to set virtual parent before componentDidMount gets called (it does.) Unfortunately the rootElement wrapper does not get called before FTZ's focus handler. As a result, FTZ incorrectly grabs focus since it can't identify the focus target as a valid child.

@JasonGore JasonGore merged commit 7c8f612 into microsoft:master Sep 27, 2018
@msft-github-bot
Copy link
Contributor

🎉[email protected] has been released which incorporates this pull request.:tada:

Handy Links:

@cschleiden
Copy link
Contributor

@JasonGore No I couldn't, it just always feels bad to artificially delay the rendering :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dropdown: not opening when used in a panel which needs to scroll into view the dropdown
4 participants