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
21.87k stars 1.69k forks source link

ContentView inherits from the wrong base Layout class #7613

Open ivan-todorov-progress opened 2 years ago

ivan-todorov-progress commented 2 years ago

Description

There are two parallel layout implementations in .NET MAUI:

  1. The new layout implementation with the base class of Microsoft.Maui.Controls.Layout
  2. The legacy layout implementation from Xamarin.Forms with the base class of Microsoft.Maui.Controls.Compatibility.Layout

The new layout implementation is compatible with the new MAUI handlers. The old layout implementation from Xamarin.Forms is not compatible with the handlers and should be avoided.

The problem with the ContentView is that it has a handler, but is still inherits from the legacy layout Microsoft.Maui.Controls.Compatibility.Layout. This is a major source of bugs, as the usual layout logic might not be executed correctly or might not get executed at all on some platforms.

Steps to Reproduce

Download and run the attached sample project on Windows.

TestApp.zip

The sample project contains a single CustomView inheriting from ContentView. The sole purpose of that CustomView is to override all the virtual Measure, Arrange, Layout etc. methods of the base class and dump some debug output when they are called. The sample applications has two additional buttons to call the InvalidateMeasure method on the CustomView from the base VisualElement class and from the IView interface.

Observe the debug output of the application. You can also put breakpoints in the overridden methods to be sure. Notice that nothing happens: nothing is called at all. All the layout logic of the CustomView is completely discarded.

Version with bug

6.0 (current)

Last version that worked well

Unknown/Other

Affected platforms

Windows

Affected platform versions

N/A

Did you find any workaround?

Unless Microsoft changes the implementation of the ContentView to inherit from the new Microsoft.Maui.Controls.Layut and implements the necessary glue code, I cannot suggest any viable workaround, except not using ContentView or any templated controls in the .NET MAUI applications.

Relevant log output

No response

drasticactions commented 2 years ago

Wouldn't this be an issue with the Telerik controls you're using? I'm not sure how this is a MAUI issue.

ivan-todorov-progress commented 2 years ago

Wouldn't this be an issue with the Telerik controls you're using? I'm not sure how this is a MAUI issue.

Please, excuse me for the confusion. The correct types are Microsoft.Maui.Controls.Layout and Microsoft.Maui.Controls.Compatibility.Layout. I have corrected the description of the bug.

The sample application does not use any Telerik controls at all. It just inherits from Microsoft.Maui.Controls.ContentView.

AswinPG commented 2 years ago

https://github.com/dotnet/maui/issues/7668 Could this be related ?

ivan-todorov-progress commented 2 years ago

@AswinPG I have not debugged the MAUI source code to that extent TBH, but the LayoutChildren method in ContentPage seems like an old paradigm from the legacy Xamarin.Forms layout implementation. The new MAUI layouts have something called ILayoutManager, with two methods: Measure and ArrangeChildren: https://github.com/dotnet/maui/blob/main/src/Core/src/Layouts/ILayoutManager.cs.

It is a total mess really, as the new and the legacy layout implementations define different paradigms and sometimes combining the two in the same visual tree does not work quite well, i.e.: InvalidateMeasure not invalidating anything, the Measure and Layout/Arrange methods not always called when they should etc. Migrating everything to the new MAUI layouts would resolve many such problems and this should be top priority for the MAUI team IMHO, as a broken layout system would mean broken apps and many disappointed developers migrating to other UI frameworks. ;-)

Xyncgas commented 2 years ago

I've been finding using webview to draw layout can already be satisfying enough, for my app to run on multiple device while having native platform capabilities. If creating layout in MAUI is easy enough then I would use it, however you are gonna have to top all the dom events they have in the webview (onclick -- I supposed there's button in MAUI, onmouseover, onkeyup...) but also is MAUI supporting everything well but linux

philipag commented 2 years ago

I agree with @ivan-todorov-progress in that this is the type of bug that is making it next to impossible to migrate complex XF apps to Maui. This should be top priority to get fixed.

ghost commented 1 year ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

Zhanglirong-Winnie commented 10 months ago

