Skip to content

Working implementation of allowsHitTest() on Android#359

Merged
marcprux merged 7 commits intoskiptools:mainfrom
tifroz:codex/allowsHitTesting-bugfix-full
Mar 20, 2026
Merged

Working implementation of allowsHitTest() on Android#359
marcprux merged 7 commits intoskiptools:mainfrom
tifroz:codex/allowsHitTesting-bugfix-full

Conversation

@tifroz
Copy link
Contributor

@tifroz tifroz commented Mar 17, 2026

This PR competes with #354. #354 is a very limited/local bug fix. This PR is closer to a full implementation of allowsHitTest() on Android

Current implementation (Android behavior):

  • button1 over button2 => button1 handle the tap
  • button1.allowsHitTest(false) over button2 => button1 disabled + swallows the tap (no-one handles it). THIS IS WRONG
  • text1 over button2 => button2 handle the tap
  • text1.allowsHitTest(false) over button2 => text1 swallows the tap (no-one handles). THIS IS WRONG

#354

  • button1 over button2 => button1 handle the tap
  • button1.allowsHitTest(false) over button2 => button1 handle the tap. THIS IS WRONG
  • text1 over button2 => button2 handle the tap (unless text1 has a tap handler)
  • text1.allowsHitTest(false) over button2 => button2 handles the tap

This PR (as tested)

  • button1 over button2 => button1 handle the tap
  • button1.allowsHitTest(false) over button2 => button2 handle the tap.
  • text1 over button2 => button2 handle the tap (unless text1 has a tap handler)
  • text1.allowsHitTest(false) over button2 => button2 handles the tap (regardless of tap handler on text1)

Skip Pull Request Checklist:

  • REQUIRED: I have signed the Contributor Agreement
  • REQUIRED: I have tested my change locally with swift test
  • OPTIONAL: I have tested my change on an iOS simulator or device
  • OPTIONAL: I have tested my change on an Android emulator or device
  • REQUIRED: I have checked whether this change requires a corresponding update in the Skip Fuse UI repository (link related PR if applicable)
  • OPTIONAL: I have added an example of any UI changes in the Showcase sample app

  • AI was used to generate or assist with generating this PR. Please specify below how you used AI to help you, and what steps you have taken to manually verify the changes.

Codex generated the code under supervision. Tested manually from a native sandbox app

@cla-bot cla-bot bot added the cla-signed label Mar 17, 2026
Copy link
Member

@marcprux marcprux left a comment

Choose a reason for hiding this comment

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

Thanks for continuing to work on this.

One note is that I needed to implement a similar feature to get the new contextMenu() modifier to work on a Button element: #360. I wonder if using that technique would work here as well, and obviate the need for the custom HitTesting.kt node…

@Composable static func RenderTextButton(label: View, context: ComposeContext, role: ButtonRole? = nil, isPlain: Bool = false, isEnabled: Bool = EnvironmentValues.shared.isEnabled, action: (() -> Void)? = nil) {
let isHitTestingEnabled = EnvironmentValues.shared._isHitTestingEnabled
if !isHitTestingEnabled {
print("SkipUI-HitTesting RenderTextButton hit testing disabled")
Copy link
Member

Choose a reason for hiding this comment

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

Need to remove print statement (you could change this to a Log.debug() call if you think it is useful).

var context = context
context.modifier = context.modifier.skipHitTesting(enabled: enabled)
if !enabled {
print("SkipUI-HitTesting allowsHitTesting(false) applied to \(type(of: renderable))")
Copy link
Member

Choose a reason for hiding this comment

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

Another print statement that needs to be removed.

Copy link
Contributor Author

@tifroz tifroz Mar 18, 2026

Choose a reason for hiding this comment

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

I am new to the Android universe, so please take with a grain of salt:

As I understand it, pointerInput could help when the competing handlers are in the same event chain (typically parent-child), because you can choose pass/consumption behavior. It would not help however when the overlapping views are siblings, a fairly common scenario (e.g. in a ZStack)

Will clean up the logs + gate the new RenderContextMenu with _isHitTestingEnabled (so the contextual menu doesn't show when hit testing is disabled)

@tifroz tifroz requested a review from marcprux March 19, 2026 20:20
@marcprux
Copy link
Member

LGTM. Thanks for all the work on it!

@marcprux marcprux merged commit 64cadb8 into skiptools:main Mar 20, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants