Skip to content

Commit

Permalink
Fix: APP-2881 - Added Disabled prop to Button component (#99)
Browse files Browse the repository at this point in the history
  • Loading branch information
sepehr2github authored Feb 23, 2024
1 parent d95e1c7 commit 3629f33
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 23 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),

### Changed

- Added `disabled` and `isLoading` props to Button component and removed state prop
- Rename `isDisabled` property of input components to `disabled`
- Bump `ip` library from 2.0.0 to 2.0.1

Expand Down
13 changes: 8 additions & 5 deletions src/components/button/button.api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import type { IconType } from '../icon';
export type ButtonVariant = 'primary' | 'secondary' | 'tertiary' | 'success' | 'warning' | 'critical';
export type ButtonContext = 'default' | 'onlyIcon';
export type ButtonSize = 'lg' | 'md' | 'sm';
export type ButtonState = 'disabled' | 'loading';

export interface IButtonBaseProps {
/**
Expand All @@ -22,10 +21,6 @@ export interface IButtonBaseProps {
* Applies responsiveness to the size of the button.
*/
responsiveSize?: ResponsiveAttribute<ButtonSize>;
/**
* State of the button.
*/
state?: ButtonState;
/**
* Icon displayed on the right side of the button. This icon is hidden in case the button has no children element
* set (only-icon variant) and the iconLeft icon is displayed instead.
Expand All @@ -36,6 +31,14 @@ export interface IButtonBaseProps {
* set (only-icon variant)
*/
iconLeft?: IconType;
/**
* A boolean indicating whether the button is loading.
*/
isLoading?: boolean;
/**
* A boolean indicating whether the button is disabled.
*/
disabled?: boolean;
}

export type IButtonElementProps = ButtonHTMLAttributes<HTMLButtonElement> | AnchorHTMLAttributes<HTMLAnchorElement>;
Expand Down
9 changes: 3 additions & 6 deletions src/components/button/button.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,10 @@ describe('<Button /> component', () => {
});

it('renders a loading indicator and hides the icon right and left on loading state', () => {
const state = 'loading';
const iconRight = IconType.CALENDAR;
const iconLeft = IconType.CHECKMARK;
const children = 'Button';
render(createTestComponent({ state, iconLeft, iconRight, children }));
render(createTestComponent({ isLoading: true, iconLeft, iconRight, children }));
expect(screen.getByRole('progressbar')).toBeInTheDocument();
expect(screen.queryByTestId(iconRight)).not.toBeInTheDocument();
expect(screen.queryByTestId(iconLeft)).not.toBeInTheDocument();
Expand All @@ -68,9 +67,8 @@ describe('<Button /> component', () => {
});

it('disables the button on disabled state', () => {
const state = 'disabled';
const onClick = jest.fn();
render(createTestComponent({ state, onClick }));
render(createTestComponent({ disabled: true, onClick }));
const button = screen.getByRole<HTMLButtonElement>('button');
expect(button).toBeDisabled();
expect(button).toHaveAttribute('aria-disabled', 'true');
Expand All @@ -79,10 +77,9 @@ describe('<Button /> component', () => {
});

it('disables the button link on disabled state', () => {
const state = 'disabled';
const onClick = jest.fn();
const href = '/test';
render(createTestComponent({ state, href, onClick }));
render(createTestComponent({ disabled: true, href, onClick }));
const link = screen.getByRole<HTMLAnchorElement>('link');
expect(link).toHaveAttribute('aria-disabled', 'true');
fireEvent.click(link);
Expand Down
17 changes: 9 additions & 8 deletions src/components/button/button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -164,12 +164,13 @@ export const Button = forwardRef<HTMLButtonElement | HTMLAnchorElement, IButtonP
iconLeft,
className,
children,
state,
isLoading,
disabled,
...otherProps
} = props;

const isOnlyIcon = children == null || children === '';
const isDisabled = state === 'disabled' || state === 'loading';
const isDisabled = disabled === true || isLoading === true;
const buttonContext = isOnlyIcon ? 'onlyIcon' : 'default';

const commonClasses = [
Expand All @@ -182,9 +183,9 @@ export const Button = forwardRef<HTMLButtonElement | HTMLAnchorElement, IButtonP
const variantClasses = variantToClassNames[variant].filter((classes) => {
// Do not apply specific state classes when button is on a disabled or loading state. Even though this
// might be done through the tailwind enabled: modifier, it won't work when the button is a link.
if (state === 'disabled') {
if (disabled) {
return !classes.includes('hover');
} else if (state === 'loading') {
} else if (isLoading) {
return !classes.includes('disabled') && !classes.includes('hover') && !classes.includes('active');
}

Expand All @@ -199,7 +200,7 @@ export const Button = forwardRef<HTMLButtonElement | HTMLAnchorElement, IButtonP
);

const classes = classNames(commonClasses, variantClasses, sizeClassNames, contextClassNames, className, {
'cursor-progress': state === 'loading',
'cursor-progress': isLoading,
});

const iconSize = sizeToIconSize[size][buttonContext];
Expand All @@ -220,15 +221,15 @@ export const Button = forwardRef<HTMLButtonElement | HTMLAnchorElement, IButtonP
{},
);

const displayIconLeft = state !== 'loading' && iconLeft != null;
const displayIconRight = state !== 'loading' && iconRight != null && !isOnlyIcon;
const displayIconLeft = !isLoading && iconLeft != null;
const displayIconRight = !isLoading && iconRight != null && !isOnlyIcon;

const commonProps = { className: classes, 'aria-disabled': isDisabled };

const buttonContent = (
<>
{displayIconLeft && <Icon icon={iconLeft} size={iconSize} responsiveSize={iconResponsiveSize} />}
{state === 'loading' && (
{isLoading && (
<Spinner
size={spinnerSize}
responsiveSize={spinnerResponsiveSize}
Expand Down
1 change: 0 additions & 1 deletion src/components/button/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ export { Button } from './button';
export type {
ButtonContext,
ButtonSize,
ButtonState,
ButtonVariant,
IButtonBaseProps,
IButtonElementProps,
Expand Down
2 changes: 1 addition & 1 deletion src/components/dropdown/dropdownContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export const DropdownContainer: React.FC<IDropdownContainerProps> = (props) => {
responsiveSize={responsiveSize}
iconLeft={!hasLabel ? triggerIcon : undefined}
iconRight={hideIcon ? undefined : triggerIcon}
state={disabled ? 'disabled' : undefined}
disabled={disabled}
className={isOpen ? 'border-neutral-300' : undefined}
>
{label}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ export const TextAreaRichTextActions: React.FC<ITextAreaRichTextActionsProps> =
responsiveSize={{ md: 'md' }}
iconLeft={icon}
onClick={action}
state={disabled ? 'disabled' : undefined}
disabled={disabled}
/>
))}
</div>
Expand All @@ -89,7 +89,7 @@ export const TextAreaRichTextActions: React.FC<ITextAreaRichTextActionsProps> =
responsiveSize={{ md: 'md' }}
iconLeft={IconType.EXPAND}
onClick={onExpandClick}
state={disabled ? 'disabled' : undefined}
disabled={disabled}
/>
</div>
);
Expand Down

0 comments on commit 3629f33

Please sign in to comment.