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

Shapes without handlers shouldn't be added as LogicalChildren #16919

Closed
drasticactions opened this issue Aug 22, 2023 · 6 comments · Fixed by #18539
Closed

Shapes without handlers shouldn't be added as LogicalChildren #16919

drasticactions opened this issue Aug 22, 2023 · 6 comments · Fixed by #18539
Assignees
Labels
area-architecture Issues with code structure, SDK structure, implementation details area-drawing Shapes, Borders, Shadows, Graphics, BoxView, custom drawing fixed-in-8.0.6 Look for this fix in 8.0.6 SR1! partner/hot-reload-xaml Issues impacting XAML Hot Reload experiences platform/android 🤖 platform/iOS 🍎 platform/macOS 🍏 macOS / Mac Catalyst 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

@drasticactions
Copy link
Contributor

Description

image

It is always null when calling for the Handler on a Maui shape. This matters as tooling such as XAML Hot Reload and the Live Visual Tree attempt to draw borders around elements when you hover over them in the menus. If an MAUI Shape appears in the list, we will try and render a border around it. That will fail, since the handler is null, and it will throw an exception above, crashing the app.

That can be worked around in XAML Hot Reload, but we should see why it's null here too. It's broken in both net7.0 and net8.0, and on all platforms.

Steps to Reproduce

  1. Create a Shape (such as a border shape)
  2. Try and reference the handler

It will be null.

Link to public reproduction project repository

https://github.com/drasticactions/MauiRepros/tree/main/MauiShapeNet7

Version with bug

7.0.92

Is this a regression from previous behavior?

No, this is something new

Last version that worked well

Unknown/Other

Affected platforms

iOS, Android, Windows, macOS

Affected platform versions

No response

Did you find any workaround?

No response

Relevant log output

No response

@drasticactions drasticactions added the t/bug Something isn't working label Aug 22, 2023
@rachelkang rachelkang added the area-drawing Shapes, Borders, Shadows, Graphics, BoxView, custom drawing label Aug 23, 2023
@rachelkang rachelkang added this to the Backlog milestone Aug 23, 2023
@ghost
Copy link

ghost commented Aug 23, 2023

We've added this issue to our backlog, and we will work to address it as time and resources allow. If you have any additional information or questions about this issue, please leave a comment. For additional info about issue management, please read our Triage Process.

@rachelkang rachelkang added the area-architecture Issues with code structure, SDK structure, implementation details label Aug 23, 2023
@XamlTest XamlTest added s/verified Verified / Reproducible Issue ready for Engineering Triage s/triaged Issue has been reviewed labels Aug 29, 2023
@XamlTest
Copy link

Verified this on Visual Studio Enterprise 17.8.0 Preview 1.0. Repro on Windows 11 .NET 8, Android 13.0-API33 and iOS 16.4 with below Project:
MauiShape.zip

After clicking the button, it will show "Border Shape Null"
image

@chabiss
Copy link
Contributor

chabiss commented Nov 3, 2023

@samhouts This bug blocks https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1911092, which is a severe regression to .NET 7.

@drasticactions drasticactions changed the title Handler not set on MAUI Shapes Handler not set on MAUI Border Shapes Nov 3, 2023
@drasticactions
Copy link
Contributor Author

With some more testing: Shapes alone rendered on screen do appear to have handlers correctly attached after they are handled. For Borders specifically, StrokeShapes (which are rendered on screen) do not have handlers.

@PureWeen PureWeen changed the title Handler not set on MAUI Border Shapes Shapes without handlers shouldn't be added as LogicalChildren Nov 4, 2023
@PureWeen PureWeen changed the title Shapes without handlers shouldn't be added as LogicalChildren Shapes without handlers shouldn't be added as LogicalChildren and review if GetVisualTreeElements should only return things with handlers Nov 4, 2023
@PureWeen PureWeen added this to the .NET 8 SR1 milestone Nov 4, 2023
@PureWeen PureWeen changed the title Shapes without handlers shouldn't be added as LogicalChildren and review if GetVisualTreeElements should only return things with handlers Shapes without handlers shouldn't be added as LogicalChildren Nov 4, 2023
@jsuarezruiz jsuarezruiz self-assigned this Nov 6, 2023
@hansmbakker
Copy link

hansmbakker commented Dec 5, 2023

I see that this issue is fixed now. I haven't been able to test it yet, but will a Border now show up its outline in Xaml Live Preview or not?

From the description saying "ignore elements with null handlers" it sounds as if it will be ignored (as a workaround) rather than the Border element having a correct implementation that provides the right info to Xaml Live Preview?

If so, is there an issue tracking this?
If not, apologies for misunderstanding.

@drasticactions
Copy link
Contributor Author

drasticactions commented Dec 5, 2023

Short answer: Yes*

Long Answer: Borders could always be drawn (as they were with net7.0), the issue was with Border StrokeShapes. They not "technically" shapes, but are used as to define how the border should be drawn. This is inconsistent with general UI elements, which have handlers that should never be null, and since StrokeShape is a UI Element, it was added to LogicalChildren with net8.0, which was then breaking the XAML Live Preview Adorner layer, since UI Elements once rendered should never have null handlers.

#3558

This issue is tracking a change of it from Shape to Geometry, but it's a big API shift.

With net8.0, those StrokeShapes (Or any element that doesn't have a handler, which so far I believe this is the only one) are removed from the Visual Tree list, so now if you have any border with a StrokeShape, it won't break the visual tree adorner layer.

*There may be cases where it doesn't work that we don't know of, since it can be used on a huge amount of controls, you could find a case where it does not work, in which case, file a bug.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-architecture Issues with code structure, SDK structure, implementation details area-drawing Shapes, Borders, Shadows, Graphics, BoxView, custom drawing fixed-in-8.0.6 Look for this fix in 8.0.6 SR1! partner/hot-reload-xaml Issues impacting XAML Hot Reload experiences platform/android 🤖 platform/iOS 🍎 platform/macOS 🍏 macOS / Mac Catalyst platform/windows 🪟 s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working
Projects
Status: Done
8 participants