pr comments

components/Ribbon/samples/Ribbon.md

@@ -25,11 +25,6 @@ some of them can be collapsed based on a priority order.
 A basic group displayed in a Ribbon.
 It mostly adds a *label* to some content and will not collapse if there is not enough space available.
 
-### Label
-The label of the group.
-
-### Content
-The content of the group.
 
 ## RibbonCollapsibleGroup
 
@@ -40,7 +35,7 @@ a flyout containing the group's content.
 ### IconSource
 The icon to display when the group is collapsed.
 
-### AutoCloseflyout
+### AutoCloseFlyout
 Set to true to automatically close the overflow flyout if one interactive element is clicked.
 Note that the logic to detect the click is very basic. It will request the flyout to close
 for all the handled pointer released events. It assumes that if the pointer has been handled

components/Ribbon/samples/RibbonCustomSample.xaml

@@ -22,7 +22,7 @@
                    TargetType="AppBarButton">
                 <Setter Property="LabelPosition" Value="Collapsed" />
                 <Setter Property="Width" Value="48" />
-                <Setter Property="Margin" Value="8" />
+                <Setter Property="Margin" Value="4" />
                 <Setter Property="Height" Value="48" />
             </Style>
         </StackPanel.Resources>
@@ -151,16 +151,21 @@
             </controls:RibbonCollapsibleGroup>
 
             <controls:RibbonCollapsibleGroup CollapsedAccessKey="G"
-                                             Label="Advanved"
+                                             Label="Advanced"
                                              Priority="5"
                                              RequestedWidths="400,200,300">
                 <controls:RibbonCollapsibleGroup.IconSource>
                     <SymbolIconSource Symbol="AllApps" />
                 </controls:RibbonCollapsibleGroup.IconSource>
-                <GridView MinHeight="72"
+                <GridView Height="96"
                           MaxWidth="400"
+                          HorizontalAlignment="Stretch"
+                          VerticalAlignment="Center"
                           BorderBrush="{ThemeResource CardStrokeColorDefaultBrush}"
-                          BorderThickness="1">
+                          BorderThickness="1"
+                          ScrollViewer.HorizontalScrollBarVisibility="Disabled"
+                          ScrollViewer.HorizontalScrollMode="Disabled"
+                          ScrollViewer.VerticalScrollBarVisibility="Auto">
                     <GridView.ItemTemplate>
                         <DataTemplate>
                             <TextBlock Width="24"

components/Ribbon/src/Ribbon.cs

@@ -63,7 +63,7 @@ public Ribbon()
         _items.CollectionChanged += OnItemsCollectionChanged;
     }
 
-    public ICollection<UIElement> Items => _items;
+    public IList<UIElement> Items => _items;
 
     protected override void OnApplyTemplate()
     {

components/Ribbon/src/RibbonCollapsibleGroup.cs

@@ -171,22 +171,26 @@ protected override void OnApplyTemplate()
             _collapsedContentContainer.RemoveHandler(KeyUpEvent, new KeyEventHandler(OnFlyoutKeyUp));
         }
 
-        _visibleContentContainer = GetOrThrow<ContentControl>(VisibleContentContainerTemplatePart);
-        _collapsedContentContainer = GetOrThrow<ContentControl>(CollapsedContentPresenterTemplatePart);
-        _collapsedButton = GetOrThrow<Button>(CollapsedButtonTemplatePart);
-        _collapsedFlyout = GetOrThrow<Flyout>(CollapsedFlyoutTemplatePart);
+        _visibleContentContainer = Get<ContentControl>(VisibleContentContainerTemplatePart);
+        _collapsedContentContainer = Get<ContentControl>(CollapsedContentPresenterTemplatePart);
+        _collapsedButton = Get<Button>(CollapsedButtonTemplatePart);
+        _collapsedFlyout = Get<Flyout>(CollapsedFlyoutTemplatePart);
 
