Skip to content
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.

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

[Android] Prevent the tap that closes an open SwipeView from being propagated to children #24275

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

sjordanGSS
Copy link

Description of Change

An open SwipeView will close on tap, so a tap on an open SwipeView should be intercepted before it can be handled by child views. Otherwise tapping to close an open swipeview will result in Buttons, TapGestureRecognisers etc within SwipeView.Content being activated.

Issues Fixed

Fixes #23921

an open swipeview will close on tap, so intercept the touch if the swipeview is currently open
@sjordanGSS sjordanGSS requested a review from a team as a code owner August 16, 2024 09:28
@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Aug 16, 2024
Copy link
Contributor

Hey there @sjordanGSS! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@sjordanGSS
Copy link
Author

A more verbose explanation of the issue and the fix is here: #23921 (comment), I can add this to the PR description if desired.
This is my first PR here so I would appreciate any guidance :)

@jfversluis
Copy link
Member

Hey @sjordanGSS thanks for your very first contribution! Exciting!

A couple of things:

  • Please add a more meaningful title. This will end up in our commit history, having something that describes better what this does makes it more easy to navigate that later. Fixing issue ... is the result, not what is in this PR 😉
  • It would be great to add a test! We have a couple of options for that, would you be able to look into that please?

We appreciate you looking into this <3

@sjordanGSS sjordanGSS changed the title fix 23921 Prevent the tap that closes an open SwipeView from being propagated to children Aug 19, 2024
@sjordanGSS sjordanGSS changed the title Prevent the tap that closes an open SwipeView from being propagated to children [Android] Prevent the tap that closes an open SwipeView from being propagated to children Aug 19, 2024
@sjordanGSS
Copy link
Author

Quick update on this: I've renamed the PR as requested, hopefully it's is a little more useful now. I've been trying to write unit tests for the change but have been having trouble getting things to work: Visual Studio 17.11.1 crashes when I open any solution file, and VS Code fails to run the unit test app with an error that it can't find MainActivity. I'm working on this between other things at my day job and we've already fixed the issue on our side so I'm not able to dedicate a whole lot of time to working this out.

@sjordanGSS
Copy link
Author

I've pushed a commit with two unit tests which I'm not able to run locally (I got the projects to load in Visual Studio, but it throws the same exception as VS Code: Cannot start the 'com.microsoft.maui.uitests' application. Consider checking the driver's troubleshooting documentation. Original error: Activity name '.com.microsoft.maui.uitests.MainActivity' used to start the app doesn't exist or cannot be launched! Make sure it exists and is a launchable activity), would it be possible to have them reviewed and run to see if I got them right? Thanks

@sjordanGSS
Copy link
Author

sjordanGSS commented Aug 30, 2024

@dotnet-policy-service agree company="Global Stock Systems Ltd"

@jsuarezruiz
Copy link
Contributor

/rebase

@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

set automationId properties on test view
tests currently succeed if they are run independently
@sjordanGSS
Copy link
Author

@jsuarezruiz Thanks for running the tests. I've gotten the working on my pc now and I can see there are some problems with the tests I've written for this PR. I've pushed some changes, but it's currently in the situation where one test will fail if it is run immediately after the other. I'm still getting used to this testing framework unfortunately. Is there a better way to check if a button has been clicked than to set the text, and then check it in the test case? Thanks!

…, add two of them and point one test at each
@sjordanGSS
Copy link
Author

I've fixed the tests by duplicating the controls for each test. Can they be rerun?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-controls-swipeview SwipeView community ✨ Community Contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tapping to close a SwipeView will activate TapGestureRecognizers on .Content
4 participants