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

Picking functions TapGestureRecognizer & GetVisualTreeElements(point) have been improved in .NET 8.0 but both still cannot find a Border with no BackgroundColor #19612

Open
jonmdev opened this issue Dec 28, 2023 · 5 comments
Labels
area-gestures Gesture types platform/android 🤖 platform/iOS 🍎 platform/windows 🪟 s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working
Milestone

Comments

@jonmdev
Copy link

jonmdev commented Dec 28, 2023

Description

I just tested GetVisualTreeElements and TapGestureRecognizer in .NET 8.0 and I am very happy and excited to see at least one major problem has been resolved:

  • Objects like AbsoluteLayout and BoxView can now also be found whether they have a background or not (previously could only be found if Color/BackgroundColor was assigned or a child had a Color/BackgroundColor to fill the space).

There is also still a problem with click detection on Border. AbsoluteLayout and BoxView now detect clicks at all times, but Border still does not trigger the TapGestureRecognizer or GetVisualTreeElements if it doesn't have a BackgroundColor.

Given everything else of this nature has been fixed (.NET 7.0 still can't find any of AbsoluteLayout/BoxView/Border if it has no background on this same project, now in .NET 8.0 this only affects Border), I hope it is not too hard to fix.

@drasticactions I know you worked on fixing the Border bug. Thanks for that. Being able to find AbsoluteLayout and others like it has been hugely helpful (previously had to assign redundant backgrounds causing massive rendering burden).

Any thoughts or ideas on the cause of the remaining residual Border bug? Also do we know what version or timeline there is to see your fix for the Border shape handler click bug?

Thanks to everyone for the continued development and support.

Steps to Reproduce

Bug project code is all contained in: https://github.com/jonmdev/PickingBug/blob/main/PickingBug/App.xaml.cs

Otherwise this bug report is a default MAUI 8.0 project.

Can simply load project and press play to see Border is missed by current configuration. Or adjust settings as directed.

Reproduction Code

 //BUG DESCRIPTION:
     //Whether by TapGestureRecognizer or GetVisualTreeElements, Border cannot catch clicks unless it has a background assigned.
     //Set project with three options below (object type, whether background assigned, whether testing by tap gesture or timer running GetVisualTreeElements).
     //Note that you will get 3 outputs for objects found at position in all cases except:
     //If (ObjectToClickType == "Border" && hasBackground == false), the Border is not found.


 //SET TEST MODE: can only test TapGestureRecognizer or GetVisualTreeElements one at a time (gesture recognizer blocks get VisualTreeElements on Timer otherwise)
 enum TestMode {
     GestureRecognizer,
     GetVisualTreeElements
 }
 //BUG PROJECT:
 public partial class App : Application {

     //SELECT OBJECT TYPE TO TRY TO CLICK
     Type ObjectToClickType = typeof(Border); //use AbsoluteLayout, BoxView, or Border (only Border is missed if no background)
     
     //TOGGLE BACKGROUND COLOR
     bool hasBackground = false; // only catches click or getVisualTreeElements for Border if has background color

     //TOGGLE WHICH FUNCTION TO TEST (getVisualTreeElements or GestureRecognizer)
     TestMode testMode = TestMode.GetVisualTreeElements;

     public App() {

         //======================
         //BASIC PAGE CONFIG
         //======================

         //content page
         ContentPage contentPage = new ContentPage();
         contentPage.BackgroundColor = Colors.DarkRed;
         MainPage = contentPage;

         //dummy absolute layout to prevent resizing bug already reported here: https://github.com/dotnet/maui/issues/17883
         AbsoluteLayout abs = new();
         contentPage.Content = abs;

         //build click object
         var clickObject = Activator.CreateInstance(ObjectToClickType);
         abs.Add(clickObject as VisualElement);

         //resize objects to screen
         contentPage.SizeChanged += delegate {
             if (contentPage.Width > 0) {
                 abs.WidthRequest = (clickObject as View).WidthRequest = contentPage.Width;
                 abs.HeightRequest = (clickObject as View).HeightRequest = contentPage.Height;
             }
         };

         //add background coloration
         Color clickObjectColor = Colors.CornflowerBlue;
         if (hasBackground) {
             if (clickObject as BoxView != null) {
                 (clickObject as BoxView).Color = clickObjectColor;
             }
             else if (clickObject as AbsoluteLayout != null) {
                 (clickObject as AbsoluteLayout).BackgroundColor = clickObjectColor;
             }
             else if (clickObject as Border != null) {
                 (clickObject as Border).BackgroundColor = clickObjectColor;
             }
         }

         //========================================
         //TEST GESTURE BASED CLICK CATCHING
         //========================================
         if (testMode == TestMode.GestureRecognizer) {
             TapGestureRecognizer tap = new();
             tap.Tapped += (s, e) => {
                 TappedEventArgs args = e as TappedEventArgs;
                 var position = args.GetPosition(contentPage) ?? new Point(0, 0);
                 Debug.WriteLine("OBJECT TAPPED AT " + position + " AND FOUND:");
                 var picked = contentPage.GetVisualTreeElements(position);
                 foreach (var element in picked) {
                     Debug.WriteLine(element.GetType());
                 }
             };
             (clickObject as View).GestureRecognizers.Add(tap);
         }

         //======================================================================================================
         //TIMER FUNCTION TO TEST "GetVisualTreeElements" ON ITS OWN (ADDING GESTURE BLOCKS THIS OTHERWISE):
         //======================================================================================================
         else {

             var timer = Dispatcher.CreateTimer();
             timer.Interval = TimeSpan.FromSeconds(1);
             timer.IsRepeating = true;
             timer.Tick += delegate {
                 var picked = contentPage.GetVisualTreeElements(new Point(100, 100));
                 Debug.WriteLine("TIMER FUNCTION RUN & FOUND: ");
                 foreach (var element in picked) {
                     Debug.WriteLine(element.GetType());
                 }
             };
             timer.Start();
         }


     }
 }

Link to public reproduction project repository

https://github.com/jonmdev/PickingBug/

Version with bug

8.0.3

Is this a regression from previous behavior?

No, this is something new

Last version that worked well

Unknown/Other

Affected platforms

iOS, Android, Windows

Did you find any workaround?

None

ADDENDUM

Fyi @drasticactions I see you proposed a fix here for the other Border click issue where the shape was catching the click without a handler:

A simple solution I just implemented now as I don't have that fix yet was changing the line in GetVisualTreeElementsWindowsInternal from

var viewTree = visualElement.GetVisualTreeDescendants().Where(n => n is IView).Select(n => new Tuple<IView, object?>((IView)n, ((IView)n).ToPlatform(((IView)n).Handler.MauiContext)));

to:

var uniqueElements = findChildren(uiElement).Distinct();
var viewTree1 = visualElement.GetVisualTreeDescendants().Where(n => n is IView && n as Microsoft.Maui.Controls.Shapes.Rectangle == null && n as Microsoft.Maui.Controls.Shapes.RoundRectangle == null);
var viewTree = viewTree1.Select(n => new Tuple<IView, object?>((IView)n, ((IView)n).ToPlatform(((IView)n).Handler.MauiContext)));

And in Android/iOS, GetVisualTreeElementsInternal as:

if (visualElement is IView view && visualElement as Microsoft.Maui.Controls.Shapes.RoundRectangle == null && visualElement as Microsoft.Maui.Controls.Shapes.Rectangle == null) {

to replace:
if (visualElement is IView view) {

I mean, you can just filter out the Shapes here and the click seems to operate normally. A different solution for what it's worth.

@jonmdev jonmdev added the t/bug Something isn't working label Dec 28, 2023
@drasticactions
Copy link
Contributor

drasticactions commented Dec 29, 2023

@jonmdev I think it all stems from #3558, where the underlying border stroke shape is not a real rendered "Shape", but used to describe how the border should work. That decision to use IShape leads to issues you hit. They are not actual Shape elements that are implemented so you can't add Tap Gestures to them since there's no shape to attach to.

Forcing the handler to be added to that shape is wrong, since that makes a new shape element to be rendered that's not actually used by the border itself (It may happen to work, but that's a hack and incorrect due to how the API works under the covers.) hence why I didn't change it, since that wouldn't help.

So, IMO, I wouldn't try using gesture around borders shapes, and would instead either wrap them around another UI view, or use the underlying element instead. Any other changes to Border are beyond what I would do as I don't work on the MAUI team and I don't feel comfortable with making larger architecture decisions.

@jonmdev
Copy link
Author

jonmdev commented Dec 29, 2023

Thanks @drasticactions I didn't realize you weren't on the team. I am happy with this current situation at least where AbsoluteLayout can take the hits whether it has a background or not. I can at least wrap everything in that now and that is mostly what I have already done.

My workaround to filter out the Handler-free Shapes on picking error is also now working reasonably well.

I just mention this then for the MAUI team as they should seek to create a functional system that does not have all these strange potholes in the road. Or they should document that picking will not reliably work on Borders now without any background and this is "intended function." (Although I don't think it should be as now other objects can pick okay with no background).

The big problem I see with MAUI currently are there are just too many unexpected results that make designing anything a mystery. I upgraded to .NET 8 with the new picking fix I described above (previously couldn't) and .NET 8 fixed numerous visual glitches but then you still get weird things like this you have to figure out and some new glitches I see they're working on.

Many far more important bugs are just in the backlog. Eg. These Label bugs I believe are related and are a nightmare as nothing can reliably display Labels in Android and still no milestone or target:

The more bugs they fix and the less weird "undefined behavior" the more people may be willing to use the product and the longer lifespan we may get out of our work in it.

@Eilon Eilon added the area-gestures Gesture types label Dec 29, 2023
@gabsamples6
Copy link

Hi is the take on this to avoid using a tapgesturerecognizer with a border?.

@jonmdev
Copy link
Author

jonmdev commented Mar 3, 2024

Hi is the take on this to avoid using a tapgesturerecognizer with a border?.

I just saw your reply now so it has been some time since I checked this. But as I recall (from what I wrote) the TapGestureRecognizer will work if the Border has a background color assigned, or something in it with a background color, but otherwise not.

I have personally just stuck to identifying clicks on Layout (AbsoluteLayout, etc.) objects so my functions work consistently.

@Zhanglirong-Winnie Zhanglirong-Winnie added s/verified Verified / Reproducible Issue ready for Engineering Triage s/triaged Issue has been reviewed labels Apr 29, 2024
@Zhanglirong-Winnie
Copy link

Verified this issue with Visual Studio 17.10.0 Preview 5(8.0.21&8.0.7&8.0.3). Can repro on all platforms with sample project. hasBackground == false, Border cannot be found
image

@jsuarezruiz jsuarezruiz added this to the Backlog milestone Jun 7, 2024
@samhouts samhouts removed s/verified Verified / Reproducible Issue ready for Engineering Triage s/triaged Issue has been reviewed labels Jul 3, 2024
@samhouts samhouts added s/verified Verified / Reproducible Issue ready for Engineering Triage s/triaged Issue has been reviewed labels Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-gestures Gesture types platform/android 🤖 platform/iOS 🍎 platform/windows 🪟 s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

7 participants