fix: hide navigation button when toolbar variant of AppLayout has hideNavigation

src/app-layout/__tests__/multi-layout-props.test.tsx

@@ -0,0 +1,109 @@
+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+import React from 'react';
+
+import {
+  mergeMultiAppLayoutProps,
+  SharedMultiAppLayoutProps,
+} from '../../../lib/components/app-layout/visual-refresh-toolbar/multi-layout';
+
+describe('mergeMultiAppLayoutProps', () => {
+  const mockParentNavigationToggle = jest.fn();
+  const mockParentActiveDrawerChange = jest.fn();
+  const mockParentSplitPanelToggle = jest.fn();
+  const ownProps: SharedMultiAppLayoutProps = {
+    forceDeduplicationType: 'primary',
+    ariaLabels: {
+      navigation: 'Navigation',
+      drawers: 'Drawers',
+    },
+    navigation: <div>Navigation</div>,
+    navigationOpen: true,
+    navigationFocusRef: React.createRef(),
+    onNavigationToggle: mockParentNavigationToggle,
+    breadcrumbs: <div>Breadcrumbs</div>,
+    activeDrawerId: 'drawer1',
+    drawers: [
+      {
+        id: 'drawer1',
+        ariaLabels: { drawerName: 'Drawer 1' },
+        content: <div>Drawer 1 Content</div>,
+      },
+    ],
+    onActiveDrawerChange: mockParentActiveDrawerChange,
+    drawersFocusRef: React.createRef(),
+    splitPanel: <div>Split Panel</div>,
+    splitPanelToggleProps: {
+      displayed: false,
+      active: false,
+      position: 'bottom',
+      controlId: 'test',
+      ariaLabel: 'test',
+    },
+    splitPanelFocusRef: React.createRef(),
+    onSplitPanelToggle: mockParentSplitPanelToggle,
+  };
+
+  const additionalPropsBase: Partial<SharedMultiAppLayoutProps>[] = [
+    {
+      ariaLabels: {
+        navigation: 'New Navigation',
+      },
+      drawers: [
+        {
+          id: 'drawer2',
+          ariaLabels: { drawerName: 'Drawer 2' },
+          content: <div>Drawer 2 Content</div>,
+        },
+      ],
+      activeDrawerId: 'drawer2',
+    },
+    {
+      splitPanelToggleProps: {
+        displayed: false,
+        active: false,
+        position: 'bottom',
+        controlId: 'test',
+        ariaLabel: 'test',
+      },
+    },
+  ];
+
+  it('should merge ownProps and additionalProps correctly', () => {
+    const result = mergeMultiAppLayoutProps(ownProps, additionalPropsBase);
+
+    expect(result).toEqual({
+      //asserting new aria labels overwrite existing yet preserve others
+      ariaLabels: {
+        navigation: 'New Navigation',
+        drawers: 'Drawers',
+      },
+      hasNavigation: true,
+      navigationOpen: true,
+      navigationFocusRef: ownProps.navigationFocusRef,
+      onNavigationToggle: mockParentNavigationToggle,
+      hasBreadcrumbsPortal: true,
+      hasSplitPanel: true,
+      splitPanelToggleProps: {
+        displayed: false,
+        active: false,
+        position: 'bottom',
+        controlId: 'test',
+        ariaLabel: 'test',
+      },
+      splitPanelFocusRef: ownProps.splitPanelFocusRef,
+      onSplitPanelToggle: mockParentSplitPanelToggle,
+      //asserting the ownProps drawer is not overwritten
+      activeDrawerId: ownProps.activeDrawerId,
+      drawers: ownProps.drawers,
+      drawersFocusRef: ownProps.drawersFocusRef,
+      onActiveDrawerChange: mockParentActiveDrawerChange,
+    });
+  });
+
+  it('should return null if no fields are defined, except ariaLabels', () => {
+    const result = mergeMultiAppLayoutProps({ ariaLabels: {} } as SharedMultiAppLayoutProps, []);
+
+    expect(result).toBeNull();
+  });
+});

