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

Work around XAML Islands issues with TextCommandBarFlyout #8249

Merged
merged 6 commits into from
Aug 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "Work around XAML Islands issue with TextCommandBarFlyout",
"packageName": "react-native-windows",
"email": "[email protected]",
"dependentChangeType": "patch"
}
2 changes: 2 additions & 0 deletions vnext/Microsoft.ReactNative/Microsoft.ReactNative.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,7 @@
<ClInclude Include="Utils\UwpPreparedScriptStore.h" />
<ClInclude Include="Utils\UwpScriptStore.h" />
<ClInclude Include="Utils\ValueUtils.h" />
<ClInclude Include="Utils\XamlIslandUtils.h" />
<ClInclude Include="Views\ActivityIndicatorViewManager.h" />
<ClInclude Include="Views\ControlViewManager.h" />
<ClInclude Include="Views\DatePickerViewManager.h" />
Expand Down Expand Up @@ -613,6 +614,7 @@
<ClCompile Include="Utils\UwpPreparedScriptStore.cpp" />
<ClCompile Include="Utils\UwpScriptStore.cpp" />
<ClCompile Include="Utils\ValueUtils.cpp" />
<ClCompile Include="Utils\XamlIslandUtils.cpp" />
<ClCompile Include="Views\ActivityIndicatorViewManager.cpp" />
<ClCompile Include="Views\ConfigureBundlerDlg.cpp" />
<ClCompile Include="Views\ControlViewManager.cpp" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,9 @@
<ClCompile Include="Utils\ValueUtils.cpp">
<Filter>Utils</Filter>
</ClCompile>
<ClCompile Include="Utils\XamlIslandUtils.cpp">
<Filter>Utils</Filter>
</ClCompile>
<ClCompile Include="JsiApi.cpp" />
<ClCompile Include="RedBoxErrorInfo.cpp" />
<ClCompile Include="RedBoxErrorFrameInfo.cpp" />
Expand Down Expand Up @@ -637,6 +640,9 @@
<ClInclude Include="Utils\ValueUtils.h">
<Filter>Utils</Filter>
</ClInclude>
<ClInclude Include="Utils\XamlIslandUtils.h">
<Filter>Utils</Filter>
</ClInclude>
<ClInclude Include="XamlLoadState.h" />
<ClInclude Include="XamlView.h" />
<ClInclude Include="ReactHost\IReactInstance.h">
Expand Down
8 changes: 8 additions & 0 deletions vnext/Microsoft.ReactNative/Utils/Helpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ bool IsAPIContractV8Available() {
return IsAPIContractVxAvailable<8>();
}

bool IsAPIContractV12Available() {
return IsAPIContractVxAvailable<12>();
}

bool IsRS3OrHigher() {
return IsAPIContractV5Available();
}
Expand All @@ -94,6 +98,10 @@ bool Is19H1OrHigher() {
return IsAPIContractV8Available();
}

bool Is21H1OrHigher() {
return IsAPIContractV12Available();
}

