[NavMenu] Fix keyboard navigation (#1950)

* [FluentNavMenu] Fix keyboard navigation * [FluentNavMenu] Fix tests --------- Co-authored-by: Alessio Dell Aquila <[email protected]> Co-authored-by: Vincent Baaij <[email protected]>
microsoft · Apr 29, 2024 · a9484d6 · a9484d6
1 parent 118cd7f
commit a9484d6
Show file tree

Hide file tree

Showing 7 changed files with 135 additions and 44 deletions.
diff --git a/src/Core/Components/NavMenu/FluentNavGroup.razor b/src/Core/Components/NavMenu/FluentNavGroup.razor
@@ -6,7 +6,7 @@
 
 @if (Owner.Expanded || HasIcon)
 {
-    <FluentKeyCode Anchor="@Id" OnKeyDown="@HandleExpanderKeyDownAsync" />
+    <FluentKeyCode Anchor="@Id" PreventDefaultOnly="new[] { KeyCode.Enter, KeyCode.Left, KeyCode.Right }" StopPropagation="@true" OnKeyDown="@HandleExpanderKeyDownAsync" />
     <div id="@Id" @ref="@Element" @attributes="AdditionalAttributes" class="@ClassValue" disabled="@Disabled" style="@StyleValue" role="menuitem">
 
         @if (!string.IsNullOrEmpty(Href))
@@ -26,8 +26,8 @@
         }
         else
         {
-            <div class="fluent-nav-link notactive" tabindex="0">
-                <div class="positioning-region" @onclick="ToggleExpandedAsync"  title="@(Tooltip ?? Title)">
+            <div class="fluent-nav-link notactive" tabindex="@(Disabled ? "-1" : "0")">
+                <div class="positioning-region" @onclick="ToggleExpandedAsync" title="@(Tooltip ?? Title)">
                     <div class="content-region">
                         @_renderContent
                         @_renderButton
@@ -37,13 +37,51 @@
         }
 
         <FluentCollapsibleRegion @bind-Expanded="@Expanded" MaxHeight="@MaxHeight" Class="items" role="group">
-            <FluentNavMenu>
+            <FluentNavMenu Data="_data">
                 @ChildContent
             </FluentNavMenu>
         </FluentCollapsibleRegion>
     </div>
+
+    if (!Owner.Expanded && Owner.CollapsedChildNavigation && SubMenu == null)
+    {
+        <FluentMenu @bind-Open="_open"
+                    Anchor="@Id"
+                    HorizontalPosition="@HorizontalPosition.End"
+                    HorizontalInset="@false"
+                    VerticalPosition="@VerticalPosition.Bottom"
+                    VerticalInset="@true">
+            @ChildContent
+        </FluentMenu>
+    }
+    else
+    {
+        _open = false;
+    }
 }
+else if (!Owner.Expanded && Owner.CollapsedChildNavigation && SubMenu != null)
+{
+    <FluentMenuItem Disabled="@Disabled" OnClick="OnClickHandlerAsync">
+        <ChildContent>
+            @Title
+            @TitleTemplate
+
+            @if (Icon is not null)
+            {
+                <span slot="start">
+                    <FluentIcon Value="@Icon" Width="20px" Color="@IconColor" Class="fluent-nav-icon" />
+                </span>
+            }
+        </ChildContent>
+        <MenuItems>
+            @ChildContent
+        </MenuItems>
+    </FluentMenuItem>
+}
+
+
 @code {
+    private string _data = "no-pagescript";
     private void RenderContent(RenderTreeBuilder __builder)
     {
         @if (Icon is not null)
@@ -72,7 +110,6 @@
                  @onclick:preventDefault="true">
                 <FluentIcon Value="@ExpandIcon" Color="@(Disabled ? Color.Disabled : Color.Neutral)" Class="@ExpandIconClassValue" />
             </div>
-
         }
     }
 }
diff --git a/src/Core/Components/NavMenu/FluentNavGroup.razor.cs b/src/Core/Components/NavMenu/FluentNavGroup.razor.cs
@@ -1,4 +1,8 @@
-using Microsoft.AspNetCore.Components;
+// ------------------------------------------------------------------------
+// MIT License - Copyright (c) Microsoft Corporation. All rights reserved.
+// ------------------------------------------------------------------------
+
+using Microsoft.AspNetCore.Components;
 using Microsoft.AspNetCore.Components.Web;
 using Microsoft.Fast.Components.FluentUI.Utilities;
 
