[SettingsCard] Accesibility improvements (#284)

* Push * Code cleanup * Updating samples * Update SettingsCard.xaml * Minor cleanup * Fix initial render bug * A11y improvements for SettingsCard * Updates to SettingsExpander narrator announcements * Update hover color in highcontrast * Adding click event to Clickable card sample --------- Co-authored-by: Arlo Godfrey <[email protected]>
CommunityToolkit · Jan 9, 2024 · 8285f8d · 8285f8d
1 parent eb98b48
commit 8285f8d
Show file tree

Hide file tree

Showing 6 changed files with 86 additions and 45 deletions.
diff --git a/components/SettingsControls/samples/ClickableSettingsCardSample.xaml b/components/SettingsControls/samples/ClickableSettingsCardSample.xaml
@@ -1,4 +1,4 @@
-<!--  Licensed to the .NET Foundation under one or more agreements. The .NET Foundation licenses this file to you under the MIT license. See the LICENSE file in the project root for more information.  -->
+<!--  Licensed to the .NET Foundation under one or more agreements. The .NET Foundation licenses this file to you under the MIT license. See the LICENSE file in the project root for more information.  -->
 <Page x:Class="SettingsControlsExperiment.Samples.ClickableSettingsCardSample"
       xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
       xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
@@ -9,6 +9,7 @@
       mc:Ignorable="d">
     <StackPanel Spacing="4">
         <controls:SettingsCard x:Name="settingsCard"
+                               Click="OnCardClicked"
                                Description="A SettingsCard can be made clickable and you can leverage the Command property or Click event."
                                Header="A clickable SettingsCard"
                                HeaderIcon="{ui:FontIcon Glyph=&#xE799;}"
@@ -20,13 +21,15 @@
 
         <controls:SettingsCard ActionIcon="{ui:FontIcon Glyph=&#xE8A7;}"
                                ActionIconToolTip="Open in new window"
+                               Click="OnCardClicked"
                                Description="You can customize the ActionIcon and ActionIconToolTip."
                                Header="Customizing the ActionIcon"
                                HeaderIcon="{ui:FontIcon Glyph=&#xE774;}"
                                IsClickEnabled="True"
                                IsEnabled="{x:Bind IsCardEnabled, Mode=OneWay}" />
 
-        <controls:SettingsCard Header="Hiding the ActionIcon"
+        <controls:SettingsCard Click="OnCardClicked"
+                               Header="Hiding the ActionIcon"
                                HeaderIcon="{ui:FontIcon Glyph=&#xE72E;}"
                                IsActionIconVisible="False"
                                IsClickEnabled="True"

diff --git a/components/SettingsControls/src/SettingsCard/SettingsCard.cs b/components/SettingsControls/src/SettingsCard/SettingsCard.cs
@@ -74,8 +74,7 @@ protected override void OnApplyTemplate()
         OnDescriptionChanged();
         OnIsClickEnabledChanged();
         CheckInitialVisualState();
-
-        RegisterAutomation();
+        SetAccessibleContentName();
         RegisterPropertyChangedCallback(ContentProperty, OnContentChanged);
         IsEnabledChanged += OnIsEnabledChanged;     
     }
@@ -91,11 +90,12 @@ private void CheckInitialVisualState()
             contentAlignmentStatesGroup.CurrentStateChanged += this.ContentAlignmentStates_Changed;
         }
     }