-        _collapsedFlyout.Opened += OnFlyoutOpened;
-        _collapsedContentContainer.AddHandler(PointerReleasedEvent, new PointerEventHandler(OnFlyoutPointerReleased), handledEventsToo: true);
-        _collapsedContentContainer.AddHandler(KeyUpEvent, new KeyEventHandler(OnFlyoutKeyUp), handledEventsToo: true);
+        if (_collapsedFlyout is not null)
+        {
+            _collapsedFlyout.Opened += OnFlyoutOpened;
+        }
+
+        if (_collapsedContentContainer is not null)
+        {
+            _collapsedContentContainer.AddHandler(PointerReleasedEvent, new PointerEventHandler(OnFlyoutPointerReleased), handledEventsToo: true);
+            _collapsedContentContainer.AddHandler(KeyUpEvent, new KeyEventHandler(OnFlyoutKeyUp), handledEventsToo: true);
+        }
 
         UpdateState();
     }
 
-    private T GetOrThrow<T>(string templatePart) where T : class
-        => GetTemplateChild(templatePart) is T element
-            ? element
-            : throw new ArgumentException($"{templatePart} missing or not of the expected type: {typeof(T).Name}");
+    private T? Get<T>(string templatePart) where T : class => GetTemplateChild(templatePart) as T;
 
     private void OnFlyoutOpened(object? sender, object e)
         => _collapsedContentContainer?.Focus(FocusState.Programmatic);
@@ -209,18 +213,23 @@ private void AutoCollapseFlyout(bool eventHasBeenHandled, object originalSource)
         // We only consider events which have been processed since it usually means
         // that a control has processed the event and that the click is not in an
         // empty/non-interactive area.
-        if (eventHasBeenHandled && AutoCloseFlyout && _collapsedFlyout!.IsOpen && !DoesRoutedEventOriginateFromAFlyoutHost(originalSource as UIElement))
+        if (eventHasBeenHandled && AutoCloseFlyout && _collapsedFlyout?.IsOpen == true && !DoesRoutedEventOriginateFromAFlyoutHost(originalSource as UIElement))
         {
             _collapsedFlyout.Hide();
         }
     }
 
     private bool DoesRoutedEventOriginateFromAFlyoutHost(UIElement? source)
     {
+        if (_collapsedContentContainer is null)
+        {
+            return false;
+        }
+
         while (source != null && source != _collapsedContentContainer)
         {
-            // TODO: handle MUX variants in UWP
-            if (source is DropDownButton ||
+            if (source is MUXC.DropDownButton ||
+                source is DropDownButton ||
                 source is ComboBox ||
                 source is ComboBoxItem ||
                 (source is Button buttonSource && buttonSource.Flyout != null) ||
@@ -243,28 +252,35 @@ private static void OnStatePropertyChanged(DependencyObject d, DependencyPropert
 
     private void UpdateState()
     {
-        if (_visibleContentContainer is null)
-        {
-            // Template is not ready yet.
-            return;
-        }
-
         switch (State)
         {
             case Visibility.Visible:
-                _collapsedFlyout!.Hide();
-                _collapsedContentContainer!.Content = null;
-                _visibleContentContainer.Content = Content;
-
-                _collapsedButton!.Visibility = Visibility.Collapsed;
-                _visibleContentContainer.Visibility = Visibility.Visible;
+                _collapsedFlyout?.Hide();
+
+                if (_collapsedContentContainer is not null && _visibleContentContainer is not null)
+                {
+                    _collapsedContentContainer.Content = null;
+                    _visibleContentContainer.Content = Content;
+                }
+
+                if (_collapsedButton is not null && _visibleContentContainer is not null)
+                {
+                    _collapsedButton.Visibility = Visibility.Collapsed;
+                    _visibleContentContainer.Visibility = Visibility.Visible;
+                }
                 break;
             case Visibility.Collapsed:
-                _visibleContentContainer.Content = null;
-                _collapsedContentContainer!.Content = Content;
-
-                _visibleContentContainer.Visibility = Visibility.Collapsed;
-                _collapsedButton!.Visibility = Visibility.Visible;
+                if (_collapsedContentContainer is not null && _visibleContentContainer is not null)
+                {
+                    _visibleContentContainer.Content = null;
+                    _collapsedContentContainer.Content = Content;
+                }
+
+                if (_collapsedButton is not null && _visibleContentContainer is not null)
+                {
+                    _visibleContentContainer.Visibility = Visibility.Collapsed;
+                    _collapsedButton.Visibility = Visibility.Visible;
+                }
                 break;
             default:
                 throw new ArgumentException("Invalid state");