-
Notifications
You must be signed in to change notification settings - Fork 2k
[Android] Fix single taps #16561
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] Fix single taps #16561
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| using Microsoft.Maui.Controls; | ||
|
|
||
| namespace Maui.Controls.Sample | ||
| { | ||
| public class DoubleTapGallery : Microsoft.Maui.Controls.ContentView | ||
| { | ||
| public DoubleTapGallery() | ||
| { | ||
| AutomationId = "DoubleTapGallery"; | ||
|
|
||
| var layout = new VerticalStackLayout() { Margin = 10, Spacing = 10 }; | ||
|
|
||
| var result = new Label() { AutomationId = "DoubleTapResults" }; | ||
|
|
||
| var tapSurface = new Grid() | ||
| { | ||
| HeightRequest = 200, | ||
| WidthRequest = 200, | ||
| BackgroundColor = Microsoft.Maui.Graphics.Colors.AliceBlue, | ||
| AutomationId = "DoubleTapSurface" | ||
| }; | ||
|
|
||
| var doubleTapRecognizer = new TapGestureRecognizer() { NumberOfTapsRequired = 2 }; | ||
| doubleTapRecognizer.Tapped += (sender, args) => { result.Text = "Success"; }; | ||
|
|
||
| tapSurface.GestureRecognizers.Add(doubleTapRecognizer); | ||
|
|
||
| layout.Add(tapSurface); | ||
| layout.Add(result); | ||
|
|
||
| Content = layout; | ||
| } | ||
| } | ||
| } | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| <?xml version="1.0" encoding="utf-8" ?> | ||
| <ContentPage xmlns="http://schemas.microsoft.com/dotnet/2021/maui" | ||
| xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml" | ||
| x:Class="Maui.Controls.Sample.Issues.Issue16561"> | ||
| <StackLayout> | ||
|
|
||
| <Grid x:Name="TapArea" | ||
| AutomationId="TapArea" | ||
| BackgroundColor="LightBlue" | ||
| Margin="10" | ||
| HeightRequest="300"> | ||
| <Label HorizontalOptions="Center" | ||
| VerticalOptions="Center" | ||
| TextColor="Black" | ||
| Text="Tap In This Grid" /> | ||
| </Grid> | ||
|
|
||
| <Label HeightRequest="100" | ||
| AutomationId="Tap1Label" | ||
| Background="green" | ||
| x:Name="Tap1Label" /> | ||
|
|
||
| <Label HeightRequest="100" | ||
| AutomationId="Tap2Label" | ||
| Background="green" | ||
| x:Name="Tap2Label" /> | ||
|
|
||
| </StackLayout> | ||
| </ContentPage> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| using Microsoft.Maui.Controls; | ||
| using Microsoft.Maui.Controls.Xaml; | ||
| using Microsoft.Maui.Platform; | ||
|
|
||
| namespace Maui.Controls.Sample.Issues | ||
| { | ||
| [XamlCompilation(XamlCompilationOptions.Compile)] | ||
| [Issue(IssueTracker.Github, 16561, "Quick single taps on Android have wrong second tap location", PlatformAffected.Android)] | ||
| public partial class Issue16561 : ContentPage | ||
| { | ||
| int _taps; | ||
|
|
||
| public Issue16561() | ||
| { | ||
| InitializeComponent(); | ||
|
|
||
| var tapGesture = new TapGestureRecognizer(); | ||
|
hartez marked this conversation as resolved.
|
||
| tapGesture.Tapped += TapHandler; | ||
|
|
||
| TapArea.GestureRecognizers.Add(tapGesture); | ||
| } | ||
|
|
||
| void TapHandler(object sender, TappedEventArgs e) | ||
| { | ||
| var pos = e.GetPosition(TapArea); | ||
|
|
||
| if (pos == null) | ||
| { | ||
| Tap1Label.Text = $"Error, could not get tap position"; | ||
| return; | ||
| } | ||
|
|
||
| #if ANDROID | ||
| // Adjust the results for display density, so they make sense to the | ||
| // Appium test consuming this. | ||
| var x = this.Handler.MauiContext.Context.ToPixels(pos.Value.X); | ||
| var y = this.Handler.MauiContext.Context.ToPixels(pos.Value.Y); | ||
| pos = new(x, y); | ||
| #endif | ||
|
Comment on lines
+33
to
+39
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is scary. Is Android not scaling the coordinates like the other platforms? Do we have an issue to track this or is it by design?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Appium layer doesn't know anything about our SDK's point->pixel conversions (why would it?). So when we're asking it to retrieve the At the moment, we don't have any other UI tests which require this sort of point-to-point comparison. If we start to need more tests like that in the future, we could introduce an automatic conversion mechanism for the Appium |
||
|
|
||
| if (_taps % 2 == 0) | ||
| { | ||
| Tap1Label.Text = $"{pos.Value.X}, {pos.Value.Y}"; | ||
| } | ||
| else | ||
| { | ||
| Tap2Label.Text = $"{pos.Value.X}, {pos.Value.Y}"; | ||
| } | ||
|
|
||
| _taps += 1; | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| using System.Drawing; | ||
| using Microsoft.Maui.Appium; | ||
| using NUnit.Framework; | ||
| using OpenQA.Selenium.Appium.MultiTouch; | ||
| using TestUtils.Appium.UITests; | ||
|
|
||
| namespace Microsoft.Maui.AppiumTests.Issues | ||
| { | ||
| public class Issue16561 : _IssuesUITest | ||
| { | ||
| private string _tapAreaId = "TapArea"; | ||
|
|
||
| public Issue16561(TestDevice device) : base(device) | ||
| { | ||
| } | ||
|
|
||
| public override string Issue => "Quick single taps on Android have wrong second tap location"; | ||
|
|
||
| [Test] | ||
| public void TapTwoPlacesQuickly() | ||
| { | ||
| if (App is not IApp2 app2 || app2 is null || app2.Driver is null) | ||
| { | ||
| throw new InvalidOperationException("Cannot run test. Missing driver to run quick tap actions."); | ||
| } | ||
|
|
||
| var tapAreaResult = App.WaitForElement(_tapAreaId); | ||
| var tapArea = tapAreaResult[0].Rect; | ||
|
|
||
| // The test harness coordinates are absolute | ||
| var xOffset = 50; | ||
| var harnessCenterX = tapArea.CenterX; | ||
| var harnessCenterY = tapArea.CenterY; | ||
|
|
||
| var point1 = new PointF(harnessCenterX - xOffset, harnessCenterY); | ||
| var point2 = new PointF(harnessCenterX + xOffset, harnessCenterY); | ||
|
|
||
| // The TapGesture coordinates are relative to the container, so we need to adjust | ||
| // for the container position | ||
| var expectedY = harnessCenterY - tapArea.Y; | ||
| var expectedX1 = point1.X - tapArea.X; | ||
| var expectedX2 = point2.X - tapArea.X; | ||
|
|
||
| // Just calling Tap twice will be too slow; we need to queue up the actions so they happen quickly | ||
| var actionsList = new TouchAction(app2.Driver); | ||
|
|
||
| // Tap the first point, then the second point | ||
| actionsList.Tap(point1.X, point1.Y).Tap(point2.X, point2.Y); | ||
| app2.Driver.PerformTouchAction(actionsList); | ||
|
|
||
| // The results for each tap should show up in the labels on the screen; find the text | ||
| // of each tap result and check to see that it meets the expected values | ||
| var result = App.WaitForElement("Tap1Label"); | ||
| AssertCorrectTapLocation(result[0].Text, expectedX1, expectedY, "First"); | ||
|
|
||
| result = App.WaitForElement("Tap2Label"); | ||
| AssertCorrectTapLocation(result[0].Text, expectedX2, expectedY, "Second"); | ||
| } | ||
|
|
||
| static void AssertCorrectTapLocation(string tapData, float expectedX, float expectedY, string which) | ||
| { | ||
| // Turn the text values into numbers so we can compare with a tolerance | ||
| (var tapX, var tapY) = ParseCoordinates(tapData); | ||
|
|
||
| Assert.AreEqual((double)expectedX, tapX, 1, $"{which} tap has unexpected X value"); | ||
| Assert.AreEqual((double)expectedY, tapY, 1, $"{which} tap has unexpected Y value"); | ||
| } | ||
|
|
||
| static (double, double) ParseCoordinates(string text) | ||
| { | ||
| var values = text.Split(',', StringSplitOptions.TrimEntries) | ||
| .Select(double.Parse) | ||
| .ToArray(); | ||
|
|
||
| return (values[0], values[1]); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert changes on this file.
Otherwise looks like it works super!