-    private void RegisterAutomation()
+
+    // We automatically set the AutomationProperties.Name of the Content if not configured.
+    private void SetAccessibleContentName()
     {
         if (Header is string headerString && headerString != string.Empty)
         {
-            AutomationProperties.SetName(this, headerString);
             // We don't want to override an AutomationProperties.Name that is manually set, or if the Content basetype is of type ButtonBase (the ButtonBase.Content will be used then)
             if (Content is UIElement element && string.IsNullOrEmpty(AutomationProperties.GetName(element)) && element.GetType().BaseType != typeof(ButtonBase) && element.GetType() != typeof(TextBlock))
             {

diff --git a/components/SettingsControls/src/SettingsCard/SettingsCard.xaml b/components/SettingsControls/src/SettingsCard/SettingsCard.xaml
@@ -80,7 +80,7 @@
             <StaticResource x:Key="SettingsCardForegroundPointerOver"
                             ResourceKey="SystemColorHighlightColorBrush" />
             <StaticResource x:Key="SettingsCardForegroundPressed"
-                            ResourceKey="SystemColorHighlightTextColorBrush" />
+                            ResourceKey="SystemColorHighlightColorBrush" />
             <StaticResource x:Key="SettingsCardForegroundDisabled"
                             ResourceKey="SystemControlDisabledBaseMediumLowBrush" />
 

diff --git a/components/SettingsControls/src/SettingsCard/SettingsCardAutomationPeer.cs b/components/SettingsControls/src/SettingsCard/SettingsCardAutomationPeer.cs
@@ -2,46 +2,67 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-using System;
-using System.Collections.Generic;
-using System.Text;
-
 namespace CommunityToolkit.WinUI.Controls;
 
+/// <summary>
+/// AutomationPeer for SettingsCard
+/// </summary>
+public class SettingsCardAutomationPeer : FrameworkElementAutomationPeer
+{
+    /// <summary>
+    /// Initializes a new instance of the <see cref="SettingsCard"/> class.
+    /// </summary>
+    /// <param name="owner">SettingsCard</param>
+    public SettingsCardAutomationPeer(SettingsCard owner)
+        : base(owner)
+    {
+    }
+
     /// <summary>
-    /// AutomationPeer for SettingsCard
+    /// Gets the control type for the element that is associated with the UI Automation peer.
     /// </summary>
-    public class SettingsCardAutomationPeer : FrameworkElementAutomationPeer
+    /// <returns>The control type.</returns>
+    protected override AutomationControlType GetAutomationControlTypeCore()
     {
-        /// <summary>
-        /// Initializes a new instance of the <see cref="SettingsCard"/> class.
-        /// </summary>
-        /// <param name="owner">SettingsCard</param>
-        public SettingsCardAutomationPeer(SettingsCard owner)
-            : base(owner)
+        if (Owner is SettingsCard settingsCard && settingsCard.IsClickEnabled)
         {
+            return AutomationControlType.Button;
         }
-
-        /// <summary>
-        /// Gets the control type for the element that is associated with the UI Automation peer.
-        /// </summary>
-        /// <returns>The control type.</returns>
-        protected override AutomationControlType GetAutomationControlTypeCore()
+        else
         {
             return AutomationControlType.Group;
         }
+    }
 
-        /// <summary>
-        /// Called by GetClassName that gets a human readable name that, in addition to AutomationControlType,
-        /// differentiates the control represented by this AutomationPeer.
-        /// </summary>
-        /// <returns>The string that contains the name.</returns>
-        protected override string GetClassNameCore()
+    /// <summary>
+    /// Called by GetClassName that gets a human readable name that, in addition to AutomationControlType,
+    /// differentiates the control represented by this AutomationPeer.
+    /// </summary>
+    /// <returns>The string that contains the name.</returns>
+    protected override string GetClassNameCore()
+    {
+        return Owner.GetType().Name;
+    }
+
+    protected override string GetNameCore()
+    {
+        // We only want to announce the button card name if it is clickable, else it's just a regular card that does not receive focus
+        if (Owner is SettingsCard owner && owner.IsClickEnabled)
         {
-            string classNameCore = Owner.GetType().Name;
-#if DEBUG_AUTOMATION
-            System.Diagnostics.Debug.WriteLine("SettingsCardAutomationPeer.GetClassNameCore returns " + classNameCore);
-#endif
-            return classNameCore;
+            string name = AutomationProperties.GetName(owner);
+            if (!string.IsNullOrEmpty(name))
+            {
+                return name;
+            }
+            else
+            {
+                if (owner.Header is string headerString && !string.IsNullOrEmpty(headerString))
+                {
+                    return headerString;
+                }
+            }
         }
+
+        return base.GetNameCore();
     }
+}
diff --git a/components/SettingsControls/src/SettingsExpander/SettingsExpander.cs b/components/SettingsControls/src/SettingsExpander/SettingsExpander.cs
@@ -25,7 +25,7 @@ public SettingsExpander()
     protected override void OnApplyTemplate()
     {
         base.OnApplyTemplate();
-        RegisterAutomation();
+        SetAccessibleName();
 
         if (_itemsRepeater != null)
         {
@@ -43,11 +43,11 @@ protected override void OnApplyTemplate()
         }
     }
 
-    private void RegisterAutomation()
+    private void SetAccessibleName()
     {
-        if (Header is string headerString && headerString != string.Empty)
+        if (string.IsNullOrEmpty(AutomationProperties.GetName(this)))
         {
-            if (!string.IsNullOrEmpty(headerString) && string.IsNullOrEmpty(AutomationProperties.GetName(this)))
+            if (Header is string headerString && !string.IsNullOrEmpty(headerString))
             {
                 AutomationProperties.SetName(this, headerString);
             }

diff --git a/components/SettingsControls/src/SettingsExpander/SettingsExpanderAutomationPeer.cs b/components/SettingsControls/src/SettingsExpander/SettingsExpanderAutomationPeer.cs
@@ -39,11 +39,28 @@ protected override AutomationControlType GetAutomationControlTypeCore()
     /// <returns>The string that contains the name.</returns>
     protected override string GetClassNameCore()
     {
-        string classNameCore = Owner.GetType().Name;
-#if DEBUG_AUTOMATION
-            System.Diagnostics.Debug.WriteLine("SettingsCardAutomationPeer.GetClassNameCore returns " + classNameCore);
-#endif
-        return classNameCore;
+        return Owner.GetType().Name;
+    }
+
+    protected override string GetNameCore()
+    {
+        string name = base.GetNameCore();
+
+        if (Owner is SettingsExpander owner)
+        {
+            if (!string.IsNullOrEmpty(AutomationProperties.GetName(owner)))
+            {
+                name = AutomationProperties.GetName(owner);
+            }
+            else
+            {
+                if (owner.Header is string headerString && !string.IsNullOrEmpty(headerString))
+                {
+                    name = headerString;
+                }
+            }
+        }
+        return name;
     }
 
     /// <summary>