Revert "Code Quality: Removed ISidebarItemModel 2 (#18061)"

This reverts commit 3e1fb20.

Author: yair100

src/Files.App.Controls/Sidebar/ISidebarItemModel.cs

@@ -0,0 +1,24 @@
+// Copyright (c) Files Community
+// Licensed under the MIT License.
+
+namespace Files.App.Controls
+{
+	public interface ISidebarItemModel : INotifyPropertyChanged
+	{
+		/// <summary>
+		/// The children of this item that will be rendered as child elements of the SidebarItem
+		/// </summary>
+		object? Children { get; }
+
+		/// <summary>
+		/// Determines whether the SidebarItem is expanded and the children are visible 
+		/// or if it is collapsed and children are not visible.
+		/// </summary>
+		bool IsExpanded { get; set; }
+
+		/// <summary>
+		/// Indicates whether the children should have an indentation or not.
+		/// </summary>
+		bool PaddedItem { get; }
+	}
+}

src/Files.App.Controls/Sidebar/SidebarItem.Properties.cs

@@ -2,12 +2,19 @@
 // Licensed under the MIT License.
 
 using CommunityToolkit.WinUI;
-using System.Collections.Specialized;
 
 namespace Files.App.Controls
 {
 	public sealed partial class SidebarItem : Control
 	{
+		public SidebarView? Owner
+		{
+			get { return (SidebarView?)GetValue(OwnerProperty); }
+			set { SetValue(OwnerProperty, value); }
+		}
+		public static readonly DependencyProperty OwnerProperty =
+			DependencyProperty.Register(nameof(Owner), typeof(SidebarView), typeof(SidebarItem), new PropertyMetadata(null));
+
 		public bool IsSelected
 		{
 			get { return (bool)GetValue(IsSelectedProperty); }
@@ -24,6 +31,14 @@ public bool IsExpanded
 		public static readonly DependencyProperty IsExpandedProperty =
 			DependencyProperty.Register(nameof(IsExpanded), typeof(bool), typeof(SidebarItem), new PropertyMetadata(true, OnPropertyChanged));
 
+		public bool IsInFlyout
+		{
+			get { return (bool)GetValue(IsInFlyoutProperty); }
+			set { SetValue(IsInFlyoutProperty, value); }
+		}
+		public static readonly DependencyProperty IsInFlyoutProperty =
+			DependencyProperty.Register(nameof(IsInFlyout), typeof(bool), typeof(SidebarItem), new PropertyMetadata(false));
+
 		public double ChildrenPresenterHeight
 		{
 			get { return (double)GetValue(ChildrenPresenterHeightProperty); }
@@ -33,6 +48,14 @@ public double ChildrenPresenterHeight
 		public static readonly DependencyProperty ChildrenPresenterHeightProperty =
 			DependencyProperty.Register(nameof(ChildrenPresenterHeight), typeof(double), typeof(SidebarItem), new PropertyMetadata(30d));
 
+		public ISidebarItemModel? Item
+		{
+			get { return (ISidebarItemModel)GetValue(ItemProperty); }
+			set { SetValue(ItemProperty, value); }
+		}
+		public static readonly DependencyProperty ItemProperty =
+			DependencyProperty.Register(nameof(Item), typeof(ISidebarItemModel), typeof(SidebarItem), new PropertyMetadata(null));
+
 		public bool UseReorderDrop
 		{
 			get { return (bool)GetValue(UseReorderDropProperty); }
@@ -71,52 +94,16 @@ public SidebarDisplayMode DisplayMode
 		[GeneratedDependencyProperty]
 		public partial object? ToolTip { get; set; }
 
-		[GeneratedDependencyProperty]
-		public partial bool IsPaddedItem { get; set; }
-
-		[GeneratedDependencyProperty]
-		public partial object? Children { get; set; }
-
-		[GeneratedDependencyProperty, Obsolete]
-		public partial object? Item { get; set; }
-
-		[GeneratedDependencyProperty]
-		public partial SidebarView? Owner { get; set; }
-
-		[GeneratedDependencyProperty]
-		public partial bool IsInFlyout { get; set; }
-
-		partial void OnChildrenPropertyChanged(DependencyPropertyChangedEventArgs e)
+		public static void SetTemplateRoot(DependencyObject target, FrameworkElement value)
 		{
-			if (e.OldValue is INotifyCollectionChanged oldNCC)
-				oldNCC.CollectionChanged -= ChildItems_CollectionChanged;
-			if (e.NewValue is INotifyCollectionChanged newNCC)
-				newNCC.CollectionChanged += ChildItems_CollectionChanged;
+			target.SetValue(TemplateRootProperty, value);
 		}
-
-		partial void OnOwnerPropertyChanged(DependencyPropertyChangedEventArgs e)
-		{
-			if (Owner is null)
-				return;
-
-			DisplayMode = Owner.DisplayMode;
-
-			Owner.RegisterPropertyChangedCallback(SidebarView.DisplayModeProperty, (sender, args) =>
-			{
-				DisplayMode = Owner.DisplayMode;
-			});
-			Owner.RegisterPropertyChangedCallback(SidebarView.SelectedItemProperty, (sender, args) =>
-			{
-				ReevaluateSelection();
-			});
-
-			ReevaluateSelection();
-		}
-
-		partial void OnIsInFlyoutPropertyChanged(DependencyPropertyChangedEventArgs e)
+		public static FrameworkElement GetTemplateRoot(DependencyObject target)
 		{
-			VisualStateManager.GoToState(this, DisplayMode is SidebarDisplayMode.Compact && !IsInFlyout ? "Compact" : "NonCompact", true);
+			return (FrameworkElement)target.GetValue(TemplateRootProperty);
 		}
+		public static readonly DependencyProperty TemplateRootProperty =
+			DependencyProperty.Register("TemplateRoot", typeof(FrameworkElement), typeof(FrameworkElement), new PropertyMetadata(null));
 
 		public static void OnPropertyChanged(DependencyObject sender, DependencyPropertyChangedEventArgs e)
 		{

src/Files.App.Controls/Sidebar/SidebarItem.cs

@@ -5,7 +5,6 @@
 using Microsoft.UI.Input;
 using Microsoft.UI.Xaml.Automation;
 using Microsoft.UI.Xaml.Automation.Peers;
-using Microsoft.UI.Xaml.Media;
 using System.Collections.Specialized;
 using Windows.ApplicationModel.DataTransfer;
 
@@ -15,15 +14,16 @@ public sealed partial class SidebarItem : Control
 	{
 		private const double DROP_REPOSITION_THRESHOLD = 0.2; // Percentage of top/bottom at which we consider a drop to be a reposition/insertion
 
-		public bool HasChildren => Children is IList enumerable && enumerable.Count > 0;
-		public bool IsGroupHeader => Children is not null;
+		public bool HasChildren => Item?.Children is IList enumerable && enumerable.Count > 0;
+		public bool IsGroupHeader => Item?.Children is not null;
 		public bool CollapseEnabled => DisplayMode != SidebarDisplayMode.Compact;
 
 		private bool hasChildSelection => selectedChildItem != null;
 		private bool isPointerOver = false;
 		private bool isClicking = false;
 		private object? selectedChildItem = null;
 		private ItemsRepeater? childrenRepeater;
+		private ISidebarItemModel? lastSubscriber;
 
 		public SidebarItem()
 		{
@@ -50,13 +50,13 @@ protected override AutomationPeer OnCreateAutomationPeer()
 
 		internal void Select()
 		{
-			Owner?.SelectedItem = Item!;
+			if (Owner is not null)
+				Owner.SelectedItem = Item!;
 		}
 
 		private void SidebarItem_Loaded(object sender, RoutedEventArgs e)
 		{
-			if (!IsInFlyout)
-				Owner = this.FindAscendant<SidebarView>()!;
+			HookupOwners();
 
 			if (GetTemplateChild("ElementBorder") is Border border)
 			{
@@ -82,38 +82,56 @@ private void SidebarItem_Loaded(object sender, RoutedEventArgs e)
 				flyoutRepeater.ElementPrepared += ChildrenPresenter_ElementPrepared;
 			}
 
-			if (GetTemplateChild("ChildrenFlyout") is Flyout flyout)
-			{
-				flyout.Opened += ChildrenFlyout_Opened;
-			}
-
 			HandleItemChange();
 		}
 
 		public void HandleItemChange()
 		{
+			HookupItemChangeListener(null, Item);
 			UpdateExpansionState();
 			ReevaluateSelection();
 		}
 
-		private void ChildrenFlyout_Opened(object? sender, object e)
+		private void HookupOwners()
 		{
-			if (sender is not Flyout)
-				return;
+			FrameworkElement resolvingTarget = this;
+			if (GetTemplateRoot(Parent) is FrameworkElement element)
+			{
+				resolvingTarget = element;
+			}
+			Owner = resolvingTarget.FindAscendant<SidebarView>()!;
 
-			var popup = VisualTreeHelper.GetOpenPopupsForXamlRoot(XamlRoot).ToList().FirstOrDefault();
-			var itemsRepeater = popup?.Child.FindDescendant<ItemsRepeater>();
-			if (itemsRepeater is null)
-				return;
+			Owner.RegisterPropertyChangedCallback(SidebarView.DisplayModeProperty, (sender, args) =>
+			{
+				DisplayMode = Owner.DisplayMode;
+			});
+			DisplayMode = Owner.DisplayMode;
 
-			var count = itemsRepeater.ItemsSourceView.Count;
-			for (int i = 0; i < count; i++)
+			Owner.RegisterPropertyChangedCallback(SidebarView.SelectedItemProperty, (sender, args) =>
 			{
-				if (itemsRepeater.GetOrCreateElement(i).FindDescendantOrSelf<SidebarItem>() is not { } sidebarItem)
-					continue;
+				ReevaluateSelection();
+			});
+			ReevaluateSelection();
+		}
 
-				sidebarItem.Owner = Owner;
-				sidebarItem.IsInFlyout = true;
+		private void HookupItemChangeListener(ISidebarItemModel? oldItem, ISidebarItemModel? newItem)
+		{
+			if (lastSubscriber != null)
+			{
+				if (lastSubscriber.Children is INotifyCollectionChanged observableCollection)
+					observableCollection.CollectionChanged -= ChildItems_CollectionChanged;
+			}
+
+			if (oldItem != null)
+			{
+				if (oldItem.Children is INotifyCollectionChanged observableCollection)
+					observableCollection.CollectionChanged -= ChildItems_CollectionChanged;
+			}
+			if (newItem != null)
+			{
+				lastSubscriber = newItem;
+				if (newItem.Children is INotifyCollectionChanged observableCollection)
+					observableCollection.CollectionChanged += ChildItems_CollectionChanged;
 			}
 		}
 
@@ -124,7 +142,7 @@ private void SidebarItem_DragStarting(UIElement sender, DragStartingEventArgs ar
 
 		private void SetFlyoutOpen(bool isOpen = true)
 		{
-			if (Children is null) return;
+			if (Item?.Children is null) return;
 
 			var flyoutOwner = (GetTemplateChild("ElementGrid") as FrameworkElement)!;
 			if (isOpen)
@@ -157,7 +175,7 @@ private void ReevaluateSelection()
 					Owner?.UpdateSelectedItemContainer(this);
 				}
 			}
-			else if (Children is IList list)
+			else if (Item?.Children is IList list)
 			{
 				if (list.Contains(Owner?.SelectedItem))
 				{
@@ -176,7 +194,7 @@ private void ChildrenPresenter_ElementPrepared(ItemsRepeater sender, ItemsRepeat
 		{
 			if (args.Element is SidebarItem item)
 			{
-				if (Children is IList enumerable)
+				if (Item?.Children is IList enumerable)
 				{
 					var newElement = enumerable[args.Index];
 					if (newElement == selectedChildItem)
@@ -275,18 +293,20 @@ private void UpdatePointerState(bool isPointerDown = false)
 
 		private void UpdateExpansionState(bool useAnimations = true)
 		{
-			if (Children is null || !CollapseEnabled)
+			if (Item?.Children is null || !CollapseEnabled)
 			{
-				VisualStateManager.GoToState(this, IsPaddedItem ? "NoExpansionWithPadding" : "NoExpansion", useAnimations);
+				VisualStateManager.GoToState(this, Item?.PaddedItem == true ? "NoExpansionWithPadding" : "NoExpansion", useAnimations);
 			}
 			else if (!HasChildren)
 			{
 				VisualStateManager.GoToState(this, "NoChildren", useAnimations);
 			}
 			else
 			{
-				if (Children is IList { Count: > 0 } enumerable && childrenRepeater?.ItemsSourceView.Count > 0 && childrenRepeater?.GetOrCreateElement(0) is UIElement firstChild)
+				if (Item?.Children is IList enumerable && enumerable.Count > 0 && childrenRepeater is not null)
 				{
+					var firstChild = childrenRepeater.GetOrCreateElement(0);
+
 					// Collapsed elements might have a desired size of 0 so we need to have a sensible fallback
 					var childHeight = firstChild.DesiredSize.Height > 0 ? firstChild.DesiredSize.Height : 32;
 					ChildrenPresenterHeight = enumerable.Count * childHeight;

src/Files.App.Controls/Sidebar/SidebarItemAutomationPeer.cs

@@ -103,7 +103,7 @@ protected override int GetPositionInSetCore()
 
 		private IList GetOwnerCollection()
 		{
-			if (Owner.FindAscendant<SidebarItem>() is SidebarItem parent && parent.Children is IList list)
+			if (Owner.FindAscendant<SidebarItem>() is SidebarItem parent && parent.Item?.Children is IList list)
 				return list;
 
 			if (Owner?.Owner is not null && Owner.Owner?.MenuItemsSource is IList items)

src/Files.App.Controls/Sidebar/SidebarStyles.xaml

@@ -39,11 +39,9 @@
 	<DataTemplate x:Key="DefaultSidebarItemTemplate">
 		<local:SidebarItem
 			AutomationProperties.AutomationId="{Binding Text}"
-			Children="{Binding Children}"
 			Decorator="{Binding ItemDecorator}"
 			Icon="{Binding IconElement}"
 			IsExpanded="{Binding IsExpanded, Mode=TwoWay}"
-			IsPaddedItem="{Binding PaddedItem}"
 			Item="{Binding}"
 			Text="{Binding Text}"
 			ToolTip="{Binding ToolTip}" />
@@ -195,10 +193,24 @@
 										<ItemsRepeater
 											x:Name="FlyoutChildrenPresenter"
 											HorizontalAlignment="Stretch"
+											local:SidebarItem.TemplateRoot="{Binding ElementName=RootPanel}"
 											AutomationProperties.AccessibilityView="Content"
-											ItemTemplate="{StaticResource DefaultSidebarItemTemplate}"
-											ItemsSource="{TemplateBinding Children}"
-											XYFocusKeyboardNavigation="Enabled" />
+											ItemsSource="{Binding Children, Mode=OneWay}"
+											XYFocusKeyboardNavigation="Enabled">
+											<ItemsRepeater.ItemTemplate>
+												<DataTemplate>
+													<local:SidebarItem
+														AutomationProperties.AutomationId="{Binding Text}"
+														Decorator="{Binding ItemDecorator}"
+														Icon="{Binding IconElement}"
+														IsExpanded="{Binding IsExpanded, Mode=TwoWay}"
+														IsInFlyout="True"
+														Item="{Binding}"
+														Text="{Binding Text}"
+														ToolTip="{Binding ToolTip}" />
+												</DataTemplate>
+											</ItemsRepeater.ItemTemplate>
+										</ItemsRepeater>
 									</Flyout>
 								</FlyoutBase.AttachedFlyout>
 							</Grid>
@@ -210,9 +222,10 @@
 							Grid.Row="2"
 							Margin="-4,2"
 							HorizontalAlignment="Stretch"
+							local:SidebarItem.TemplateRoot="{Binding ElementName=RootPanel}"
 							AutomationProperties.AccessibilityView="Content"
 							ItemTemplate="{StaticResource DefaultSidebarItemTemplate}"
-							ItemsSource="{TemplateBinding Children}"
+							ItemsSource="{Binding Children, Mode=OneWay}"
 							Visibility="Collapsed"
 							XYFocusKeyboardNavigation="Enabled">
 							<ItemsRepeater.Layout>

src/Files.App.Controls/Sidebar/SidebarView.Properties.cs

@@ -1,8 +1,6 @@
 // Copyright (c) Files Community
 // Licensed under the MIT License.
 
-using CommunityToolkit.WinUI;
-
 namespace Files.App.Controls
 {
 	public sealed partial class SidebarView
@@ -67,13 +65,16 @@ public double NegativeOpenPaneLength
 		public static readonly DependencyProperty NegativeOpenPaneLengthProperty =
 			DependencyProperty.Register(nameof(NegativeOpenPaneLength), typeof(double), typeof(SidebarView), new PropertyMetadata(null));
 
-		public object SelectedItem
+		public ISidebarItemModel SelectedItem
 		{
-			get => GetValue(SelectedItemProperty);
-			set => SetValue(SelectedItemProperty, value);
+			get => (ISidebarItemModel)GetValue(SelectedItemProperty);
+			set
+			{
+				SetValue(SelectedItemProperty, value);
+			}
 		}
 		public static readonly DependencyProperty SelectedItemProperty =
-			DependencyProperty.Register(nameof(SelectedItem), typeof(object), typeof(SidebarView), new PropertyMetadata(null));
+			DependencyProperty.Register(nameof(SelectedItem), typeof(ISidebarItemModel), typeof(SidebarView), new PropertyMetadata(null));
 
 		public object MenuItemsSource
 		{