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

[iOS] Fix gestures in Label Spans #15544

Merged
merged 6 commits into from Feb 15, 2024
Merged

[iOS] Fix gestures in Label Spans #15544

merged 6 commits into from Feb 15, 2024

Conversation

jsuarezruiz
Copy link
Contributor

@jsuarezruiz jsuarezruiz commented Jun 9, 2023

Description of Change

Fix gestures in Label Spans on iOS.

To test/validate the changes, open the .NET MAUI Gallery and navigate to the Label page. Tap the Span with the text "Click Me". If the Span changes the TextColor, is working as expected.

Also tested using the Issue3525 UI Test.

Issues Fixed

Fixes #8004
Fixes #4734

@mattleibow
Copy link
Member

Is there a reason this is still in draft @jsuarezruiz?

@mattleibow
Copy link
Member

/rebase

@@ -124,5 +126,116 @@ public static class FormattedStringExtensions

return attrString;
}

public static void RecalculateSpanPositions(this UILabel control, Label element)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This comes from Xamarin.Forms, although in .NET MAUI I see an incorrect calculation of the Region that defines the limits of each Span when there are multiple lines.

@jsuarezruiz
Copy link
Contributor Author

Is there a reason this is still in draft @jsuarezruiz?

Yes, sorry. All my open PRs, are Draft except 4 to focus on them, speed up merges and reduce the number of open PRs (20 at the moment).

@Redth Redth added this to the .NET 8 GA milestone Jul 18, 2023
src/Core/src/Platform/iOS/MauiLabel.cs Outdated Show resolved Hide resolved
@jsuarezruiz
Copy link
Contributor Author

Added UI Test.

@jsuarezruiz jsuarezruiz marked this pull request as ready for review August 17, 2023 10:24
@jsuarezruiz jsuarezruiz requested a review from a team as a code owner August 17, 2023 10:24
@samhouts samhouts added the stale Indicates a stale issue/pr and will be closed soon label Sep 11, 2023
@samhouts
Copy link
Member

Tests are failing?

@samhouts samhouts added the s/pr-needs-author-input PR needs an update from the author label Sep 11, 2023
@ghost
Copy link

ghost commented Sep 11, 2023

Hi @jsuarezruiz. We have added the "s/pr-needs-author-input" label to this issue, which indicates that we have an open question/action for you before we can take further action. This PRwill be closed automatically in 14 days if we do not hear back from you by then - please feel free to re-open it if you come back to this PR after that time.

@ghost ghost removed stale Indicates a stale issue/pr and will be closed soon s/pr-needs-author-input PR needs an update from the author labels Sep 12, 2023
Copy link
Member

@PureWeen PureWeen left a comment

Choose a reason for hiding this comment

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

I re-enabled the span tests on this one. It looks like Issue4734Test ian't passing. Ignore the rest of the tests, those will be fixed by different PRs and aren't related to this one.

https://dev.azure.com/xamarin/public/_build/results?buildId=96398&view=ms.vss-test-web.build-test-results-tab&runId=2113711&resultId=100004&paneView=debug

@tj-devel709
Copy link
Contributor

iOS update Android update
iOS_Span_update1.mov
android_Span_update1.mov