src/app-layout/__tests__/multi-layout.test.tsx

@@ -75,7 +75,26 @@ describeEachAppLayout({ themes: ['refresh-toolbar'], sizes: ['desktop'] }, () =>
     expect(isDrawerClosed(firstLayout.findNavigation())).toEqual(false);
   });
 
-  test('merges tools from two instances', async () => {
+  test('navigationHide in primary is respected when navigation is defined when merging from two instances', async () => {
+    const { firstLayout, secondLayout } = await renderAsync(
+      <AppLayout
+        {...defaultAppLayoutProps}
+        data-testid="first"
+        navigation="testing nav"
+        navigationHide={true}
+        toolsHide={true}
+        content={
+          <AppLayout {...defaultAppLayoutProps} data-testid="second" navigationHide={true} tools="testing tools" />
+        }
+      />
+    );
+    expect(firstLayout.findNavigation()).toBeFalsy();
+    expect(firstLayout.findNavigationToggle()).toBeFalsy();
+    expect(secondLayout.findNavigation()).toBeFalsy();
+    expect(secondLayout.findNavigationToggle()).toBeFalsy();
+  });
+
+  test('merges tools from two instances with where navigationHide is true in secondary', async () => {
     const { firstLayout, secondLayout } = await renderAsync(
       <AppLayout
         {...defaultAppLayoutProps}
@@ -89,6 +108,8 @@ describeEachAppLayout({ themes: ['refresh-toolbar'], sizes: ['desktop'] }, () =>
     expect(createWrapper().findAllByClassName(testUtilStyles.tools)).toHaveLength(1);
 
     firstLayout.findToolsToggle().click();
+    expect(secondLayout.findNavigation()).toBeFalsy();
+    expect(secondLayout.findNavigationToggle()).toBeFalsy();
     expect(isDrawerClosed(secondLayout.findTools())).toEqual(false);
   });
 

src/app-layout/visual-refresh-toolbar/index.tsx

@@ -235,8 +235,11 @@ const AppLayoutVisualRefreshToolbar = React.forwardRef<AppLayoutProps.Ref, AppLa
       focusSplitPanel: () => splitPanelFocusControl.refs.slider.current?.focus(),
     }));
 
-    const resolvedNavigation = navigationHide ? null : navigation ?? <></>;
     const resolvedStickyNotifications = !!stickyNotifications && !isMobile;
+    //navigation must be null if hidden so toolbar knows to hide the toggle button
+    const resolvedNavigation = navigationHide ? null : navigation ?? <></>;
+    //navigation must not be open if navigationHide is true
+    const resolvedNavigationOpen = !!resolvedNavigation && navigationOpen;
     const {
       maxDrawerSize,
       maxSplitPanelSize,
@@ -248,7 +251,7 @@ const AppLayoutVisualRefreshToolbar = React.forwardRef<AppLayoutProps.Ref, AppLa
       activeDrawerSize: activeDrawer ? activeDrawerSize : 0,
       splitPanelSize,
       minContentWidth,
-      navigationOpen: !!resolvedNavigation && navigationOpen,
+      navigationOpen: resolvedNavigationOpen,
       navigationWidth,
       placement,
       splitPanelOpen,
@@ -261,7 +264,7 @@ const AppLayoutVisualRefreshToolbar = React.forwardRef<AppLayoutProps.Ref, AppLa
       forceDeduplicationType,
       ariaLabels: ariaLabelsWithDrawers,
       navigation: resolvedNavigation,
-      navigationOpen,
+      navigationOpen: resolvedNavigationOpen,
       onNavigationToggle,
       navigationFocusRef: navigationFocusControl.refs.toggle,
       breadcrumbs,
@@ -299,7 +302,7 @@ const AppLayoutVisualRefreshToolbar = React.forwardRef<AppLayoutProps.Ref, AppLa
       breadcrumbs,
       discoveredBreadcrumbs,
       stickyNotifications: resolvedStickyNotifications,
-      navigationOpen,
+      navigationOpen: resolvedNavigationOpen,
       navigation: resolvedNavigation,
       navigationFocusControl,
       activeDrawer,
@@ -393,7 +396,7 @@ const AppLayoutVisualRefreshToolbar = React.forwardRef<AppLayoutProps.Ref, AppLa
           // delay rendering the content until registration of this instance is complete
           content={registered ? content : null}
           navigation={resolvedNavigation && <AppLayoutNavigation appLayoutInternals={appLayoutInternals} />}
-          navigationOpen={navigationOpen}
+          navigationOpen={resolvedNavigationOpen}
           navigationWidth={navigationWidth}
           tools={drawers && drawers.length > 0 && <AppLayoutDrawer appLayoutInternals={appLayoutInternals} />}
           globalTools={

src/app-layout/visual-refresh-toolbar/multi-layout.ts

@@ -10,7 +10,7 @@ import { AppLayoutProps } from '../interfaces';
 import { Focusable } from '../utils/use-focus-control';
 import { SplitPanelToggleProps, ToolbarProps } from './toolbar';
 
-interface SharedProps {
+export interface SharedMultiAppLayoutProps {
   forceDeduplicationType?: 'primary' | 'secondary';
   ariaLabels: AppLayoutProps.Labels | undefined;
   navigation: React.ReactNode;
@@ -39,7 +39,10 @@ function checkAlreadyExists(value: boolean, propName: string) {
   return false;
 }
 
-function mergeProps(ownProps: SharedProps, additionalProps: ReadonlyArray<Partial<SharedProps>>): ToolbarProps | null {
+export function mergeMultiAppLayoutProps(
+  ownProps: SharedMultiAppLayoutProps,
+  additionalProps: ReadonlyArray<Partial<SharedMultiAppLayoutProps>>
+): ToolbarProps | null {
   const toolbar: ToolbarProps = {};
   for (const props of [ownProps, ...additionalProps]) {
     toolbar.ariaLabels = Object.assign(toolbar.ariaLabels ?? {}, props.ariaLabels);
@@ -50,6 +53,8 @@ function mergeProps(ownProps: SharedProps, additionalProps: ReadonlyArray<Partia
       toolbar.onActiveDrawerChange = props.onActiveDrawerChange;
     }
     if (props.navigation && !checkAlreadyExists(!!toolbar.hasNavigation, 'navigation')) {
+      // there is never a case where navigation will exist and a toggle will not so toolbar
+      // can use the hasNavigation here to conditionally render the navigationToggle button
       toolbar.hasNavigation = true;
       toolbar.navigationOpen = props.navigationOpen;
       toolbar.navigationFocusRef = props.navigationFocusRef;
@@ -69,13 +74,13 @@ function mergeProps(ownProps: SharedProps, additionalProps: ReadonlyArray<Partia
   return Object.keys(toolbar).filter(key => key !== 'ariaLabels').length > 0 ? toolbar : null;
 }
 
-export function useMultiAppLayout(props: SharedProps) {
-  const [registration, setRegistration] = useState<RegistrationState<SharedProps> | null>(null);
+export function useMultiAppLayout(props: SharedMultiAppLayoutProps) {
+  const [registration, setRegistration] = useState<RegistrationState<SharedMultiAppLayoutProps> | null>(null);
   const { forceDeduplicationType } = props;
 
   useLayoutEffect(() => {
     return awsuiPluginsInternal.appLayoutWidget.register(forceDeduplicationType, props =>
-      setRegistration(props as RegistrationState<SharedProps>)
+      setRegistration(props as RegistrationState<SharedMultiAppLayoutProps>)
     );
   }, [forceDeduplicationType]);
 
@@ -87,6 +92,7 @@ export function useMultiAppLayout(props: SharedProps) {
 
   return {
     registered: !!registration?.type,
-    toolbarProps: registration?.type === 'primary' ? mergeProps(props, registration.discoveredProps) : null,
+    toolbarProps:
+      registration?.type === 'primary' ? mergeMultiAppLayoutProps(props, registration.discoveredProps) : null,
   };
 }