Skip to content

Commit

Permalink
[rn][win] Adds HyperlinkViewManager for selectable-safe onPress
Browse files Browse the repository at this point in the history
Summary:
When `selectable={true}` on the Text component in react-native-windows, the TextBlock native component captures and handles all pointer events, thus none of the pointer events are sent to JS. We could just allow the TouchEventHandler to respond to all events, even handled ones, but I'm concerned that this may raise other issues with other components that are expected to handle events (e.g., getting onPress events before ScrollViewer gets a chance to capture the pointer). There are other limitations to that approach as well, including issues with the mouse cursor and how the control would interoperate with accessibility.

Adding Hyperlink works around this limitation as we can emit direct events that can be tied to a JS `onPress` callback via the `Click` event, which "plays nicely" with `selectable={true}`. Also, Hyperlink has expected behavior out of the box for focusability and pointer cursor.

The reason this needs to be implemented in our fork and not in Zeratul is because the `IViewManager` interface exposed from the ABI for react-native-windows uses `FrameworkElement` as a base class, so we could not create a ViewManager for `Hyperlink`, which derives from `DependencyObject` and not `FrameworkElement`.

Here are the GitHub issues tracking that this is working around:
microsoft#1679
microsoft#5539



Test Plan:
- Can we invoke onPress for FocusableText when selectable=true?

https://pxl.cl/1G90F

- Does the mouse cursor change from the selection IBeam to signal that the FocusableText can be clicked?

{F564338487}

- Can I still set selection on link text?

{F564343437}

- Can we tab focus to FocusableText?
Not currently, but I believe this is not a limitation of the Hyperlink approach, something else seems to be trapping focus in the chat window.

- Can we use the Enter key to invoke onPress for FocusableText?
Not currently because the hyperlink cannot get focus.

- Stretch: Is there any feedback when onPressIn for FocusableText?
No, and it's likely that if someone adds it using `onPressIn` or `onPressOut` it will be broken.

Reviewers: skyle, lyahdav, mylando

Reviewed By: lyahdav

Subscribers: necolas, eliwhite

Differential Revision: https://phabricator.intern.facebook.com/D27474588

Tasks: T87521248

Signature: 27474588:1617305556:de80de6759c483628f86d3453533eacf360ce234
  • Loading branch information
rozele committed Apr 1, 2021
1 parent a9452c8 commit d7a1827
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 0 deletions.
2 changes: 2 additions & 0 deletions vnext/Microsoft.ReactNative/Base/CoreUIManagers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <Views/DatePickerViewManager.h>
#include <Views/FlyoutViewManager.h>
#include <Views/FocusZoneViewManager.h>
#include <Views/HyperlinkTextViewManager.h>
#include <Views/Image/ImageViewManager.h>
#include <Views/PickerViewManager.h>
#include <Views/PopupViewManager.h>
Expand Down Expand Up @@ -46,6 +47,7 @@ void AddStandardViewManagers(
viewManagers.push_back(std::make_unique<DatePickerViewManager>(instance));
viewManagers.push_back(std::make_unique<FlyoutViewManager>(instance));
viewManagers.push_back(std::make_unique<FocusZoneViewManager>(instance));
viewManagers.push_back(std::make_unique<HyperlinkTextViewManager>(instance));
viewManagers.push_back(std::make_unique<ImageViewManager>(instance));
viewManagers.push_back(std::make_unique<PickerViewManager>(instance));
viewManagers.push_back(std::make_unique<PopupViewManager>(instance));
Expand Down
2 changes: 2 additions & 0 deletions vnext/Microsoft.ReactNative/Microsoft.ReactNative.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,7 @@
<ClInclude Include="Views\ExpressionAnimationStore.h" />
<ClInclude Include="Views\FlyoutViewManager.h" />
<ClInclude Include="Views\FrameworkElementViewManager.h" />
<ClInclude Include="Views\HyperlinkTextViewManager.h" />
<ClInclude Include="Views\Image\Effects.h" />
<ClInclude Include="Views\Image\ImageViewManager.h" />
<ClInclude Include="Views\Image\Microsoft.UI.Composition.Effects_Impl.h" />
Expand Down Expand Up @@ -502,6 +503,7 @@
<ClCompile Include="Views\ExpressionAnimationStore.cpp" />
<ClCompile Include="Views\FlyoutViewManager.cpp" />
<ClCompile Include="Views\FrameworkElementViewManager.cpp" />
<ClCompile Include="Views\HyperlinkTextViewManager.cpp" />
<ClCompile Include="Views\Image\ImageViewManager.cpp" />
<ClCompile Include="Views\Image\ReactImage.cpp" />
<ClCompile Include="Views\Image\ReactImageBrush.cpp" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,9 @@
<ClCompile Include="Views\Impl\ScrollViewViewChanger.cpp">
<Filter>Views\Impl</Filter>
</ClCompile>
<ClCompile Include="Views\HyperlinkTextViewManager.cpp">
<Filter>Views</Filter>
</ClCompile>
</ItemGroup>
<ItemGroup>
<ClInclude Include="ABICxxModule.h" />
Expand Down Expand Up @@ -720,6 +723,9 @@
<ClInclude Include="Views\Impl\ScrollViewViewChanger.h">
<Filter>Views\Impl</Filter>
</ClInclude>
<ClInclude Include="Views\HyperlinkTextViewManager.h">
<Filter>Views</Filter>
</ClInclude>
</ItemGroup>
<ItemGroup>
<Midl Include="IJSValueReader.idl" />
Expand Down
47 changes: 47 additions & 0 deletions vnext/Microsoft.ReactNative/Views/HyperlinkTextViewManager.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

#include "pch.h"

#include "HyperlinkTextViewManager.h"

#include <UI.Xaml.Documents.h>

namespace winrt {
using namespace xaml::Documents;
} // namespace winrt

namespace react::uwp {

HyperlinkTextViewManager::HyperlinkTextViewManager(const std::shared_ptr<IReactInstance> &reactInstance)
: Super(reactInstance) {}

const char *HyperlinkTextViewManager::GetName() const {
return "RCTHyperlinkText";
}

folly::dynamic HyperlinkTextViewManager::GetExportedCustomDirectEventTypeConstants() const {
auto directEvents = Super::GetExportedCustomDirectEventTypeConstants();
directEvents["topClick"] = folly::dynamic::object("registrationName", "onClick");
return directEvents;
}

XamlView HyperlinkTextViewManager::CreateViewCore(int64_t tag) {
winrt::Hyperlink hyperlink;

// Underline should be handled by base class using TextDecorations
hyperlink.UnderlineStyle(winrt::UnderlineStyle::None);

hyperlink.Click([=](auto &&, auto &&) {
const auto instance = GetReactInstance().lock();
if (instance == nullptr)
return;

folly::dynamic eventData = folly::dynamic::object("target", tag);
instance->DispatchEvent(tag, "topClick", std::move(eventData));
});

return hyperlink;
}

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

#pragma once

#include <Views/VirtualTextViewManager.h>

namespace react::uwp {

class HyperlinkTextViewManager : public VirtualTextViewManager {
using Super = VirtualTextViewManager;

public:
HyperlinkTextViewManager(const std::shared_ptr<IReactInstance> &reactInstance);

const char *GetName() const override;
folly::dynamic GetExportedCustomDirectEventTypeConstants() const override;

protected:
XamlView CreateViewCore(int64_t tag) override;
};

} // namespace react::uwp

0 comments on commit d7a1827

Please sign in to comment.