fix: button: ensure loading dots use text color and enable two state …

…loader (#636) * fix: button: ensure loading dots use text color and enable two state loader * chore: loader: address pr feedback by removing unneeded rtl code
EightfoldAI · Jun 12, 2023 · 94f7759 · 94f7759
1 parent a07fb0d
commit 94f7759
Show file tree

Hide file tree

Showing 8 changed files with 96 additions and 17 deletions.
diff --git a/src/components/Button/Button.tsx b/src/components/Button/Button.tsx
@@ -258,7 +258,13 @@ export const Button: FC<ButtonProps> = React.forwardRef(
     };
 
     const getButtonLoader = (): JSX.Element =>
-      loading && <Loader classNames={styles.loader} size={getLoaderSize()} />;
+      loading && (
+        <Loader
+          classNames={styles.loader}
+          dotClassNames={styles.loaderDot}
+          size={getLoaderSize()}
+        />
+      );
 
     const getButtonIcon = (): JSX.Element => (
       <Icon

diff --git a/src/components/Button/TwoStateButton/TwoStateButton.stories.tsx b/src/components/Button/TwoStateButton/TwoStateButton.stories.tsx
@@ -102,6 +102,7 @@ Two_State_Button.args = {
     'data-test-id': 'myTwoStateButtonIconTwoTestId',
   },
   id: 'myTwoStateButton',
+  loading: false,
   shape: ButtonShape.Pill,
   size: ButtonSize.Medium,
   style: {},

diff --git a/src/components/Button/TwoStateButton/TwoStateButton.tsx b/src/components/Button/TwoStateButton/TwoStateButton.tsx
@@ -13,6 +13,7 @@ import {
 import { Badge } from '../../Badge';
 import { Icon, IconSize } from '../../Icon';
 import { InnerNudge, NudgeAnimation, NudgeProps } from '../Nudge';
+import { Loader, LoaderSize } from '../../Loader';
 import { Breakpoints, useMatchMedia } from '../../../hooks/useMatchMedia';
 import { mergeClasses } from '../../../shared/utilities';
 import { useCanvasDirection } from '../../../hooks/useCanvasDirection';
@@ -41,6 +42,7 @@ export const TwoStateButton: FC<TwoStateButtonProps> = React.forwardRef(
       iconOneProps,
       iconTwoProps,
       id,
+      loading = false,
       nudgeProps: defaultNudgeProps,
       onClick,
       shape = ButtonShape.Pill,
@@ -126,6 +128,7 @@ export const TwoStateButton: FC<TwoStateButtonProps> = React.forwardRef(
       { [styles.left]: alignText === ButtonTextAlign.Left },
       { [styles.right]: alignText === ButtonTextAlign.Right },
       { [styles.disabled]: allowDisabledFocus || mergedDisabled },
+      { [styles.loading]: loading },
       { [styles.buttonRtl]: htmlDir === 'rtl' },
     ]);
 
@@ -177,6 +180,36 @@ export const TwoStateButton: FC<TwoStateButtonProps> = React.forwardRef(
       return iconSize;
     };
 
+    const getLoaderSize = (): LoaderSize => {
+      let loaderSize: LoaderSize;
+      if (size === ButtonSize.Flex && largeScreenActive) {
+        loaderSize = LoaderSize.Small;
+      } else if (
+        size === ButtonSize.Flex &&
+        (mediumScreenActive || smallScreenActive)
+      ) {
+        loaderSize = LoaderSize.Medium;
+      } else if (size === ButtonSize.Flex && xSmallScreenActive) {
+        loaderSize = LoaderSize.Large;
+      } else if (size === ButtonSize.Large) {
+        loaderSize = LoaderSize.Large;
+      } else if (size === ButtonSize.Medium) {
+        loaderSize = LoaderSize.Medium;
+      } else if (size === ButtonSize.Small) {
+        loaderSize = LoaderSize.Small;
+      }
+      return loaderSize;
+    };
+
+    const getButtonLoader = (): JSX.Element =>
+      loading && (
+        <Loader
+          classNames={styles.loader}
+          dotClassNames={styles.loaderDot}
+          size={getLoaderSize()}
+        />
+      );
+
     const getButtonIcon = (position: string): JSX.Element => (
       <Icon
         {...(position === 'left' ? iconOneProps : iconTwoProps)}
@@ -197,11 +230,11 @@ export const TwoStateButton: FC<TwoStateButtonProps> = React.forwardRef(
         {...rest}
         ref={mergedRef}
         aria-checked={toggle ? !!checked : undefined}
-        aria-disabled={mergedDisabled}
+        aria-disabled={mergedDisabled || loading}
         aria-label={ariaLabel}
         aria-pressed={toggle ? !!checked : undefined}
         defaultChecked={checked}
-        disabled={!allowDisabledFocus && mergedDisabled}
+        disabled={(!allowDisabledFocus && mergedDisabled) || loading}
         className={twoStateButtonClassNames}
         id={id}
         onClick={!allowDisabledFocus ? onClick : null}
@@ -228,6 +261,7 @@ export const TwoStateButton: FC<TwoStateButtonProps> = React.forwardRef(
             {counterExists && (
               <Badge classNames={badgeClassNames}>{counter}</Badge>
             )}
+            {getButtonLoader()}
           </span>
           <span className={styles.column}>
             {iconTwoExists && getButtonIcon('right')}

diff --git a/src/components/Button/__snapshots__/Button.test.tsx.snap b/src/components/Button/__snapshots__/Button.test.tsx.snap
@@ -46,13 +46,13 @@ exports[`Button Button is loading 1`] = `
       class="loader-container loader"
     >
       <div
-        class="dot medium"
+        class="loader-dot dot medium"
       />
       <div
-        class="dot medium"
+        class="loader-dot dot medium"
       />
       <div
-        class="dot medium"
+        class="loader-dot dot medium"
       />
     </div>
   </button>

diff --git a/src/components/Button/button.module.scss b/src/components/Button/button.module.scss
@@ -510,6 +510,10 @@
     }
   }
 
+  .loader-dot {
+    background: var(--button-primary-text-color);
+  }
+
   &:hover:not([disabled]) {
     color: var(--button-primary-hover-text-color);
     background: var(--button-primary-hover-background-color);
@@ -540,6 +544,10 @@
   border-style: var(--button-secondary-border-style);
   border-color: var(--button-secondary-border-color);
 
+  .loader-dot {
+    background: var(--button-secondary-text-color);
+  }
+
   &:hover:not([disabled]) {
     color: var(--button-secondary-hover-text-color);
     background: var(--button-secondary-hover-background-color);
@@ -623,6 +631,10 @@
   border-style: var(--button-primary-disruptive-border-style);
   border-color: var(--button-primary-disruptive-border-color);
 
+  .loader-dot {
+    background: var(--button-primary-disruptive-text-color);
+  }
+
   &:hover:not([disabled]) {
     color: var(--button-primary-disruptive-hover-text-color);
     background: var(--button-primary-disruptive-hover-background-color);
@@ -658,6 +670,10 @@
   border-style: var(--button-secondary-disruptive-border-style);
   border-color: var(--button-secondary-disruptive-border-color);
 
+  .loader-dot {
+    background: var(--button-secondary-disruptive-text-color);
+  }
+
   &:hover:not([disabled]) {
     color: var(--button-secondary-disruptive-hover-text-color);
     background: var(--button-secondary-disruptive-hover-background-color);
@@ -688,6 +704,10 @@
   border-style: var(--button-default-border-style);
   border-color: var(--button-default-border-color);
 
+  .loader-dot {
+    background: var(--button-default-text-color);
+  }
+
   &.button-conic {
     border-color: transparent;
   }
@@ -737,6 +757,10 @@
   border-style: var(--button-default-disruptive-border-style);
   border-color: var(--button-default-disruptive-border-color);
 
+  .loader-dot {
+    background: var(--button-default-disruptive-text-color);
+  }
+
   &:hover:not([disabled]) {
     color: var(--button-default-disruptive-hover-text-color);
     background: var(--button-default-disruptive-hover-background-color);
@@ -764,6 +788,10 @@
   color: var(--color);
   background: var(--bg);
 
+  .loader-dot {
+    background: var(--color);
+  }
+
   &:hover:not([disabled]) {
     --bg: var(--button-neutral-hover-background-color);
     --color: var(--button-neutral-hover-text-color);
@@ -784,6 +812,10 @@
   --color: var(--button-system-ui-text-color);
   color: var(--color);
 
+  .loader-dot {
+    background: var(--color);
+  }
+
   .inner-nudge {
     &.background {
       &:after {
@@ -947,9 +979,17 @@
 
   --bg: var(--button-two-state-background-color);
   --color: var(--button-two-state-text-color);
-  background-color: var(--bg);
+  background: var(--bg);
   color: var(--color);
 
+  .loader {
+    background: var(--button-two-state-background-color);
+  }
+
+  .loader-dot {
+    background: var(--color);
+  }
+
   .counter {
     background-color: var(--button-counter-default-background-color);
     display: inline-block;

diff --git a/src/components/Loader/Loader.tsx b/src/components/Loader/Loader.tsx
@@ -1,24 +1,23 @@
 import React, { FC } from 'react';
 import { LoaderProps, LoaderSize } from './Loader.types';
 import { mergeClasses } from '../../shared/utilities';
-import { useCanvasDirection } from '../../hooks/useCanvasDirection';
 
 import styles from './loader.module.scss';
 
 export const Loader: FC<LoaderProps> = ({
-  size = LoaderSize.Small,
   classNames,
+  dotClassNames,
+  size = LoaderSize.Small,
   ...rest
 }) => {
-  const htmlDir: string = useCanvasDirection();
   const dotClasses = mergeClasses([
+    dotClassNames,
     styles.dot,
     {
       [styles.small]: size === LoaderSize.Small,
       [styles.medium]: size === LoaderSize.Medium,
       [styles.large]: size === LoaderSize.Large,
     },
-    { [styles.dotRtl]: htmlDir === 'rtl' },
   ]);
   return (
     <div

diff --git a/src/components/Loader/Loader.types.ts b/src/components/Loader/Loader.types.ts
@@ -7,6 +7,10 @@ export enum LoaderSize {
 }
 
 export interface LoaderProps extends OcBaseProps<HTMLDivElement> {
+  /**
+   * Custom dot class names.
+   */
+  dotClassNames?: string;
   /**
    * The size of the loader.
    * @default LoaderSize.Small

diff --git a/src/components/Loader/loader.module.scss b/src/components/Loader/loader.module.scss
@@ -5,7 +5,7 @@
     --translate: -2px;
     background: var(--primary-color);
     border-radius: 50%;
-    margin-right: $space-xxs;
+    margin: 0 $space-xxxs;
 
     &.small {
       width: 4px;
@@ -35,11 +35,6 @@
     &:nth-child(3) {
       animation: 2s linear infinite both thirdBall;
     }
-
-    &-rtl {
-      margin-left: $space-xxs;
-      margin-right: unset;
-    }
   }
 }