fix(mobile): tap outside to dismiss mention popup#5740
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
@codex review |
|
@claude review once |
|
Codex Review: Didn't find any major issues. What shall we delve into next? ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
wca4a
force-pushed
the
wa/mobile-mention-tap-dismiss
branch
from
152b058 to
74583ef
Compare
|
@claude review once |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74583ef620
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| (keyboardHeight > 0 ? keyboardHeight : insets.bottom) + | ||
| containerHeight + | ||
| 24; | ||
| const inputTopFromBottom = | ||
| (keyboardHeight > 0 ? keyboardHeight : insets.bottom) + containerHeight; |
There was a problem hiding this comment.
On Android, this popup is now rendered in a Portal and its vertical position is computed with keyboardHeight added to bottom, but the app activity is configured with android:windowSoftInputMode="adjustResize" (apps/tlon-mobile/android/app/src/main/AndroidManifest.xml), which already resizes the root view when the IME appears. In that context, adding keyboardHeight again pushes both the mention card and dismiss backdrop too high above the input, so the mention UI is detached/mispositioned whenever the keyboard is open (most Android chat usage).
Useful? React with 👍 / 👎.
wca4a
force-pushed
the
wa/mobile-mention-tap-dismiss
branch
from
74583ef to
67cac20
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
| function useKeyboardHeight() { | ||
| const [height, setHeight] = useState(0); | ||
| useEffect(() => { | ||
| const showSub = Keyboard.addListener('keyboardDidShow', (e) => { | ||
| setHeight(e.endCoordinates.height); | ||
| }); | ||
| const hideSub = Keyboard.addListener('keyboardDidHide', () => { | ||
| setHeight(0); | ||
| }); | ||
| return () => { | ||
| showSub.remove(); | ||
| hideSub.remove(); | ||
| }; | ||
| }, []); | ||
| return height; | ||
| } |
There was a problem hiding this comment.
🔴 The useKeyboardHeight hook in InputMentionPopup.tsx initializes to 0 and relies solely on keyboardDidShow/keyboardDidHide events, so if the keyboard is already visible when the component mounts (e.g., navigating to a new channel while the keyboard stays up), keyboardHeight remains 0 for the session. This causes the mention popup bottomOffset to fall back to insets.bottom (~34px) instead of the actual keyboard height (~350-400px), placing the popup ~300px below its intended position—behind the keyboard—until the user dismisses and re-shows it. Fix: initialize with Keyboard.metrics()?.height ?? 0 to read the current keyboard state synchronously on mount.
Extended reasoning...
What the bug is and how it manifests
The new useKeyboardHeight hook in InputMentionPopup.tsx (lines 12-27) initializes state with useState(0) and updates only via keyboardDidShow/keyboardDidHide event listeners. This is a gap: if the keyboard is already visible at the time InputMentionPopupInternal first mounts, no keyboardDidShow event fires and keyboardHeight stays 0 for the entire lifetime of that component instance.
The specific code path that triggers it
- User is in channel A with the keyboard open.
- User navigates to channel B (in-app navigation that preserves keyboard visibility on iOS/Android).
- BareChatInput and MessageInputContainer remount fresh. useKeyboardHeight starts at 0.
- The keyboard is already displayed, so no keyboardDidShow fires again.
- User types @ to trigger a mention. isMentionModeActive becomes true, options.length > 0 — the Portal path executes.
- bottomOffset = (keyboardHeight > 0 ? keyboardHeight : insets.bottom) + containerHeight + 24 = (0 > 0 ? 0 : ~34) + 48 + 24 = ~106px.
- The correct value would be ~350 + 48 + 24 = ~422px.
- The popup renders ~316px too low — behind the keyboard or overlapping the input area — and stays there until the user dismisses and re-shows the keyboard.
Why existing code does not prevent it
The existing useKeyboardHeight in BareChatInput/index.tsx has the same event-listener-only pattern but uses Keyboard.metrics() inside the handler and has a DEFAULT_KEYBOARD_HEIGHT=300 fallback. The new hook in InputMentionPopup.tsx has no such fallback. The effect dependency array is [] so it never re-runs on re-render to pick up the current state.
What the impact would be
Cross-channel navigation with a persistent keyboard is a common and natural flow on iOS (keyboard often stays up as users switch channels) and even more so on Android. When it occurs, the mention popup is hidden behind the keyboard for the entire channel session — the feature that this PR specifically introduces for mobile. The visual regression is severe and immediate: the popup appears in the wrong place with no workaround short of keyboard dismiss and re-show.
How to fix it
Initialize the hook with the synchronous Keyboard.metrics() call:
Keyboard.metrics() returns the current keyboard dimensions synchronously if the keyboard is visible, so the initial render uses the real height and no event is needed for the common mount-with-keyboard-already-up case.
Step-by-step proof
- User opens channel A, taps the input — keyboard rises to ~350px, keyboardDidShow fires, keyboardHeight = 350.
- User navigates to channel B while the keyboard remains up. BareChatInput remounts. useKeyboardHeight reinitializes to useState(0). Keyboard is still shown but keyboardDidShow does not re-fire. keyboardHeight = 0.
- User types @fo. isMentionModeActive = true, options = [contactA, contactB].
- bottomOffset = (0 > 0 ? 0 : insets.bottom) + containerHeight + 24 = 34 + 48 + 24 = 106px.
- Portal renders the popup 106px from the bottom of the screen — behind the keyboard which occupies ~350px from the bottom.
- Popup is completely invisible. User cannot see or interact with it. The only recovery is dismissing and re-showing the keyboard.
| const inputTopFromBottom = effectiveBottomInset + containerHeight; | ||
|
|
||
| return ( | ||
| <Portal> | ||
| {onDismiss ? ( | ||
| <Pressable | ||
| onPress={onDismiss} | ||
| style={{ | ||
| position: 'absolute', | ||
| top: 0, | ||
| left: 0, | ||
| right: 0, |
There was a problem hiding this comment.
🔴 The transparent Pressable backdrop at lines 68-79 covers the entire chat area above the keyboard inside a Portal, which places it as a z-order sibling of the chat ScrollView/FlatList rather than an ancestor - so the standard Pressable/ScrollView responder-handoff mechanism does not apply, and any swipe starting in the chat area is silently swallowed: the popup neither dismisses nor does the chat scroll. Use a transparent={true} React Native Modal as the backdrop, or a gesture handler that distinguishes taps from pans, so scroll gestures can pass through to the underlying chat view.
Extended reasoning...
What the bug is and how it manifests
The PR renders a full-screen transparent Pressable inside a Tamagui Portal (InputMentionPopup.tsx lines 68-79). The Portal places its content at the root of the React Native view hierarchy, above the normal chat view tree in z-order. The Pressable's style covers the entire chat area: top: 0, left: 0, right: 0, bottom: inputTopFromBottom. When the mention popup is open, any swipe gesture starting in the chat area lands on this Pressable, which intercepts it. The chat neither scrolls nor does the popup dismiss for swipe movements - the gesture is silently consumed with no effect.
The specific code path that triggers it
On a touch start anywhere in the covered area: (1) UIKit/Android hit-testing finds the Pressable view (it is topmost in z-order). (2) React Native's responder system calls the Pressable's onStartShouldSetResponder, which returns true. (3) The Pressable becomes the active responder and begins tracking the press. (4) As the user moves their finger (swipe gesture), onResponderMove is dispatched to the Pressable. (5) The movement exceeds Pressable's tap threshold, so onPress is not fired - but because the Pressable still holds the responder, the chat ScrollView/FlatList never receives the move events. The gesture is fully consumed by the Pressable with no visible effect.
Why existing code does not prevent it
One verifier refuted this bug, arguing that React Native's Pressable returns true from onResponderTerminationRequest, allowing a ScrollView to steal the responder on a scroll gesture. This is correct in the normal case where the Pressable is rendered inside the ScrollView's view hierarchy - the ScrollView can request the responder via onMoveShouldSetResponder as the event bubbles up through the common ancestor chain. However, the Portal breaks this assumption: the Pressable is a sibling of the ScrollView at the root native view level, not a descendant. On both iOS (UIKit hit-testing) and Android (ViewGroup touch dispatching), the Portal's overlay view intercepts the native touch event before it reaches the ScrollView's native view. Since the ScrollView never receives the native touch start, it has no opportunity to issue a requestTerminate call. The responder termination path simply never executes.
What the impact would be
This is a regression relative to the pre-PR state where there was no overlay at all. While the mention popup is open, the chat is effectively frozen: users cannot scroll through messages. The backdrop is transparent, so users will naturally attempt to scroll (seeing chat content behind the popup). Each attempted swipe is silently discarded - no scroll, no dismiss. The workaround is to tap precisely (not swipe) to dismiss, then scroll in a second gesture - a confusing two-step that violates standard mobile UX expectations.
How to fix it
Replace the Portal + Pressable pattern with a React Native Modal using transparent={true} and animationType=none. A Modal renders in a separate native layer (an independent Window on iOS / Dialog on Android) that is completely disjoint from the chat ScrollView's native view hierarchy. Because the two native layers are independent, a transparent backdrop can be given a tap-dismiss handler while scroll events on the underlying layer pass through via the normal responder system. Alternatively, react-native-gesture-handler's TapGestureHandler with a hitSlop guard that lets scroll velocities fall through is another valid approach.
Step-by-step proof
- User types @fo in a channel input on iOS. The Portal renders the Pressable with style {position:absolute, top:0, left:0, right:0, bottom:inputTopFromBottom}.
- User places finger on a chat message above the popup and begins a downward swipe.
- UIKit's hitTest:withEvent: checks the Portal's overlay view first (it is on top in z-order). The touch point falls within the Pressable's bounds. hitTest returns the Pressable's underlying UIView.
- React Native dispatches onStartShouldSetResponder to the Pressable which returns true. Pressable becomes the active responder.
- The user moves their finger (swipe). onResponderMove fires on the Pressable. The delta exceeds Pressable's press threshold, so onPress/onDismiss is NOT called. But the ScrollView is a sibling - it receives no move events through its own subtree and cannot call requestTerminate.
- The swipe ends. No scroll occurred. The popup is still visible. The user's gesture was silently discarded.
wca4a
force-pushed
the
wa/mobile-mention-tap-dismiss
branch
from
67cac20 to
c4304de
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4304deef9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| top: 0, | ||
| left: 0, | ||
| right: 0, | ||
| bottom: inputTopFromBottom, |
There was a problem hiding this comment.
The mobile tap-outside overlay stops at inputTopFromBottom, which is derived from containerHeight; in BareChatInput this prop is hard-coded to 48 while the multiline composer can grow taller (TextInput has multiline with variable height). When the user has a taller draft and mention candidates are open, the backdrop overlaps the upper part of the composer, so taps intended to move the cursor inside the input are intercepted and only dismiss mentions. This regression comes from the new backdrop math and is reproducible on mobile with multi-line input.
Useful? React with 👍 / 👎.
This comment has been minimized.
This comment has been minimized.
On mobile there's no Escape key, so an open @-mention popup has no obvious dismiss. Add a transparent backdrop above the input that dismisses on tap, and render it through a Portal so parent View bounds (Android clipChildren) don't block hits above the popup. Uses a soft dismiss path (only toggles isMentionModeActive) rather than reusing handleMentionEscape — otherwise the trigger index would stay "poisoned" and the popup wouldn't reopen as the user keeps typing. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
wca4a
force-pushed
the
wa/mobile-mention-tap-dismiss
branch
from
c4304de to
21bb862
Compare
|
Addressed the review feedback across several amends — current commit is Claude bot — soft vs. hard dismiss. Backdrop tap was calling Claude bot — Android clipping. Codex P1 — double-counting keyboard height on Android. Android uses Claude bot — keyboard-already-visible on mount. Event-listener-only hook stayed at 0 if the component mounted while the keyboard was already up (e.g., channel-to-channel navigation). Initialized with Claude bot — scroll gestures swallowed by backdrop. Decided to accept this as standard mobile UX. Every comparable app (Slack, Discord, iMessage autocompletes) treats the mention dropdown as modal-lite: dismiss or pick first, then scroll. The pre-PR state was "no way to dismiss at all," so trading free scroll for tap-dismiss is the right direction. Codex P2 — backdrop over tall multi-line composer. Tested on iOS simulator: tap-outside dismisses with soft-dismiss behavior (continued typing reopens), keyboard-open and keyboard-closed positioning both correct, tall multi-line composer accepts cursor taps. |
|
Found 18 test failures on Blacksmith runners: Failures
|
2 participants
Summary
On mobile there's no Escape key, so an open
@-mention popup can feel stuck until the user types enough to clear all candidates or picks one. Tapping elsewhere in the chat should dismiss the popup — this is standard mobile dropdown UX.Fix
Pass
handleMentionEscapethroughMessageInputContainer→InputMentionPopup. OnPlatform.OS !== 'web'(and only while candidates are visible), render a transparentPressablebackdrop positioned behind the popup card that extends above/around the popup but not over the message input, and dismisses on tap.Web is unchanged and still uses the Escape key.
Test plan
~or@in a DM or channel input → popup appears → tap on chat messages above → popup dismissesPlatform.OS !== 'web'guard)