fix(tooltip): forward press event to wrapped child onPress#5010
Open
patrickwehbe wants to merge 1 commit into
Open
fix(tooltip): forward press event to wrapped child onPress#5010patrickwehbe wants to merge 1 commit into
patrickwehbe wants to merge 1 commit into
Conversation
The Tooltip injects its own handlePress as the wrapped child's onPress
via cloneElement. handlePress took no argument and called
props.onPress?.() with nothing, so a child such as Button or IconButton
with onPress={(e) => ...} received undefined for the press event on
native.
handlePress now accepts the GestureResponderEvent from Pressable and
forwards it to the child's onPress. The onPress type in TooltipChildProps
is widened to (e: GestureResponderEvent) => void to match the Button and
IconButton signatures.
Closes callstack#5003
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
On native, the press event object is not propagated to the component wrapped by
Tooltip. A child such asButtonorIconButtonwithonPress={(e) => ...}receivesundefinedinstead of theGestureResponderEvent.The cause is in
Tooltip. The component injects its ownhandlePressas the child'sonPressthroughcloneElement.handlePresstook no argument and calledprops.onPress?.()with nothing, so the event coming fromPressablewas dropped before it reached the child's handler.This change makes
handlePressaccept theGestureResponderEventfromPressableand forward it to the child'sonPress. TheonPresstype inTooltipChildProps(inTooltip/utils.ts) was too narrow (() => void) and is widened to(e: GestureResponderEvent) => voidto match theonPresssignatures ofButtonandIconButton.Note: PR #4994 also rewrites
Tooltip/Tooltip.tsx(for the Material Design 3 update) and overlaps with this file. That PR targets a different issue (#4980) and does not address the event-forwarding problem, so there may be a small conflict to resolve depending on merge order.Related issue
Closes #5003
A reproduction is provided in the issue (Expo Snack).
Test plan
yarn jest TooltipA test was added to the existing mobile suite in
src/components/__tests__/Tooltip.test.tsx. It renders a tooltip-wrapped component with anonPressspy, fires apresswith anativeEvent, and asserts the spy is called with that event. The test fails on the previous code (the spy is called with no arguments) and passes with this change.Locally verified:
yarn typescriptandyarn jest Tooltipboth pass.