Skip to content

Fix: Text component pressRetentionOffset issue on Windows #14596

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

Merged
merged 9 commits into from
Apr 25, 2025
Merged

Fix: Text component pressRetentionOffset issue on Windows #14596

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
df331c2
Fix: Text component pressRetentionOffset issue on Windows
Apr 21, 2025
2703a66
Change files
Apr 21, 2025
737bf05
Merge branch 'main' into text-pressoffsetfix
iamAbhi-916 Apr 21, 2025
41d0b46
lint formatting fixed
Apr 21, 2025
b792837
Merge branch 'text-pressoffsetfix' of https://github.com/iamAbhi-916/…
Apr 21, 2025
3c72cea
Merge branch 'main' into text-pressoffsetfix
iamAbhi-916 Apr 21, 2025
aec3c0a
Merge branch 'main' into text-pressoffsetfix
iamAbhi-916 Apr 22, 2025
4f646c4
Merge branch 'main' into text-pressoffsetfix
iamAbhi-916 Apr 23, 2025
f1494e9
Merge branch 'main' into text-pressoffsetfix
iamAbhi-916 Apr 25, 2025
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
7 changes: 7 additions & 0 deletions change/react-native-windows-39944bb5-d21b-45da-b750-d01f32e77200.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "Fix: Text component pressRetentionOffset issue on Windows",
"packageName": "react-native-windows",
"email": "[email protected]",
"dependentChangeType": "none"
}
25 changes: 16 additions & 9 deletions vnext/Microsoft.ReactNative/Fabric/Composition/CompositionEventHandler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1059,21 +1059,33 @@ void CompositionEventHandler::onPointerMoved(

facebook::react::PointerEvent pointerEvent = CreatePointerEventFromIncompleteHoverData(ptScaled, ptLocal);

auto handler = [&targetView,
&pointerEvent](std::vector<winrt::Microsoft::ReactNative::ComponentView> &eventPathViews) {
// check if this pointer corresponds to active touch that has a responder
auto activeTouch = m_activeTouches.find(pointerId);
bool isActiveTouch = activeTouch != m_activeTouches.end() && activeTouch->second.eventEmitter != nullptr;

auto handler = [&targetView, &pointerEvent, isActiveTouch](
std::vector<winrt::Microsoft::ReactNative::ComponentView> &eventPathViews) {
const auto eventEmitter = targetView
? winrt::get_self<winrt::Microsoft::ReactNative::implementation::ComponentView>(targetView)
->eventEmitterAtPoint(pointerEvent.offsetPoint)
: nullptr;
bool hasMoveEventListeners =
bool hasMoveEventListeners = isActiveTouch ||
IsAnyViewInPathListeningToEvent(eventPathViews, facebook::react::ViewEvents::Offset::PointerMove) ||
IsAnyViewInPathListeningToEvent(eventPathViews, facebook::react::ViewEvents::Offset::PointerMoveCapture);

if (eventEmitter != nullptr && hasMoveEventListeners) {
// Add logging before dispatching the event
eventEmitter->onPointerMove(pointerEvent);
}
};

HandleIncomingPointerEvent(pointerEvent, targetView, pointerPoint, keyModifiers, handler);

if (isActiveTouch) {
// For active touches with responders, also dispatch through touch event system
UpdateActiveTouch(activeTouch->second, ptScaled, ptLocal);
DispatchTouchEvent(TouchEventType::Move, pointerId, pointerPoint, keyModifiers);
}
}
}

Expand Down Expand Up @@ -1387,12 +1399,7 @@ void CompositionEventHandler::DispatchTouchEvent(
activeTouch.eventEmitter->onPointerDown(pointerEvent);
break;
case TouchEventType::Move: {
bool hasMoveEventListeners =
IsAnyViewInPathListeningToEvent(eventPathViews, facebook::react::ViewEvents::Offset::PointerMove) ||
IsAnyViewInPathListeningToEvent(eventPathViews, facebook::react::ViewEvents::Offset::PointerMoveCapture);
if (hasMoveEventListeners) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to remove the hasMoveEventListeners check here? Isn't there a perf cost to this (now all move events will always have to be fired and propagated to JS, even if no-one is listening?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

only applies to active touches that have already claimed responder status - not to all pointer moves so sending these extra events for ongoing interactions, which is actually a small subset of all pointer movements

activeTouch.eventEmitter->onPointerMove(pointerEvent);
}
activeTouch.eventEmitter->onPointerMove(pointerEvent);
break;
}
case TouchEventType::End:
Expand Down
8 changes: 8 additions & 0 deletions vnext/src-win/Libraries/Text/Text.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,14 @@ export interface TextProps
* Controls how touch events are handled. Similar to `View`'s `pointerEvents`.
*/
pointerEvents?: ViewStyle['pointerEvents'] | undefined;

/**
* Insets for press retention.
* Example: { top: 20, left: 20, bottom: 20, right: 20 }
*/
pressRetentionOffset?:
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks like a bug in core. -- This property should be added to Text.d.ts upstream.
Fine to add it here too.. but we should open a PR in RN to add this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah this a bug , they are handling it textprops.js file but didnt add as a prop ins text.d.ts file

| {top: number; left: number; bottom: number; right: number}
| undefined;
}

/**
Expand Down