Verified this issue with Visual Studio Enterprise 17.8.0 Preview 1.0. Can repro on windows platform with sample project. TestApp.zip

MitchBomcanhao commented 8 months ago

not being able to use contentviews with control templates makes migrating our xamarin forms app nearly impossible.

Alex-111 commented 8 months ago

The later this will be fixed the more apps will break. Any reasons why this is delayed?

MitchBomcanhao commented 6 months ago

bump!

GalaxiaGuy commented 5 months ago

@hartez In #19794 you say this isn't exactly a bug.

Does that mean it is intentional that ContentView (and TemplatedView) inherits from Compatibility.Layout?

hartez commented 5 months ago

@hartez In #19794 you say this isn't exactly a bug.

Does that mean it is intentional that ContentView (and TemplatedView) inherits from Compatibility.Layout?

Yes, insofar as they inherited from that class in Forms, and that base class relationship was left intact for backward compatibility reasons in the first release of .NET MAUI.

Starting from scratch, and with no backward-compatibility goals, I doubt we'd have kept that inheritance chain. Which is why I say it isn't exactly a bug; given the current capabilities and design, it certainly looks like one, but it was an intentional design choice at one point. And "fixing" it would require a breaking change.

MitchBomcanhao commented 5 months ago

Considering you have to remove the compatibility mode in order to get content view to even render on windows, seems like it is already broken. Might as well just fix it.

Xyncgas commented 5 months ago

You are losing new customer for your new framework by remaining compatibility

jump32 commented 5 months ago

@hartez So why not have a version of ContentView with this inheritance chain under the Compatibility namespace? In the same way as you do for other controls, such as Stack, etc.

That way you can have a version in the Controls namespace that has the correct inheritance. That isn't fundamentally broken. And that, you know, actually works.

hartez commented 5 months ago

@hartez So why not have a version of ContentView with this inheritance chain under the Compatibility namespace? In the same way as you do for other controls, such as Stack, etc.

That way you can have a version in the Controls namespace that has the correct inheritance. That isn't fundamentally broken. And that, you know, actually works.

I'm not clear on what you all mean by "ContentView not working", but if I take the repro project attached to this issue and update it to .NET 8, the CustomView class displays just fine.

Now, if you actually put some custom measurement logic in CustomView and aren't just calling base.MeasureOverride, at the moment you're required to explicitly set the value of DesiredSize; if you don't, the layout won't work correctly.

So this won't work:

protected override Size MeasureOverride(double widthConstraint, double heightConstraint)
{
    var size = new Size(200,200);
    return size;
}

But this will:

protected override Size MeasureOverride(double widthConstraint, double heightConstraint)
{
    var size = new Size(200,200);
    DesiredSize = size;
    return size;
}

This was a design/documentation error (it wasn't documented that you needed to set DesiredSize, and really the root classes should just be doing that for you anyway). So after #19794, you won't have to do that anymore - the first version of MeasureOverride above will work.

But that's all orthogonal to the "wrong base class" claim. The original error reported (1.5 years ago) has been fixed, and the fix didn't have much to do with the base class. Changing the base class now (even if you could without breaking any existing code) is unlikely to address any problems you're having. If I could I would just close this issue.

MitchBomcanhao commented 5 months ago

@hartez what I mean by it not working is that if you have the maui compatibility mode enabled in mauiprogram.cs, then windows will not render contentview stuff. I don't know if the two problems are linked, but contentview is completely broken on windows when using compatibility mode, so this idea of not changing it because it becomes a breaking change is quite funny for everyone that is actually trying to convert existing applications and finding them to just not work at all.

hartez commented 5 months ago

This issue isn't about compatibility mode, though. And I'm not sure I see how changing the base class of ContentView would fix something that's broken with compatibility mode, especially if it's only broken in compatibility mode on one platform. That it's only broken on one platform suggests a bug in the compatibility renderer for that platform, not an issue with the hierarchy in the cross-platform code.

It looks like there are other issues open for the actual problem (i.e., that ContentView doesn't work properly in compatibility mode); I would suggest the focus be on those issues (e.g., #19779), not this one.

Unless someone has opened a PR that fixes the problem (without an API break) by changing the base class?