fix: Fix toolbar feature flag detection to only apply in visual refre…

…sh mode (#2584)
cloudscape-design · Aug 21, 2024 · 4585b4e · 4585b4e
1 parent bb12714
commit 4585b4e
Show file tree

Hide file tree

Showing 10 changed files with 69 additions and 38 deletions.
diff --git a/src/app-layout/__integ__/global-breadcrumbs.test.ts b/src/app-layout/__integ__/global-breadcrumbs.test.ts
@@ -23,30 +23,55 @@ class GlobalBreadcrumbsPage extends BasePageObject {
   }
 }
 
+function setupTest({ url }: { url: string }, testFn: (page: GlobalBreadcrumbsPage) => Promise<void>) {
+  return useBrowser(async browser => {
+    const page = new GlobalBreadcrumbsPage(browser);
+    await browser.url(url);
+    await page.waitForVisible(wrapper.findAppLayout().findContentRegion().toSelector());
+    await testFn(page);
+  });
+}
+
 describe.each(['classic', 'visual-refresh'])('%s', theme => {
+  const visualRefresh = theme === 'visual-refresh' ? 'true' : 'false';
   test(
     'does not work in this design',
-    useBrowser(async browser => {
-      const page = new GlobalBreadcrumbsPage(browser);
-      await browser.url(
-        `#/light/app-layout/global-breadcrumbs/?visualRefresh=${theme === 'visual-refresh' ? 'true' : 'false'}`
-      );
-      await expect(page.getRootBreadcrumbText()).resolves.toEqual('Default');
-      await expect(page.getBreadcrumbsCount()).resolves.toEqual(1);
+    setupTest(
+      {
+        url: `#/light/app-layout/global-breadcrumbs/?${new URLSearchParams({ visualRefresh }).toString()}`,
+      },
+      async page => {
+        await expect(page.getRootBreadcrumbText()).resolves.toEqual('Default');
+        await expect(page.getBreadcrumbsCount()).resolves.toEqual(1);
 
-      await page.toggleExtraBreadcrumb();
-      await expect(page.getRootBreadcrumbText()).resolves.toEqual('Default');
-      await expect(page.getBreadcrumbsCount()).resolves.toEqual(2);
-    })
+        await page.toggleExtraBreadcrumb();
+        await expect(page.getRootBreadcrumbText()).resolves.toEqual('Default');
+        await expect(page.getBreadcrumbsCount()).resolves.toEqual(2);
+      }
+    )
+  );
+});
+
+describe('classic', () => {
+  test(
+    'does not react to the feature flag even if it is enabled',
+    setupTest(
+      {
+        url: `#/light/app-layout/global-breadcrumbs/?${new URLSearchParams({ visualRefresh: 'false', appLayoutWidget: 'true' }).toString()}`,
+      },
+      async page => {
+        await page.toggleExtraBreadcrumb();
+        await expect(page.getRootBreadcrumbText()).resolves.toEqual('Default');
+        await expect(page.getBreadcrumbsCount()).resolves.toEqual(2);
+      }
+    )
   );
 });
 
 describe('visual-refresh-toolbar', () => {
   test(
     'deduplicates breadcrumbs',
-    useBrowser(async browser => {
-      const page = new GlobalBreadcrumbsPage(browser);
-      await browser.url(`#/light/app-layout/global-breadcrumbs/?visualRefresh=true&appLayoutWidget=true`);
+    setupTest({ url: `#/light/app-layout/global-breadcrumbs/?visualRefresh=true&appLayoutWidget=true` }, async page => {
       await expect(page.getRootBreadcrumbText()).resolves.toEqual('Default');
       await expect(page.getBreadcrumbsCount()).resolves.toEqual(1);
 

diff --git a/src/app-layout/internal.tsx b/src/app-layout/internal.tsx
@@ -5,14 +5,15 @@ import React from 'react';
 import { useVisualRefresh } from '../internal/hooks/use-visual-mode';
 import ClassicAppLayout from './classic';
 import { AppLayoutProps, AppLayoutPropsWithDefaults } from './interfaces';
-import { isAppLayoutToolbarEnabled } from './utils/feature-flags';
+import { useAppLayoutToolbarEnabled } from './utils/feature-flags';
 import RefreshedAppLayout from './visual-refresh';
 import ToolbarAppLayout from './visual-refresh-toolbar';
 
 export const AppLayoutInternal = React.forwardRef<AppLayoutProps.Ref, AppLayoutPropsWithDefaults>((props, ref) => {
   const isRefresh = useVisualRefresh();
+  const isToolbar = useAppLayoutToolbarEnabled();
   if (isRefresh) {
-    if (isAppLayoutToolbarEnabled()) {
+    if (isToolbar) {
       return <ToolbarAppLayout ref={ref} {...props} />;
     } else {
       return <RefreshedAppLayout ref={ref} {...props} />;

diff --git a/src/app-layout/utils/feature-flags.ts b/src/app-layout/utils/feature-flags.ts
@@ -2,4 +2,9 @@
 // SPDX-License-Identifier: Apache-2.0
 import { getGlobalFlag } from '@cloudscape-design/component-toolkit/internal';
 
-export const isAppLayoutToolbarEnabled = () => getGlobalFlag('appLayoutWidget') || getGlobalFlag('appLayoutToolbar');
+import { useVisualRefresh } from '../../internal/hooks/use-visual-mode';
+
+export const useAppLayoutToolbarEnabled = () => {
+  const isRefresh = useVisualRefresh();
+  return isRefresh && (getGlobalFlag('appLayoutWidget') || getGlobalFlag('appLayoutToolbar'));
+};
diff --git a/src/drawer/implementation.tsx b/src/drawer/implementation.tsx
@@ -3,7 +3,7 @@
 import React from 'react';
 import clsx from 'clsx';
 
-import { isAppLayoutToolbarEnabled } from '../app-layout/utils/feature-flags';
+import { useAppLayoutToolbarEnabled } from '../app-layout/utils/feature-flags';
 import { useInternalI18n } from '../i18n/context';
 import { getBaseProps } from '../internal/base-component';
 import LiveRegion from '../internal/components/live-region';
@@ -25,10 +25,11 @@ export function DrawerImplementation({
   ...restProps
 }: DrawerInternalProps) {
   const baseProps = getBaseProps(restProps);
+  const isToolbar = useAppLayoutToolbarEnabled();
   const i18n = useInternalI18n('drawer');
   const containerProps = {
     ...baseProps,
-    className: clsx(baseProps.className, styles.drawer, isAppLayoutToolbarEnabled() && styles['with-toolbar']),
+    className: clsx(baseProps.className, styles.drawer, isToolbar && styles['with-toolbar']),
   };
   return loading ? (
     <div {...containerProps} ref={__internalRootRef}>

diff --git a/src/help-panel/implementation.tsx b/src/help-panel/implementation.tsx
@@ -3,7 +3,7 @@
 import React from 'react';
 import clsx from 'clsx';
 
-import { isAppLayoutToolbarEnabled } from '../app-layout/utils/feature-flags';
+import { useAppLayoutToolbarEnabled } from '../app-layout/utils/feature-flags';
 import { useInternalI18n } from '../i18n/context';
 import { getBaseProps } from '../internal/base-component';
 import LiveRegion from '../internal/components/live-region';
@@ -27,10 +27,11 @@ export function HelpPanelImplementation({
   ...restProps
 }: HelpPanelInternalProps) {
   const baseProps = getBaseProps(restProps);
+  const isToolbar = useAppLayoutToolbarEnabled();
   const i18n = useInternalI18n('help-panel');
   const containerProps = {
     ...baseProps,
-    className: clsx(baseProps.className, styles['help-panel'], isAppLayoutToolbarEnabled() && styles['with-toolbar']),
+    className: clsx(baseProps.className, styles['help-panel'], isToolbar && styles['with-toolbar']),
   };
   return loading ? (
     <div {...containerProps} ref={__internalRootRef}>

diff --git a/src/internal/plugins/helpers/use-global-breadcrumbs.ts b/src/internal/plugins/helpers/use-global-breadcrumbs.ts
@@ -2,7 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 import { useEffect, useLayoutEffect, useRef, useState } from 'react';
 
-import { isAppLayoutToolbarEnabled } from '../../../app-layout/utils/feature-flags';
+import { useAppLayoutToolbarEnabled } from '../../../app-layout/utils/feature-flags';
 import { BreadcrumbGroupProps } from '../../../breadcrumb-group/interfaces';
 import { awsuiPluginsInternal } from '../api';
 import { BreadcrumbsGlobalRegistration } from '../controllers/breadcrumbs';
@@ -31,7 +31,7 @@ function useSetGlobalBreadcrumbsImplementation(props: BreadcrumbGroupProps<any>)
 
 export function useSetGlobalBreadcrumbs<T extends BreadcrumbGroupProps.Item>(props: BreadcrumbGroupProps<T>) {
   // avoid additional side effects when this feature is not active
-  if (!isAppLayoutToolbarEnabled()) {
+  if (!useAppLayoutToolbarEnabled()) {
     return false;
   }
   // getGlobalFlag() value does not change without full page reload

diff --git a/src/side-navigation/implementation.tsx b/src/side-navigation/implementation.tsx
@@ -3,7 +3,7 @@
 import React, { useCallback, useEffect, useMemo } from 'react';
 import clsx from 'clsx';
 
-import { isAppLayoutToolbarEnabled } from '../app-layout/utils/feature-flags';
+import { useAppLayoutToolbarEnabled } from '../app-layout/utils/feature-flags';
 import { getBaseProps } from '../internal/base-component';
 import { fireCancelableEvent, fireNonCancelableEvent } from '../internal/events';
 import { InternalBaseComponentProps } from '../internal/hooks/use-base-component';
@@ -27,6 +27,7 @@ export function SideNavigationImplementation({
   ...props
 }: SideNavigationInternalProps) {
   const baseProps = getBaseProps(props);
+  const isToolbar = useAppLayoutToolbarEnabled();
   const parentMap = useMemo(() => generateExpandableItemsMapping(items), [items]);
 
   if (isDevelopment) {
@@ -61,7 +62,7 @@ export function SideNavigationImplementation({
   return (
     <div
       {...baseProps}
-      className={clsx(styles.root, baseProps.className, isAppLayoutToolbarEnabled() && styles['with-toolbar'])}
+      className={clsx(styles.root, baseProps.className, isToolbar && styles['with-toolbar'])}
       ref={__internalRootRef}
     >
       {header && (

diff --git a/src/split-panel/bottom.tsx b/src/split-panel/bottom.tsx
@@ -5,7 +5,7 @@ import clsx from 'clsx';
 
 import { useResizeObserver } from '@cloudscape-design/component-toolkit/internal';
 
-import { isAppLayoutToolbarEnabled } from '../app-layout/utils/feature-flags';
+import { useAppLayoutToolbarEnabled } from '../app-layout/utils/feature-flags';
 import { TransitionStatus } from '../internal/components/transition';
 import { useSplitPanelContext } from '../internal/context/split-panel-context';
 import { useMergeRefs } from '../internal/hooks/use-merge-refs';
@@ -37,6 +37,7 @@ export function SplitPanelContentBottom({
   onToggle,
 }: SplitPanelContentBottomProps) {
   const isRefresh = useVisualRefresh();
+  const isToolbar = useAppLayoutToolbarEnabled();
   const { bottomOffset, leftOffset, rightOffset, disableContentPaddings, contentWrapperPaddings, reportHeaderHeight } =
     useSplitPanelContext();
   const transitionContentBottomRef = useMergeRefs(splitPanelRef || null, transitioningElementRef);
@@ -66,7 +67,7 @@ export function SplitPanelContentBottom({
         [styles['drawer-disable-content-paddings']]: disableContentPaddings,
         [styles.animating]: isRefresh && (state === 'entering' || state === 'exiting'),
         [styles.refresh]: isRefresh,
-        [styles['with-toolbar']]: isAppLayoutToolbarEnabled(),
+        [styles['with-toolbar']]: isToolbar,
       })}
       onClick={() => !isOpen && onToggle()}
       style={{

diff --git a/src/split-panel/implementation.tsx b/src/split-panel/implementation.tsx
@@ -3,7 +3,7 @@
 import React, { useEffect, useLayoutEffect, useRef, useState } from 'react';
 import clsx from 'clsx';
 
-import { isAppLayoutToolbarEnabled } from '../app-layout/utils/feature-flags';
+import { useAppLayoutToolbarEnabled } from '../app-layout/utils/feature-flags';
 import { SizeControlProps } from '../app-layout/utils/interfaces';
 import { useKeyboardEvents } from '../app-layout/utils/use-keyboard-events';
 import { usePointerEvents } from '../app-layout/utils/use-pointer-events';
@@ -33,6 +33,7 @@ export const SplitPanelImplementation = React.forwardRef<HTMLElement, SplitPanel
     __internalRootRef
   ) => {
     const isRefresh = useVisualRefresh();
+    const isToolbar = useAppLayoutToolbarEnabled();
 
     const {
       position,
@@ -79,10 +80,7 @@ export const SplitPanelImplementation = React.forwardRef<HTMLElement, SplitPanel
     const panelHeaderId = useUniqueId('split-panel-header');
 
     const wrappedHeader = (
-      <div
-        className={clsx(styles.header, isAppLayoutToolbarEnabled() && styles['with-toolbar'])}
-        style={appLayoutMaxWidth}
-      >
+      <div className={clsx(styles.header, isToolbar && styles['with-toolbar'])} style={appLayoutMaxWidth}>
         <h2 className={clsx(styles['header-text'], testUtilStyles['header-text'])} id={panelHeaderId}>
           {header}
         </h2>

diff --git a/src/split-panel/side.tsx b/src/split-panel/side.tsx
@@ -3,7 +3,7 @@
 import React from 'react';
 import clsx from 'clsx';
 
-import { isAppLayoutToolbarEnabled } from '../app-layout/utils/feature-flags';
+import { useAppLayoutToolbarEnabled } from '../app-layout/utils/feature-flags';
 import { ButtonProps } from '../button/interfaces';
 import InternalButton from '../button/internal';
 import { useSplitPanelContext } from '../internal/context/split-panel-context';
@@ -34,13 +34,14 @@ export function SplitPanelContentSide({
 }: SplitPanelContentSideProps) {
   const { topOffset, bottomOffset } = useSplitPanelContext();
   const isRefresh = useVisualRefresh();
+  const isToolbar = useAppLayoutToolbarEnabled();
   return (
     <div
       {...baseProps}
       className={clsx(baseProps.className, styles.drawer, styles['position-side'], testUtilStyles.root, {
         [testUtilStyles['open-position-side']]: isOpen,
         [styles['drawer-closed']]: !isOpen,
-        [styles['with-toolbar']]: isAppLayoutToolbarEnabled(),
+        [styles['with-toolbar']]: isToolbar,
       })}
       style={{
         width: isOpen && isRefresh ? cappedSize : undefined,
@@ -72,10 +73,7 @@ export function SplitPanelContentSide({
             ref={isRefresh ? null : toggleRef}
           />
         )}
-        <div
-          className={clsx(styles['content-side'], isAppLayoutToolbarEnabled() && styles['with-toolbar'])}
-          aria-hidden={!isOpen}
-        >
+        <div className={clsx(styles['content-side'], isToolbar && styles['with-toolbar'])} aria-hidden={!isOpen}>
           <div className={styles['pane-header-wrapper-side']}>{header}</div>
           <div className={styles['pane-content-wrapper-side']}>{children}</div>
         </div>