Skip to content

Fix crash in Islands when opening Flyout without a target - #8300

Merged
Nick Gerleman (NickGerleman) merged 3 commits into
microsoft:mainfrom
lyahdav:fix-islands-flyout-no-target-crash
Oct 8, 2021
Merged

Fix crash in Islands when opening Flyout without a target#8300
Nick Gerleman (NickGerleman) merged 3 commits into
microsoft:mainfrom
lyahdav:fix-islands-flyout-no-target-crash

Conversation

@lyahdav

@lyahdav Liron Yahdav (lyahdav) commented Jul 21, 2021

Copy link
Copy Markdown
Contributor

Fixes #6714.

Before:

before.mov

After:

after.mov
Microsoft Reviewers: Open in CodeFlow

Comment thread vnext/Microsoft.ReactNative/Views/FlyoutViewManager.cpp Outdated
@asklar

Copy link
Copy Markdown
Member

Liron Yahdav (@lyahdav) we have a thread with you guys about whether we'd use the root tag from the native ui manager, but I'm not sure if this particular PR should wait until we have a resolution to that discussion. Let me know if this is ready to merge from your perspective.

@lyahdav

Copy link
Copy Markdown
Contributor Author

It can wait until we have resolution there since it could affect this change too. Should I close it for now then?

@rozele

Copy link
Copy Markdown
Contributor

FYI - this change is superseded by #8327

// // XamlRoot added in 19H1
if (Is19H1OrHigher()) {
if (auto xamlRoot = React::XamlUIService::GetXamlRoot(GetViewManager()->GetReactContext().Properties())) {
m_targetElement = xamlRoot.Content().try_as<xaml::FrameworkElement>();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we closed this PR too eagerly. I solved a slightly different issue. We still need to set the target element from the XamlRoot for XAML Islands. Liron Yahdav (@lyahdav) - could you re-open this one?

Comment thread vnext/Microsoft.ReactNative/Views/FlyoutViewManager.cpp Outdated
Co-authored-by: Chris Glein <26607885+chrisglein@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flyout without target in Xaml Islands crashes app

5 participants