fix: Fix visual bugs in drawer toggle buttons

cloudscape-design · Jul 18, 2023 · 21ff56e · 21ff56e
1 parent 97e4f0c
commit 21ff56e
Show file tree

Hide file tree

Showing 17 changed files with 122 additions and 137 deletions.
diff --git a/src/__integ__/__snapshots__/themes.test.ts.snap b/src/__integ__/__snapshots__/themes.test.ts.snap
@@ -321,7 +321,6 @@ Object {
   "color-foreground-control-default": "#ffffff",
   "color-foreground-control-disabled": "#ffffff",
   "color-shadow-default": "rgba(0, 28, 36, 0.5)",
-  "color-shadow-layout-toggle": "#d5dbdb",
   "color-stroke-chart-line": "#879596",
   "color-text-accent": "#0073bb",
   "color-text-body-default": "#16191f",
@@ -931,7 +930,6 @@ Object {
   "color-foreground-control-default": "#ffffff",
   "color-foreground-control-disabled": "#687078",
   "color-shadow-default": "rgba(0, 0, 0, 0.5)",
-  "color-shadow-layout-toggle": "#414750",
   "color-stroke-chart-line": "#879596",
   "color-text-accent": "#44b9d6",
   "color-text-body-default": "#d5dbdb",
@@ -1541,7 +1539,6 @@ Object {
   "color-foreground-control-default": "#ffffff",
   "color-foreground-control-disabled": "#ffffff",
   "color-shadow-default": "rgba(0, 28, 36, 0.5)",
-  "color-shadow-layout-toggle": "#d5dbdb",
   "color-stroke-chart-line": "#879596",
   "color-text-accent": "#0073bb",
   "color-text-body-default": "#16191f",
@@ -2151,7 +2148,6 @@ Object {
   "color-foreground-control-default": "#ffffff",
   "color-foreground-control-disabled": "#ffffff",
   "color-shadow-default": "rgba(0, 28, 36, 0.5)",
-  "color-shadow-layout-toggle": "#d5dbdb",
   "color-stroke-chart-line": "#879596",
   "color-text-accent": "#0073bb",
   "color-text-body-default": "#16191f",
@@ -2761,7 +2757,6 @@ Object {
   "color-foreground-control-default": "#ffffff",
   "color-foreground-control-disabled": "#ffffff",
   "color-shadow-default": "rgba(0, 7, 22, 0.12)",
-  "color-shadow-layout-toggle": "#d1d5db",
   "color-stroke-chart-line": "#7d8998",
   "color-text-accent": "#0972d3",
   "color-text-body-default": "#000716",
@@ -3371,7 +3366,6 @@ Object {
   "color-foreground-control-default": "#ffffff",
   "color-foreground-control-disabled": "#ffffff",
   "color-shadow-default": "rgba(0, 7, 22, 0.12)",
-  "color-shadow-layout-toggle": "#d1d5db",
   "color-stroke-chart-line": "#7d8998",
   "color-text-accent": "#0972d3",
   "color-text-body-default": "#000716",
@@ -3981,7 +3975,6 @@ Object {
   "color-foreground-control-default": "#000716",
   "color-foreground-control-disabled": "#0f1b2a",
   "color-shadow-default": "rgba(0, 7, 22, 1)",
-  "color-shadow-layout-toggle": "#354150",
   "color-stroke-chart-line": "#7d8998",
   "color-text-accent": "#539fe5",
   "color-text-body-default": "#d1d5db",
@@ -4591,7 +4584,6 @@ Object {
   "color-foreground-control-default": "#000716",
   "color-foreground-control-disabled": "#0f1b2a",
   "color-shadow-default": "rgba(0, 7, 22, 1)",
-  "color-shadow-layout-toggle": "#354150",
   "color-stroke-chart-line": "#7d8998",
   "color-text-accent": "#539fe5",
   "color-text-body-default": "#d1d5db",

diff --git a/src/app-layout/__integ__/app-layout-focus-delegation.test.ts b/src/app-layout/__integ__/app-layout-focus-delegation.test.ts
@@ -91,13 +91,14 @@ function setupTest(
           'drawers focus toggles between open and close buttons',
           setupTest(
             async page => {
-              await page.click(wrapper.findDrawersTriggers().get(2).toSelector());
+              const triggerSelector = wrapper.findDrawerTriggerById('pro-help').toSelector();
+              await page.click(triggerSelector);
               await page.keys('Enter');
-              await expect(page.isFocused(wrapper.findDrawersTriggers().get(2).toSelector())).resolves.toBe(true);
+              await expect(page.isFocused(triggerSelector)).resolves.toBe(true);
               await page.keys('Enter');
               await expect(page.isFocused(wrapper.findActiveDrawerCloseButton().toSelector())).resolves.toBe(true);
               await page.keys('Enter');
-              await expect(page.isFocused(wrapper.findDrawersTriggers().get(2).toSelector())).resolves.toBe(true);
+              await expect(page.isFocused(triggerSelector)).resolves.toBe(true);
             },
             { pageName: 'with-drawers', visualRefresh, mobile }
           )

diff --git a/src/app-layout/__tests__/desktop.test.tsx b/src/app-layout/__tests__/desktop.test.tsx
@@ -171,7 +171,10 @@ describeEachThemeAppLayout(false, () => {
 
   test('Adds labels to toggle button and landmark when defined', () => {
     const { wrapper } = renderComponent(<AppLayout contentType="form" {...singleDrawer} />);
-    expect(wrapper.findDrawersTriggers()![0].getElement()).toHaveAttribute('aria-label', 'Security trigger button');
+    expect(wrapper.findDrawersTriggerById('security')!.getElement()).toHaveAttribute(
+      'aria-label',
+      'Security trigger button'
+    );
     expect(wrapper.findDrawersDesktopTriggersContainer()!.getElement()).toHaveAttribute('aria-label', 'Drawers');
   });
 

diff --git a/src/app-layout/constants.scss b/src/app-layout/constants.scss
@@ -8,14 +8,9 @@
 
 // Toggle should have 40px width, whereas button is 26px wide.
 // 40 - 26 = 14px in total or 7px on each side
-$toggle-padding-horizontal: 0.7 * styles.$base-size;
-$toggle-padding-vertical: awsui.$space-xxs;
-$toggle-padding: $toggle-padding-vertical $toggle-padding-horizontal;
-
-// New styles for when there are drawers
-$drawers-padding-horizontal: 0.6 * styles.$base-size;
-$drawers-padding-vertical: awsui.$space-scaled-xs;
-$drawers-padding: $drawers-padding-vertical $drawers-padding-horizontal;
+$_toggle-padding-horizontal: 0.7 * styles.$base-size;
+$_toggle-padding-vertical: awsui.$space-xxs;
+$toggle-padding: $_toggle-padding-vertical $_toggle-padding-horizontal;
 
 $sidebar-size-closed: 4 * styles.$base-size;
 

diff --git a/src/app-layout/drawer/index.tsx b/src/app-layout/drawer/index.tsx
@@ -2,7 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 import clsx from 'clsx';
 import React, { useRef } from 'react';
-import { AppLayoutButton, CloseButton, togglesConfig } from '../toggles';
+import { ToggleButton, CloseButton, togglesConfig } from '../toggles';
 
 import testutilStyles from '../test-classes/styles.css.js';
 import styles from './styles.css.js';
@@ -63,7 +63,7 @@ export const Drawer = React.forwardRef(
 
     const regularOpenButton = (
       <TagName ref={openButtonWrapperRef} aria-label={mainLabel} className={styles.toggle} aria-hidden={isOpen}>
-        <AppLayoutButton
+        <ToggleButton
           ref={toggleRefs.toggle}
           className={toggleClassName}
           iconName={iconName}
@@ -109,7 +109,7 @@ export const Drawer = React.forwardRef(
       >
         <div
           style={{ width: drawerContentWidth, top: topOffset, bottom: bottomOffset }}
-          className={clsx(styles['drawer-content'], contentClassName)}
+          className={clsx(styles['drawer-content'], styles['drawer-content-clickable'], contentClassName)}
         >
           {!isMobile && regularOpenButton}
           {resizeHandle}
@@ -145,29 +145,29 @@ export function DrawerTriggersBar({
         [styles['drawer-mobile']]: isMobile,
       })}
     >
-      <div
-        style={{ top: topOffset, bottom: bottomOffset }}
-        className={clsx(styles['drawer-content'], styles['non-interactive'])}
-      >
+      <div style={{ top: topOffset, bottom: bottomOffset }} className={styles['drawer-content']}>
         {!isMobile && (
-          <aside aria-label={drawers?.ariaLabel} className={clsx(styles['drawer-triggers'], contentClassName)}>
+          <aside aria-label={drawers?.ariaLabel} className={clsx(styles['drawer-triggers-wrapper'], contentClassName)}>
             {drawers?.items?.map((item: DrawerItem, index: number) => (
-              <AppLayoutButton
+              <span
+                key={index}
                 className={clsx(
-                  toggleClassName,
-                  styles.trigger,
-                  styles['trigger-drawer'],
-                  drawers.activeDrawerId === item.id && styles.selected
+                  styles['drawer-trigger'],
+                  drawers.activeDrawerId === item.id && styles['drawer-trigger-active']
                 )}
-                key={`drawer-trigger-${index}`}
-                iconName={item.trigger.iconName}
-                iconSvg={item.trigger.iconSvg}
-                ariaLabel={item.ariaLabels?.triggerButton}
                 onClick={() =>
                   drawers.onChange({ activeDrawerId: item.id !== drawers.activeDrawerId ? item.id : undefined })
                 }
-                ariaExpanded={drawers.activeDrawerId !== undefined}
-              />
+              >
+                <ToggleButton
+                  className={toggleClassName}
+                  iconName={item.trigger.iconName}
+                  iconSvg={item.trigger.iconSvg}
+                  ariaLabel={item.ariaLabels?.triggerButton}
+                  ariaExpanded={drawers.activeDrawerId !== undefined}
+                  testId={`awsui-app-layout-trigger-${item.id}`}
+                />
+              </span>
             ))}
           </aside>
         )}

diff --git a/src/app-layout/drawer/interfaces.ts b/src/app-layout/drawer/interfaces.ts
@@ -1,7 +1,6 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 import React from 'react';
-import { ButtonProps } from '../../button/interfaces';
 import { togglesConfig } from '../toggles';
 import { AppLayoutProps } from '../interfaces';
 import { IconProps } from '../../icon/interfaces';
@@ -14,8 +13,8 @@ export interface DesktopDrawerProps {
   toggleClassName: string;
   closeClassName: string;
   toggleRefs: {
-    toggle: React.Ref<ButtonProps.Ref>;
-    close: React.Ref<ButtonProps.Ref>;
+    toggle: React.Ref<{ focus(): void }>;
+    close: React.Ref<{ focus(): void }>;
   };
   width: number;
   topOffset: number | undefined;

diff --git a/src/app-layout/drawer/styles.scss b/src/app-layout/drawer/styles.scss
@@ -3,6 +3,8 @@
  SPDX-License-Identifier: Apache-2.0
 */
 @use '../../internal/styles/tokens' as awsui;
+@use '../../internal/styles' as styles;
+@use '../../internal/hooks/focus-visible' as focus-visible;
 @use '../constants' as constants;
 
 // should be above sticky notifications
@@ -29,7 +31,6 @@ $drawer-z-index-mobile: 1001;
   }
   &-closed {
     min-width: constants.$sidebar-size-closed;
-    cursor: pointer;
     &.drawer-mobile {
       display: none;
     }
@@ -48,13 +49,10 @@ $drawer-z-index-mobile: 1001;
   }
   .drawer-closed > & {
     width: constants.$sidebar-size-closed;
-    &:hover {
-      background: awsui.$color-background-layout-panel-hover;
-    }
-    &.non-interactive {
+    &.drawer-content-clickable {
+      cursor: pointer;
       &:hover {
-        background: awsui.$color-background-layout-panel-content;
-        cursor: default;
+        background: awsui.$color-background-layout-panel-hover;
       }
     }
   }
@@ -63,34 +61,25 @@ $drawer-z-index-mobile: 1001;
   }
 }
 
-.trigger {
-  .drawer-content > .drawer-triggers > & {
-    background: awsui.$color-background-layout-toggle-default;
-    border: 0;
-    border-radius: 0;
-    color: awsui.$color-text-layout-toggle;
-    padding: constants.$drawers-padding-vertical awsui.$space-s;
-    margin-top: 1px;
-
-    &:not(:last-child) {
-      box-shadow: 0px 1px awsui.$color-shadow-layout-toggle;
-    }
-    &-drawer {
-      &:hover:not(.selected) {
-        color: awsui.$color-text-layout-toggle-hover;
-        &:not(:last-child) {
-          box-shadow: 0px 1px awsui.$color-shadow-layout-toggle;
-        }
-      }
-    }
+.drawer-triggers-wrapper {
+  display: flex;
+  flex-direction: column;
+  text-align: center;
+  align-items: stretch;
+}
 
-    &.selected,
-    &.selected:hover {
-      background-color: awsui.$color-background-layout-toggle-selected-default;
-      border-top: 1px solid awsui.$color-background-layout-toggle-selected-default;
-      box-shadow: none;
-      color: awsui.$color-text-layout-toggle-active;
-      margin: 0;
-    }
+.drawer-trigger {
+  padding: constants.$toggle-padding;
+  cursor: pointer;
+  &:not(:first-child) {
+    border-top: 1px solid awsui.$color-border-layout;
+  }
+  &:hover {
+    color: awsui.$color-text-layout-toggle-hover;
+  }
+  &-active,
+  &-active:hover {
+    background-color: awsui.$color-background-layout-toggle-selected-default;
+    color: awsui.$color-text-layout-toggle-active;
   }
 }
diff --git a/src/app-layout/mobile-toolbar/index.tsx b/src/app-layout/mobile-toolbar/index.tsx
@@ -5,7 +5,7 @@ import React, { useEffect } from 'react';
 import { ButtonProps } from '../../button/interfaces';
 import { AppLayoutProps } from '../interfaces';
 import { DrawerItem } from '../drawer/interfaces';
-import { AppLayoutButton, togglesConfig } from '../toggles';
+import { ToggleButton, togglesConfig } from '../toggles';
 import styles from './styles.css.js';
 import sharedStyles from '../styles.css.js';
 import testutilStyles from '../test-classes/styles.css.js';
@@ -23,12 +23,12 @@ const MobileToggle = React.forwardRef(
 
     return (
       <TagName
-        className={clsx(styles['mobile-toggle'])}
+        className={clsx(styles['mobile-toggle'], styles[`mobile-toggle-type-${type}`])}
         aria-hidden={disabled}
         aria-label={mainLabel}
         onClick={e => e.target === e.currentTarget && onClick()}
       >
-        <AppLayoutButton
+        <ToggleButton
           ref={ref}
           className={className}
           iconName={iconName}
@@ -120,22 +120,20 @@ export function MobileToolbar({
       {drawers && (
         <aside
           aria-label={drawers.ariaLabel}
-          className={clsx(
-            styles['mobile-toggle'],
-            styles['mobile-toggle-with-drawers'],
-            testutilStyles['drawers-mobile-triggers-container']
-          )}
+          className={clsx(styles['drawers-container'], testutilStyles['drawers-mobile-triggers-container'])}
         >
           {drawers.items.map((item: DrawerItem, index: number) => (
-            <AppLayoutButton
-              className={clsx(styles['mobile-trigger-with-drawers'], testutilStyles['drawers-trigger'])}
-              key={`drawer-trigger-${index}`}
-              iconName={item.trigger.iconName}
-              iconSvg={item.trigger.iconSvg}
-              ariaLabel={item.ariaLabels?.triggerButton}
-              onClick={() => drawers.onChange({ activeDrawerId: item.id })}
-              ariaExpanded={drawers.activeDrawerId !== undefined}
-            />
+            <span className={clsx(styles['mobile-toggle'], styles['mobile-toggle-type-drawer'])} key={index}>
+              <ToggleButton
+                className={testutilStyles['drawers-trigger']}
+                iconName={item.trigger.iconName}
+                iconSvg={item.trigger.iconSvg}
+                ariaLabel={item.ariaLabels?.triggerButton}
+                onClick={() => drawers.onChange({ activeDrawerId: item.id })}
+                ariaExpanded={drawers.activeDrawerId !== undefined}
+                testId={`awsui-app-layout-trigger-${item.id}`}
+              />
+            </span>
           ))}
         </aside>
       )}