Comment on lines 105 to 115
{
var region = Regions[i];

if (i == 0) // this is the first line
region.Top -= top;
region.Top -= top;

region.Left -= left;
region.Width += right + left;

if (i == Regions.Count - 1) // This is the last line
region.Height += bottom + top;
region.Height += bottom + top;

rectangles[i] = region;
Copy link
Contributor

Choose a reason for hiding this comment

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

Made these changes since moving the region up but not making it bigger only really makes sense if we can guarantee that the next line will be right below the top line. This doesn't work in cases where the top line is at the end of the line and goes a little onto the next line for example.

@tj-devel709
Copy link
Contributor

Waiting for the iOS UITests to work again!

Copy link
Contributor Author

@jsuarezruiz jsuarezruiz left a comment

Choose a reason for hiding this comment

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

Tested on simulator and device:
fix-spans-ios
The touch region works really nice.
The build is failing with some Android UITests failing. All the failing tests fails because a timeout "OneTimeSetUp: System.TimeoutException : Timed out waiting for Go To Test button to appear", probably one of the tests is crashing or closing the App and from that point the rest fails on cascade.

@PureWeen
Copy link
Member

/rebase

mattleibow and others added 6 commits February 14, 2024 20:11
commit 694fba9
Merge: b388f57 1f21047
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Mon Dec 4 12:03:39 2023 +0100

    Merge branch 'main' into fix-4734-ios

commit b388f57
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Thu Nov 30 08:59:24 2023 +0100

    Updated tests

commit 89353df
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Wed Nov 29 17:29:14 2023 +0100

    Fix merge issue

commit 99db35e
Merge: fdb7113 4a31ee1
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Tue Nov 28 13:50:36 2023 +0100

    Merge branch 'main' into fix-4734-ios

commit fdb7113
Merge: ad27d6e f6956cb
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Fri Nov 3 09:08:01 2023 +0100

    Merge branch 'main' into fix-4734-ios

commit ad27d6e
Merge: c87a842 783db24
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Wed Oct 25 14:07:27 2023 +0200

    Merge branch 'main' into fix-4734-ios

commit c87a842
Merge: a507d1e e25d331
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Fri Oct 20 13:34:23 2023 +0200

    Merge branch 'main' into fix-4734-ios

commit a507d1e
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Wed Oct 18 17:13:11 2023 +0200

    Updated test

commit aa0f949
Merge: b12077e 3bd8421
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Wed Oct 18 13:25:01 2023 +0200

    Merge branch 'main' into fix-4734-ios

commit b12077e
Merge: 8e45456 0f764ac
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Wed Oct 18 08:40:46 2023 +0200

    Merge branch 'main' into fix-4734-ios

commit 8e45456
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Fri Oct 6 11:47:49 2023 +0200

    Fix the test

commit 08176dc
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Thu Oct 5 12:30:35 2023 +0200

    Updated test

commit e7ed571
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Wed Oct 4 14:20:15 2023 +0200

    Updated test

commit 93e8095
Merge: 1145f10 a05a5a7
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Wed Oct 4 13:52:29 2023 +0200

    Merge branch 'main' into fix-4734-ios

commit 1145f10
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Fri Sep 29 11:46:28 2023 +0200

    Update src/Controls/tests/UITests/Tests/Issues/Issue3525.cs

    Co-authored-by: Gerald Versluis <gerald.versluis@microsoft.com>

commit 6f272d4
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Thu Sep 28 16:23:00 2023 +0200

    Changes in tests

commit 7c8d533
Merge: b69cc28 9895399
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Wed Sep 27 10:51:18 2023 +0200

    Merge branch 'main' into fix-4734-ios

commit b69cc28
Author: Shane Neuville <shneuvil@microsoft.com>
Date:   Sun Sep 17 10:41:12 2023 -0500

    - add span tests back

commit 3f4b36b
Merge: c21fe9a fdbeb81
Author: Shane Neuville (HE/HIM) <shneuvil@microsoft.com>
Date:   Sun Sep 17 10:34:32 2023 -0500

    Merge remote-tracking branch 'origin/main' into fix-4734-ios

commit c21fe9a
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Wed Sep 13 15:55:22 2023 +0200

    Fix build errors

commit 0669553
Merge: fdf5296 aff9b34
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Tue Sep 12 13:02:14 2023 +0200

    Merge branch 'main' into fix-4734-ios

commit fdf5296
Merge: ee2cd4e ace9fe5
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Fri Aug 18 09:02:59 2023 +0200

    Merge branch 'main' into fix-4734-ios

commit ee2cd4e
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Thu Aug 17 12:23:35 2023 +0200

    Remove unnecessary event in iOS MauiLabel

commit cd8098f
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Thu Aug 17 12:13:20 2023 +0200

    Changes in test

commit 797872d
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Thu Aug 17 12:12:46 2023 +0200

    Fix build error

commit 5c7ff39
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Thu Aug 17 11:53:06 2023 +0200

    Added UI Test

commit deaf66d
Merge: b68a52d 34b481d
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Thu Aug 17 09:57:08 2023 +0200

    Merge branch 'main' into fix-4734-ios

commit b68a52d
Merge: 4854977 e6a4334
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Thu Aug 3 15:44:30 2023 +0200

    Merge branch 'main' into fix-4734-ios

commit 4854977
Merge: 74a8d7a f241014
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Tue Jul 25 12:00:27 2023 +0200

    Merge branch 'main' into fix-4734-ios

commit 74a8d7a
Merge: b62d39d 443d3a0
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Tue Jul 18 11:49:35 2023 +0200

    Merge branch 'main' into fix-4734-ios

commit b62d39d
Merge: 05b0495 b9f64e1
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Tue Jul 11 12:42:20 2023 +0200

    Merge branch 'main' into fix-4734-ios

commit 05b0495
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Thu Jul 6 12:16:43 2023 +0200

    Fix the build

commit b717d17
Merge: b1543a3 700944b
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Wed Jul 5 09:12:33 2023 +0200

    Merge branch 'main' into fix-4734-ios

commit b1543a3
Merge: d55348b db60162
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Fri Jun 30 09:41:55 2023 +0200

    Merge branch 'main' into fix-4734-ios

commit d55348b
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Fri Jun 23 19:34:48 2023 +0200

    Fix broken tests

commit f48ca85
Merge: a725311 3622625
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Thu Jun 22 11:56:09 2023 +0200

    Merge branch 'main' into fix-4734-ios

commit a725311
Merge: 3d4326c b7afc3b
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Wed Jun 14 12:00:53 2023 +0200

    Merge branch 'main' into fix-4734-ios

commit 3d4326c
Merge: e99fa67 233fa2f
Author: Matthew Leibowitz <mattleibow@live.com>
Date:   Thu May 18 19:53:40 2023 +0800

    Merge remote-tracking branch 'origin/main' into fix-4734-ios

commit e99fa67
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Wed May 10 10:11:43 2023 +0200

    More changes

commit 9c45894
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Wed May 10 10:11:35 2023 +0200

    More changes

commit a0747e4
Author: Javier Suárez <javiersuarezruiz@hotmail.com>
Date:   Wed May 10 10:01:00 2023 +0200

    Fix span gestures on iOS

Co-authored-by: Javier Suárez <javiersuarezruiz@hotmail.com>
@PureWeen PureWeen merged commit b2f64f2 into main Feb 15, 2024
44 of 49 checks passed
@PureWeen PureWeen deleted the fix-4734-ios branch February 15, 2024 05:35
@jameslavery
Copy link

Can someone confirm that this is fixed for Windows too, please?

@sjorsmiltenburg
Copy link

yep, I desperately need this for windows too

@RobTF
Copy link

RobTF commented Mar 4, 2024

What version of .NET MAUI will this fix appear in? We have users complaining they can't tap links on iOS and we'd like to be able to give an ETA on the fix.

thanks

@MartyIX
Copy link
Collaborator

MartyIX commented Mar 4, 2024

Can someone confirm that this is fixed for Windows too, please?

You can try a nightly build (https://github.com/dotnet/maui/wiki/Nightly-Builds) to verify if the PR fixes your issue.

@MartyIX
Copy link
Collaborator

MartyIX commented Mar 4, 2024

What version of .NET MAUI will this fix appear in? We have users complaining they can't tap links on iOS and we'd like to be able to give an ETA on the fix.

thanks

Not a MAUI team member but https://github.com/orgs/dotnet/projects/194/views/22?pane=issue&itemId=15608832 shows that it should be released as a part of the SR3 release. So hopefully at most weeks. No guarantees.

@RobTF
Copy link

RobTF commented Mar 4, 2024

Thanks @MartyIX. We wouldn't generally play with the nightly builds as we can't use them in production anyway, so we're interested in getting a version/date of when the fix is officially released but thanks or the idea and looking into it for me. I struggled to find that page.

@vallgrenerik
Copy link

vallgrenerik commented Mar 18, 2024

Can confirm that this solves my problem on iOS
Update to:

<PackageReference Include="Microsoft.Maui.Controls" Version="8.0.10" />
<PackageReference Include="Microsoft.Maui.Controls.Compatibility" Version="8.0.10" />

Then clean and build! 👏

@RobTF
Copy link

RobTF commented Mar 18, 2024

Yes we spotted this fix is in SR3, thanks!

@kimhongka
Copy link

kimhongka commented Mar 27, 2024

Found this issue happens when the Label with Span Gensture is wrapped with Frame. It is still a bug. The workaround solution for this is using Border control instead of Frame.

@alexKuchyk-godel
Copy link

issue still reproduced on iOS

@RobTF
Copy link

RobTF commented Apr 2, 2024

We found that this issue was fixed in SR3 - has it regressed in SR3.1?

@alexKuchyk-godel
Copy link

Issue still reproduced on:

<PackageReference Include="Microsoft.Maui.Controls" Version="8.0.14" />
<PackageReference Include="Microsoft.Maui.Controls.Compatibility" Version="8.0.14" />
image

Maybe I missed something?

@zleao
Copy link
Contributor

zleao commented Apr 3, 2024

Same here.
Not working on iPhone and iPad...
Using the same versions as @alexKuchyk-godel

@varyamereon
Copy link

varyamereon commented Apr 5, 2024

Same here, disappointing, have been waiting for this fix for a long time.

I have tried to make a reproduction sample but it works there using a ScrollView->VerticalStackLayout->Grid->Label->Span. Using exactly the same in my production app and the command doesn't fire 🤷🏻‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TapGestureRecognizer does not work in label's span Gestures don't work on Label Spans