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

Make Add/Remove Logical public #16046

Merged
merged 21 commits into from
Jul 18, 2023
Merged

Make Add/Remove Logical public #16046

merged 21 commits into from
Jul 18, 2023

Conversation

PureWeen
Copy link
Member

@PureWeen PureWeen commented Jul 8, 2023

Description of Change

~Microsoft.Maui.Controls.Element.AddLogicalChild(Microsoft.Maui.Controls.Element element) -> void
~Microsoft.Maui.Controls.Element.InsertLogicalChild(int index, Microsoft.Maui.Controls.Element element) -> void
~Microsoft.Maui.Controls.Element.RemoveLogicalChild(Microsoft.Maui.Controls.Element element) -> bool
Microsoft.Maui.Controls.Element.ClearLogicalChildren() -> void
  • Adds a set of APIs (AddLogicalChild, RemoveLogicalChild, ClearLogicalChildren, InsertLogicalChild) to streamline our setup of parent/child relationships and also allow for outside developers to opt into the benefits of becoming a logical child.

  • Expose a set of public APIs on Element that allow users to add/remove logical children to any Element. There are a number of features we have that rely on LogicalChildren: LiveVisualTree, HotReload, Style propagation, etc. The LogicalChildren structure is currently closed off from the outside world. If someone implements a custom control based off of Element, there's not really an easy way for them to setup LogicalChildren. This makes it almost impossible for 3rd party controls to setup a proper hierarchy that will benefit from LiveVisualTree, HotReload, Style propagation, BC propagation, etc.

  • Rework internal code to use these new APIs

Examples

For example, the MCT has a popup control that lives on a Page. Because that popup can't add itself to the LogicalChidlren of a Page the popup control doesn't work with Hot Reload or Live Visual Tree

CommunityToolkit/Maui#1168

var page = new ContentPage()
{
	Content = new VerticalStackLayout()
};

var customControl = new MyCustomControl();
page.AddLogicalChild(customControl);

fixes #3745
fixes #15760

@PureWeen PureWeen force-pushed the make_add_remove_logical_public branch from 12eb69d to 63e60b4 Compare July 10, 2023 19:13
@PureWeen PureWeen marked this pull request as ready for review July 10, 2023 21:44
@samhouts samhouts added area-controls-collectionview CollectionView, CarouselView, IndicatorView area-controls-flyoutpage FlyoutPage area-layout StackLayout, GridLayout, ContentView, AbsoluteLayout, FlexLayout, ContentPresenter area-controls-listview ListView and TableView area-controls-modal area-navigation NavigationPage area-controls-shell Shell Navigation, Routes, Tabs, Flyout area-controls-border Border area-controls-flyout Flyout area-controls-menubar Desktop MenuBarItems area-controls-pages Page types area-controls-swipeview SwipeView area-controls-window Window labels Jul 11, 2023
@ghost ghost added the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label Jul 11, 2023
@PureWeen PureWeen force-pushed the make_add_remove_logical_public branch 2 times, most recently from 74a0423 to 91ee26c Compare July 13, 2023 02:54
@@ -143,7 +143,7 @@ public async Task CollectionViewCanSizeToContent(CollectionViewSizingTestCase te
await WaitForUIUpdate(frame, collectionView);
frame = collectionView.Frame;

#if WINDOWS
#if WINDOWS || ANDROID
Copy link
Member Author

@PureWeen PureWeen Jul 13, 2023

Choose a reason for hiding this comment

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

This PR adds a slightly longer delay to OnLoaded this gives Android a little more time which causes an OnMeasure loop to now fire on the ContentPage before these tests start running. Because of this, Android also needs this wait.

@PureWeen PureWeen force-pushed the make_add_remove_logical_public branch from 38f77ec to 9e13593 Compare July 17, 2023 21:12
Copy link
Member

@mattleibow mattleibow left a comment

Choose a reason for hiding this comment

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

This is looking really good - makes me so happy to remove all the internal magic!

@PureWeen PureWeen force-pushed the make_add_remove_logical_public branch from f03cda9 to ced4dfa Compare July 18, 2023 17:46
@PureWeen PureWeen requested a review from mattleibow July 18, 2023 17:46
@PureWeen PureWeen requested a review from MartyIX July 18, 2023 18:58

return true;
}

internal void ClearLogicalChildren()
/// <summary>
/// Removes all <see cref="Element"/>s.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Removes all <see cref="Element"/>s.
/// Removes all <see cref="Element"/>s.

@@ -26,7 +26,7 @@ public void Dispose()

void OnVisualTreeChanged(object? sender, VisualTreeChangeEventArgs e)
{
Assert.True(e.ChildIndex >= 0, "Visual Tree inaccurate when OnVisualTreeChanged called");
Assert.True(e.ChildIndex >= 0,$"Visual Tree inaccurate when OnVisualTreeChanged called. ChildIndex: {e.ChildIndex}");
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Assert.True(e.ChildIndex >= 0,$"Visual Tree inaccurate when OnVisualTreeChanged called. ChildIndex: {e.ChildIndex}");
Assert.True(e.ChildIndex >= 0, $"Visual Tree inaccurate when OnVisualTreeChanged called. ChildIndex: {e.ChildIndex}");

@PureWeen PureWeen merged commit 6136a8a into main Jul 18, 2023
@PureWeen PureWeen deleted the make_add_remove_logical_public branch July 18, 2023 23:06
@spadapet spadapet added the partner/hot-reload-xaml Issues impacting XAML Hot Reload experiences label Jul 18, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Dec 8, 2023
@Eilon Eilon removed legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor area-controls-pages Page types labels May 10, 2024
@samhouts samhouts added the fixed-in-8.0.0-preview.7.8842 Look for this fix in 8.0.0-preview.7.8842! label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-controls-border Border area-controls-collectionview CollectionView, CarouselView, IndicatorView area-controls-flyout Flyout area-controls-flyoutpage FlyoutPage area-controls-listview ListView and TableView area-controls-menubar Desktop MenuBarItems area-controls-modal area-controls-shell Shell Navigation, Routes, Tabs, Flyout area-controls-swipeview SwipeView area-controls-window Window area-layout StackLayout, GridLayout, ContentView, AbsoluteLayout, FlexLayout, ContentPresenter area-navigation NavigationPage fixed-in-8.0.0-preview.7.8842 Look for this fix in 8.0.0-preview.7.8842! partner/hot-reload-xaml Issues impacting XAML Hot Reload experiences
Projects
None yet
6 participants