bool IsXamlIsland() {
AppPolicyWindowingModel e;
if (FAILED(AppPolicyGetWindowingModel(GetCurrentThreadEffectiveToken(), &e)) ||
Expand Down
1 change: 1 addition & 0 deletions vnext/Microsoft.ReactNative/Utils/Helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ bool IsRS3OrHigher();
bool IsRS4OrHigher();
bool IsRS5OrHigher();
bool Is19H1OrHigher();
bool Is21H1OrHigher();

bool IsXamlIsland();
bool IsWinUI3Island();
Expand Down
58 changes: 58 additions & 0 deletions vnext/Microsoft.ReactNative/Utils/XamlIslandUtils.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

#include "Utils/XamlIslandUtils.h"

#include <UI.Xaml.Input.h>

namespace Microsoft::ReactNative {

struct CustomAppBarButton : xaml::Controls::AppBarButtonT<CustomAppBarButton> {
void OnPointerExited(xaml::Input::PointerRoutedEventArgs const &e) {
// This method crashes in the superclass, so we purposely don't call super. But the superclass
// implementation likely cancels a timer that will show the submenu shortly after pointer enter.
// Since we don't have access to that timer, instead we reset the Flyout property, which resets
// the timer. This also fixes a crash where you can get a zombie submenu showing if the app
// loses focus while this timer is scheduled to show the submenu.
if (auto flyout = this->Flyout()) {
this->Flyout(nullptr);
this->Flyout(flyout);
}

// The superclass implementation resets the button to the normal state, so we do this ourselves.
this->SetValue(xaml::Controls::Primitives::ButtonBase::IsPointerOverProperty(), winrt::box_value(false));
xaml::VisualStateManager::GoToState(*this, L"Normal", false);
}

void OnPointerPressed(xaml::Input::PointerRoutedEventArgs const &e) {
// Clicking AppBarButton by default will dismiss the menu, but since we only use this class for
// submenus we override it to be a no-op so it behaves like MenuFlyoutSubItem.
}
};

void FixProofingMenuCrashForXamlIsland(xaml::Controls::TextCommandBarFlyout const &flyout) {
flyout.Opening([](winrt::IInspectable const &sender, auto &&) {
const auto &flyout = sender.as<xaml::Controls::TextCommandBarFlyout>();
if (const auto &textBox = flyout.Target().try_as<xaml::Controls::TextBox>()) {
const auto &commands = flyout.SecondaryCommands();
for (uint32_t i = 0; i < commands.Size(); ++i) {
if (const auto &appBarButton = commands.GetAt(i).try_as<xaml::Controls::AppBarButton>()) {
if (appBarButton.Flyout() == textBox.ProofingMenuFlyout()) {
// Replace the AppBarButton for the proofing menu with one that doesn't crash
const auto customAppBarButton = winrt::make<CustomAppBarButton>();
customAppBarButton.Label(appBarButton.Label());
customAppBarButton.Icon(appBarButton.Icon());
customAppBarButton.Flyout(appBarButton.Flyout());
commands.RemoveAt(i);
commands.InsertAt(i, customAppBarButton);

// There is only one proofing menu option
break;
}
}
}
}
});
}

} // namespace Microsoft::ReactNative
44 changes: 44 additions & 0 deletions vnext/Microsoft.ReactNative/Utils/XamlIslandUtils.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

#pragma once

#include "Utils/Helpers.h"

#include <UI.Xaml.Controls.Primitives.h>
#include <UI.Xaml.Controls.h>

namespace Microsoft::ReactNative {

void FixProofingMenuCrashForXamlIsland(xaml::Controls::TextCommandBarFlyout const &flyout);

template <typename T>
inline void EnsureUniqueTextFlyoutForXamlIsland(T const &textView) {
// This works around a XAML Islands bug where the XamlRoot of the first
// window the flyout is shown on takes ownership of the flyout and attempts
// to show the flyout on other windows cause the first window to get focus.
// https://github.com/microsoft/microsoft-ui-xaml/issues/5341
if (IsXamlIsland()) {
xaml::Controls::TextCommandBarFlyout flyout;
flyout.Placement(xaml::Controls::Primitives::FlyoutPlacementMode::BottomEdgeAlignedLeft);

// This works around a XAML Islands bug where the Proofing sub-menu for
// TextBox causes a crash while animating to open / close before 21H1.
// https://github.com/microsoft/microsoft-ui-xaml/issues/3529
if constexpr (std::is_same_v<T, xaml::Controls::TextBox>) {
if (!Is21H1OrHigher()) {
FixProofingMenuCrashForXamlIsland(flyout);
}
}

textView.ContextFlyout(flyout);
textView.SelectionFlyout(flyout);
}
}

inline void ClearUniqueTextFlyoutForXamlIsland(xaml::Controls::TextBlock const &textBlock) {
textBlock.ClearValue(xaml::UIElement::ContextFlyoutProperty());
textBlock.ClearValue(xaml::Controls::TextBlock::SelectionFlyoutProperty());
}

} // namespace Microsoft::ReactNative
3 changes: 3 additions & 0 deletions vnext/Microsoft.ReactNative/Views/TextInputViewManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "TextInputViewManager.h"

#include "Unicode.h"
#include "Utils/XamlIslandUtils.h"

#include <UI.Xaml.Controls.h>
#include <UI.Xaml.Input.h>
Expand Down Expand Up @@ -206,11 +207,13 @@ void TextInputShadowNode::registerEvents() {
m_passwordBoxPasswordChangedRevoker = {};
m_passwordBoxPasswordChangingRevoker = {};
auto textBox = control.as<xaml::Controls::TextBox>();
EnsureUniqueTextFlyoutForXamlIsland(textBox);
m_textBoxTextChangingRevoker = textBox.TextChanging(
winrt::auto_revoke, [=](auto &&, auto &&) { dispatchTextInputChangeEvent(textBox.Text()); });
} else {
m_textBoxTextChangingRevoker = {};
auto passwordBox = control.as<xaml::Controls::PasswordBox>();
EnsureUniqueTextFlyoutForXamlIsland(passwordBox);
if (control.try_as<xaml::Controls::IPasswordBox4>()) {
m_passwordBoxPasswordChangingRevoker = passwordBox.PasswordChanging(
winrt::auto_revoke, [=](auto &&, auto &&) { dispatchTextInputChangeEvent(passwordBox.Password()); });
Expand Down
13 changes: 10 additions & 3 deletions vnext/Microsoft.ReactNative/Views/TextViewManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "pch.h"

#include "TextViewManager.h"
#include "Utils/XamlIslandUtils.h"

#include <Views/RawTextViewManager.h>
#include <Views/ShadowNodeBase.h>
Expand Down Expand Up @@ -211,10 +212,16 @@ bool TextViewManager::UpdateProperty(
textBlock.ClearValue(xaml::Controls::TextBlock::LineStackingStrategyProperty());
}
} else if (propertyName == "selectable") {
if (propertyValue.Type() == winrt::Microsoft::ReactNative::JSValueType::Boolean)
textBlock.IsTextSelectionEnabled(propertyValue.AsBoolean());
else if (propertyValue.IsNull())
if (propertyValue.Type() == winrt::Microsoft::ReactNative::JSValueType::Boolean) {
const auto selectable = propertyValue.AsBoolean();
textBlock.IsTextSelectionEnabled(selectable);
if (selectable) {
EnsureUniqueTextFlyoutForXamlIsland(textBlock);
}
} else if (propertyValue.IsNull()) {
textBlock.ClearValue(xaml::Controls::TextBlock::IsTextSelectionEnabledProperty());
ClearUniqueTextFlyoutForXamlIsland(textBlock);
}
} else if (propertyName == "allowFontScaling") {
if (propertyValue.Type() == winrt::Microsoft::ReactNative::JSValueType::Boolean) {
textBlock.IsTextScaleFactorEnabled(propertyValue.AsBoolean());
Expand Down