Skip to content

Fix .onLongPressGesture not firing on Text with markdown link - #403

Merged
marcprux merged 1 commit into
skiptools:mainfrom
digitalby:fix/371-onlongpress-with-link
May 14, 2026
Merged

Fix .onLongPressGesture not firing on Text with markdown link#403
marcprux merged 1 commit into
skiptools:mainfrom
digitalby:fix/371-onlongpress-with-link

Conversation

@digitalby

Copy link
Copy Markdown
Contributor

Summary

Fixes #371.onLongPressGesture (and any other .gesture(...) / .onTapGesture modifier) does not fire on Android when applied to a Text whose body contains a markdown [Link](url). The same code works on iOS and on plain Text.

Root cause

_Text.Render() installed an inner Modifier.pointerInput(true) { detectTapGestures { pos in ... } } to handle link taps whenever the rendered AttributedString had URL annotations (Sources/SkipUI/SkipUI/Text/Text.swift). The same node already had an outer pointerInput(true) { detectTapGestures(onLongPress:, onTap:, ...) } installed by GestureModifier (Sources/SkipUI/SkipUI/System/Gesture.swift) for any attached .onLongPressGesture / .onTapGesture / .gesture(...).

Compose's detectTapGestures consumes the down event for its own state machine, so the outer detector's awaitFirstDown(requireUnconsumed = true) never returned and its long-press timer never started.

Plain Text is unaffected because if !links.isEmpty() is false and the inner block is never installed. iOS is unaffected because the inner block is #if SKIP-gated and SwiftUI's recogniser-based gesture composition isn't subject to Compose's "first consumer wins" pointer semantics.

Fix

Replaces the inner detectTapGestures with a non-consuming awaitEachGesture loop on PointerEventPass.Initial, mirroring the pattern already used in Sources/SkipUI/SkipUI/Commands/ContextMenu.swift. The inner detector observes pointer events, identifies a clean tap within viewConfiguration.touchSlop and viewConfiguration.longPressTimeoutMillis, opens the URL on hit, and never calls consume() — so the outer GestureModifier's detectTapGestures runs unobstructed.

Test plan

  • New testLongPressOnTextWithMarkdownLink in Tests/SkipUITests/SkipUITests.swift renders a Text("Long press [me](url)") with .onLongPressGesture and a status indicator, then performs composeTestRule.onNodeWithTag("text").performTouchInput { longClick() }. Verified to pass on Robolectric (transpiled Kotlin/Compose path) — without the fix, the status stays "no"; with the fix, it flips to "yes".
  • The full swift test run is green: 55 passed, 2 skipped (pre-existing), 0 failed.
  • Manual reproduction with the verbatim 17-line snippet from the issue: long-press now fires the dialog on both Texts on Android. Repro project: https://github.com/digitalby/skip-ui-issue-371-research.

Out of scope

  • Refactoring the link-tap path to share a single detectTapGestures instance with GestureModifier (would require new internal gesture-registration plumbing). The non-consuming detector is a localised fix that resolves the user-visible bug without that surface-area change. Happy to follow up with a larger refactor if maintainers want it.
  • Sweep of other pointerInput { detectTapGestures } call sites for the same class of collision. None obvious from the codebase, but a deliberate audit would be a separate PR.

@cla-bot

cla-bot Bot commented Apr 25, 2026

Copy link
Copy Markdown

Thank you for your pull request and welcome to the Skip community. We require contributors to sign our contributor license agreement (CLA), and we don't seem to have the user(s) @digitalby on file. In order for us to review and merge your code, for each noted user please add your GitHub username to Skip's .clabot file

@digitalby

Copy link
Copy Markdown
Contributor Author

recheck

@marcprux

Copy link
Copy Markdown
Member

Looks great, thanks!

@marcprux
marcprux merged commit 82a5c42 into skiptools:main May 14, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

.onLongPressGesture is not working for Text with link (only on Android)

2 participants