From 5d05b8de0eff7c275032499e046b276cacb9eba2 Mon Sep 17 00:00:00 2001 From: Dylan Kilgore Date: Tue, 6 Jun 2023 13:56:12 -0700 Subject: [PATCH 1/2] fix: button: ensure loading dots use text color and enable two state loader --- src/components/Button/Button.tsx | 8 +++- .../TwoStateButton/TwoStateButton.stories.tsx | 1 + .../Button/TwoStateButton/TwoStateButton.tsx | 38 ++++++++++++++++- .../Button/__snapshots__/Button.test.tsx.snap | 6 +-- src/components/Button/button.module.scss | 42 ++++++++++++++++++- src/components/Loader/Loader.tsx | 4 +- src/components/Loader/Loader.types.ts | 4 ++ src/components/Loader/loader.module.scss | 5 +-- 8 files changed, 97 insertions(+), 11 deletions(-) diff --git a/src/components/Button/Button.tsx b/src/components/Button/Button.tsx index b2b74a267..50b28813a 100644 --- a/src/components/Button/Button.tsx +++ b/src/components/Button/Button.tsx @@ -258,7 +258,13 @@ export const Button: FC = React.forwardRef( }; const getButtonLoader = (): JSX.Element => - loading && ; + loading && ( + + ); const getButtonIcon = (): JSX.Element => ( = React.forwardRef( iconOneProps, iconTwoProps, id, + loading = false, nudgeProps: defaultNudgeProps, onClick, shape = ButtonShape.Pill, @@ -126,6 +128,7 @@ export const TwoStateButton: FC = 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 = 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 && ( + + ); + const getButtonIcon = (position: string): JSX.Element => ( = 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 = React.forwardRef( {counterExists && ( {counter} )} + {getButtonLoader()} {iconTwoExists && getButtonIcon('right')} diff --git a/src/components/Button/__snapshots__/Button.test.tsx.snap b/src/components/Button/__snapshots__/Button.test.tsx.snap index 4cdb68b4f..2fa8161f7 100644 --- 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" >
diff --git a/src/components/Button/button.module.scss b/src/components/Button/button.module.scss index abf77838f..90b6b00b1 100644 --- 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 index 0aa79652b..8156d10ac 100644 --- a/src/components/Loader/Loader.tsx +++ b/src/components/Loader/Loader.tsx @@ -6,12 +6,14 @@ import { useCanvasDirection } from '../../hooks/useCanvasDirection'; import styles from './loader.module.scss'; export const Loader: FC = ({ - size = LoaderSize.Small, classNames, + dotClassNames, + size = LoaderSize.Small, ...rest }) => { const htmlDir: string = useCanvasDirection(); const dotClasses = mergeClasses([ + dotClassNames, styles.dot, { [styles.small]: size === LoaderSize.Small, diff --git a/src/components/Loader/Loader.types.ts b/src/components/Loader/Loader.types.ts index 770670b5e..748d65293 100644 --- 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 { + /** + * 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 index fca6ff106..c8eac4ac5 100644 --- 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; @@ -37,8 +37,7 @@ } &-rtl { - margin-left: $space-xxs; - margin-right: unset; + margin: 0 $space-xxxs; } } } From f58ad384a5c5a93d747d45712b9575701e53da8e Mon Sep 17 00:00:00 2001 From: Dylan Kilgore Date: Thu, 8 Jun 2023 16:09:52 -0700 Subject: [PATCH 2/2] chore: loader: address pr feedback by removing unneeded rtl code --- src/components/Loader/Loader.tsx | 3 --- src/components/Loader/loader.module.scss | 4 ---- 2 files changed, 7 deletions(-) diff --git a/src/components/Loader/Loader.tsx b/src/components/Loader/Loader.tsx index 8156d10ac..da9c76303 100644 --- a/src/components/Loader/Loader.tsx +++ b/src/components/Loader/Loader.tsx @@ -1,7 +1,6 @@ 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'; @@ -11,7 +10,6 @@ export const Loader: FC = ({ size = LoaderSize.Small, ...rest }) => { - const htmlDir: string = useCanvasDirection(); const dotClasses = mergeClasses([ dotClassNames, styles.dot, @@ -20,7 +18,6 @@ export const Loader: FC = ({ [styles.medium]: size === LoaderSize.Medium, [styles.large]: size === LoaderSize.Large, }, - { [styles.dotRtl]: htmlDir === 'rtl' }, ]); return (