@@ -8,6 +12,7 @@ public partial class FluentNavGroup : FluentNavBase
 {
     private readonly RenderFragment _renderContent;
     private readonly RenderFragment _renderButton;
+    private bool _open;
 
     protected string? ClassValue =>
         new CssBuilder("fluent-nav-group")
@@ -21,13 +26,12 @@ public partial class FluentNavGroup : FluentNavBase
        .AddStyle("margin", $"{Gap} 0", !string.IsNullOrEmpty(Gap))
        .Build();
 
-
     protected string? ButtonClassValue =>
         new CssBuilder("expand-collapse-button")
             .AddClass("rotate", Expanded)
             .Build();
 
-    protected string? ExpandIconClassValue =>
+    protected static string? ExpandIconClassValue =>
         new CssBuilder("fluent-nav-expand-icon")
             .Build();
 
@@ -99,7 +103,17 @@ public FluentNavGroup()
         _renderButton = RenderButton;
     }
 
-    private Task ToggleExpandedAsync() => SetExpandedAsync(!Expanded);
+    private Task ToggleExpandedAsync()
+    {
+        if (!Owner.Expanded && Owner.CollapsedChildNavigation)
+        {
+            return SetExpandedAsync(!_open);
+        }
+        else
+        {
+            return SetExpandedAsync(!Expanded);
+        }
+    }
 
     private async Task HandleExpanderKeyDownAsync(FluentKeyCodeEventArgs args)
     {
@@ -109,7 +123,7 @@ private async Task HandleExpanderKeyDownAsync(FluentKeyCodeEventArgs args)
         }
         Task handler = args.Key switch
         {
-            KeyCode.Enter => SetExpandedAsync(!Expanded),
+            KeyCode.Enter => ToggleExpandedAsync(),
             KeyCode.Right => SetExpandedAsync(true),
             KeyCode.Left => SetExpandedAsync(false),
             _ => Task.CompletedTask
@@ -119,17 +133,24 @@ private async Task HandleExpanderKeyDownAsync(FluentKeyCodeEventArgs args)
 
     private async Task SetExpandedAsync(bool value)
     {
-        if (value == Expanded)
-        {
-            return;
-        }
-
         if (!Owner.Expanded)
         {
-            await Owner.ExpandedChanged.InvokeAsync(true);
+            if (Owner.CollapsedChildNavigation)
+            {
+                _open = value;
+            }
+            else
+            {
+                await Owner.ExpandedChanged.InvokeAsync(true);
+            }
         }
         else
         {
+            if (value == Expanded)
+            {
+                return;
+            }
+
             Expanded = value;
 
             if (ExpandedChanged.HasDelegate)
@@ -138,4 +159,4 @@ private async Task SetExpandedAsync(bool value)
             }
         }
     }
-}
+}
diff --git a/src/Core/Components/NavMenu/FluentNavGroup.razor.css b/src/Core/Components/NavMenu/FluentNavGroup.razor.css
@@ -56,3 +56,7 @@
 ::deep .rotate.expand-collapse-button svg {
     transform: rotate(90deg);
 }
+
+:not(.expanded) ::deep .items {
+    display: none;
+} 
diff --git a/src/Core/Components/NavMenu/FluentNavLink.razor b/src/Core/Components/NavMenu/FluentNavLink.razor
@@ -2,33 +2,56 @@
 @inherits FluentNavBase
 @using Microsoft.AspNetCore.Components.Rendering
 @using Microsoft.AspNetCore.Components.Routing
+@inject NavigationManager NavigationManager
 
