dotnet / maui

.NET MAUI is the .NET Multi-platform App UI, a framework for building native device applications spanning mobile, tablet, and desktop.
https://dot.net/maui
MIT License
22.24k stars 1.76k forks source link

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 10 months ago

jonmdev commented 10 months ago

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:

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.

drasticactions commented 10 months ago

@jonmdev I think it all stems from https://github.com/dotnet/maui/issues/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 commented 10 months ago

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.

gabsamples6 commented 10 months ago

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

jonmdev commented 8 months ago

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 commented 6 months ago

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