fix: Fix visual bugs in drawer toggle buttons

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

Hide file tree

Showing 9 changed files with 101 additions and 120 deletions.
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,28 @@ 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}
+                />
+              </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,19 @@ 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}
+              />
+            </span>
           ))}
         </aside>
       )}

diff --git a/src/app-layout/mobile-toolbar/styles.scss b/src/app-layout/mobile-toolbar/styles.scss
@@ -12,7 +12,6 @@
 }
 
 .mobile-bar {
-  /* stylelint-disable-next-line plugin/no-unsupported-browser-features */
   position: sticky;
   display: flex;
   align-items: center;
@@ -32,38 +31,25 @@
   margin-right: awsui.$space-m;
 }
 
-.mobile-trigger-with-drawers {
-  width: constants.$sidebar-size-closed;
-  display: flex;
-  justify-content: center;
-}
-
 .mobile-toggle {
   box-sizing: border-box;
   cursor: pointer;
   z-index: 1;
   padding: constants.$toggle-padding;
   width: constants.$sidebar-size-closed;
-  &:first-child {
+  &-type-navigation {
     border-right: 1px solid awsui.$color-border-layout;
   }
-  &:last-child:not(.mobile-toggle-with-drawers) {
+  &-type-tools,
+  &-type-drawer {
     border-left: 1px solid awsui.$color-border-layout;
   }
-  &:hover:not(.mobile-toggle-with-drawers) {
+  &:hover {
     background: awsui.$color-background-layout-panel-hover;
   }
+}
 
-  &-with-drawers {
-    display: flex;
-    width: auto;
-    padding: 0;
-    height: 100%;
-
-    > .mobile-trigger-with-drawers {
-      border-left: 1px solid awsui.$color-border-layout;
-      display: flex;
-      align-items: center;
-    }
-  }
+.drawers-container {
+  display: flex;
+  align-items: stretch;
 }
diff --git a/src/app-layout/toggles/index.tsx b/src/app-layout/toggles/index.tsx
@@ -1,8 +1,10 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
+import clsx from 'clsx';
 import React from 'react';
 import { ButtonProps } from '../../button/interfaces';
 import { InternalButton } from '../../button/internal';
+import InternalIcon from '../../icon/internal';
 import { AppLayoutProps } from '../interfaces';
 import { AppLayoutButtonProps } from './interfaces';
 import styles from './styles.css.js';
@@ -28,25 +30,24 @@ export const togglesConfig = {
   },
 } as const;
 
-export const AppLayoutButton = React.forwardRef(
+export const ToggleButton = React.forwardRef(
   (
     { className, ariaLabel, ariaExpanded, iconName, iconSvg, disabled, onClick }: AppLayoutButtonProps,
-    ref: React.Ref<ButtonProps.Ref>
+    ref: React.Ref<{ focus(): void }>
   ) => {
     return (
-      <InternalButton
-        ref={ref}
-        className={className}
-        ariaLabel={ariaLabel}
-        variant="icon"
-        formAction="none"
+      <button
+        ref={ref as React.Ref<HTMLButtonElement>}
+        className={clsx(className, styles['toggle-button'])}
+        aria-label={ariaLabel}
+        type="button"
         onClick={onClick}
-        iconName={iconName}
-        iconSvg={iconSvg}
         disabled={disabled}
-        ariaExpanded={ariaExpanded ? undefined : false}
-        __nativeAttributes={{ 'aria-haspopup': ariaExpanded ? undefined : true }}
-      />
+        aria-expanded={ariaExpanded ? undefined : false}
+        aria-haspopup={ariaExpanded ? undefined : true}
+      >
+        <InternalIcon svg={iconSvg} name={iconName} />
+      </button>
     );
   }
 );
@@ -61,13 +62,15 @@ export const CloseButton = React.forwardRef(
   ({ className, ariaLabel, onClick }: CloseButtonProps, ref: React.Ref<ButtonProps.Ref>) => {
     return (
       <span className={styles['close-button']}>
-        <AppLayoutButton
+        <InternalButton
           ref={ref}
           className={className}
-          ariaExpanded={true}
           ariaLabel={ariaLabel}
+          variant="icon"
+          formAction="none"
           iconName="close"
           onClick={onClick}
+          __nativeAttributes={{ 'aria-haspopup': true }}
         />
       </span>
     );

diff --git a/src/app-layout/toggles/interfaces.ts b/src/app-layout/toggles/interfaces.ts
@@ -1,5 +1,6 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
+import React from 'react';
 import { IconProps } from '../../icon/interfaces';
 
 export interface AppLayoutButtonProps {
@@ -8,6 +9,6 @@ export interface AppLayoutButtonProps {
   ariaExpanded?: boolean;
   iconName?: IconProps.Name;
   iconSvg?: React.ReactNode;
-  onClick: () => void;
+  onClick?: () => void;
   disabled?: boolean;
 }