-<div id="@Id" @attributes="AdditionalAttributes" class="@ClassValue" disabled="@Disabled" style="@Style" role="menuitem">
-    @if (!OnClick.HasDelegate && !string.IsNullOrEmpty(Href))
-    {
-        <NavLink class="@LinkClassValue"
-                 @attributes="@Attributes"
-                 Match="@Match"
-                 ActiveClass="@ActiveClass"
-                 title="@Tooltip">
-            <div class="positioning-region">
+@if (Owner == null || Owner.Expanded || (HasIcon && (!Owner.CollapsedChildNavigation || SubMenu == null)))
+{
+    <div id="@Id" @attributes="AdditionalAttributes" class="@ClassValue" disabled="@Disabled" style="@Style" role="menuitem">
+        @if (!OnClick.HasDelegate && !string.IsNullOrEmpty(Href))
+        {
+            <NavLink class="@LinkClassValue"
+                     @attributes="@Attributes"
+                     Match="@Match"
+                     ActiveClass="@ActiveClass"
+                     title="@Tooltip">
+                <div class="positioning-region">
+                    <div class="content-region">
+                        @_renderContent
+                    </div>
+                </div>
+            </NavLink>
+        }
+        else
+        {
+            <div class="positioning-region" title="@Tooltip">
                 <div class="content-region">
-                    @_renderContent
+                    <div class="@LinkClassValue" @onclick="OnClickHandlerAsync">
+                        @_renderContent
+                    </div>
                 </div>
             </div>
+        }
+    </div>
+}
+else if (!Owner.Expanded && Owner.CollapsedChildNavigation && SubMenu != null)
+{
+    <FluentMenuItem Disabled="@Disabled" OnClick="OnClickHandlerAsync" @onmenuchange="async ev => await OnClickHandlerAsync(null)">
+        <NavLink class="@LinkClassValue"
+                    @attributes="@Attributes"
+                    Match="@Match"
+                    ActiveClass="@ActiveClass"
+                    title="@Tooltip">
+            @ChildContent
         </NavLink>
-    }
-    else
-    {
-        <div class="positioning-region" title="@Tooltip">
-            <div class="content-region">
-                <div class="@LinkClassValue" @onclick="OnClickHandler">
-                    @_renderContent
-                </div>
-            </div>
-        </div>
-    }
-</div>
+
+        @if (Icon is not null)
+        {
+            <span slot="start">
+                <FluentIcon Value="@Icon" Color="@IconColor" Class="fluent-nav-icon" />
+            </span>
+        }
+    </FluentMenuItem>
+}
 
 @code {
     private void RenderContent(RenderTreeBuilder __builder)
@@ -44,7 +67,5 @@
         <div class="fluent-nav-text">
             @ChildContent
         </div>
-
-
     }
 }
diff --git a/src/Core/Components/NavMenu/FluentNavMenu.razor b/src/Core/Components/NavMenu/FluentNavMenu.razor
@@ -4,7 +4,7 @@
     <CascadingValue Value="this" IsFixed="true">
         @if (Collapsible)
         {
-            <FluentKeyCode Anchor="expander" OnKeyDown="@HandleExpandCollapseKeyDownAsync" />
+            <FluentKeyCode Anchor="@($"{@Id}-expander")" OnKeyDown="@HandleExpandCollapseKeyDownAsync" />
             <div id="@($"{@Id}-expander")"
                  aria-label="@Title"
                  aria-expanded="@(Expanded.ToAttributeValue())"

diff --git a/src/Core/Components/NavMenu/FluentNavMenu.razor.css b/src/Core/Components/NavMenu/FluentNavMenu.razor.css
@@ -59,6 +59,14 @@
     margin-bottom: 2px;
 }
 
+::deep fluent-menu-item .fluent-nav-link {
+    width: 100%;
+    color: inherit;
+    align-items: center;
+    text-decoration: none;
+    display: flex;
+}
+
 /* level indenting */
 ::deep .fluent-nav-group * .fluent-nav-menu > .fluent-nav-item .content-region {
     padding-inline-start: 24px;

diff --git a/tests/Core/NavMenu/FluentNavGroupTests.FluentNavGroup_Disabled.verified.html b/tests/Core/NavMenu/FluentNavGroupTests.FluentNavGroup_Disabled.verified.html
@@ -1,6 +1,6 @@
 
 <div id="xxx" class="fluent-nav-item disabled fluent-nav-group" disabled="" role="menuitem" b-hv9rzieq3w="" blazor:elementreference="xxx">
-  <div class="fluent-nav-link notactive" tabindex="0" b-hv9rzieq3w="">
+  <div class="fluent-nav-link notactive" tabindex="-1" b-hv9rzieq3w="">
     <div class="positioning-region" blazor:onclick="1" blazor:onkeydown="2" title="Group title" b-hv9rzieq3w="">
       <div class="content-region" b-hv9rzieq3w="">
         <span class="fluent-nav-icon" style="min-width: 20px;" b-hv9rzieq3w=""></span>
@@ -17,4 +17,4 @@
   <div role="group" class="fluent-collapsible-region-container items" style="height: 0;" b-hk0m26j2or="">
     <div id="xxx" class="fluent-nav-menu" style="width: 100%;" aria-label="Navigation menu" role="menu" aria-expanded="" b-vs37bumpa7="" blazor:elementreference="xxx">NavGroups and NavLinks here</div>
   </div>
-</div>
+</div>