From d35c24af42dcc202625aebcb17aa6bbd7369971b Mon Sep 17 00:00:00 2001
From: Huyen Nguyen <25715018+huyenltnguyen@users.noreply.github.com>
Date: Mon, 18 Apr 2022 15:26:36 +0700
Subject: [PATCH] feat(ui-components): add states and full-width support to
Button component (#45665)
* add active style
* add disable state
* add support for full-width
* add custom focus outline
---
tools/ui-components/src/button/button.css | 27 ------
.../src/button/button.stories.tsx | 22 ++++-
.../ui-components/src/button/button.test.tsx | 31 +++++++
tools/ui-components/src/button/button.tsx | 86 ++++++++++++++++---
tools/ui-components/src/button/types.ts | 2 +
tools/ui-components/src/colors.css | 4 +
tools/ui-components/tailwind.config.js | 21 ++++-
7 files changed, 151 insertions(+), 42 deletions(-)
delete mode 100644 tools/ui-components/src/button/button.css
diff --git a/tools/ui-components/src/button/button.css b/tools/ui-components/src/button/button.css
deleted file mode 100644
index 005eaf52ece..00000000000
--- a/tools/ui-components/src/button/button.css
+++ /dev/null
@@ -1,27 +0,0 @@
-.storybook-button {
- font-family: 'Nunito Sans', 'Helvetica Neue', Helvetica, Arial, sans-serif;
- font-weight: 700;
- cursor: pointer;
- display: inline-block;
- line-height: 1;
-}
-
-.storybook-button--small {
- font-size: 12px;
- padding: 10px 16px;
-}
-.storybook-button--medium {
- font-size: 14px;
- padding: 11px 20px;
-}
-.storybook-button--large {
- font-size: 16px;
- padding: 12px 24px;
-}
-
-.button-default-style {
- @apply bg-default-background-quaternary;
- @apply border-default-foreground-secondary;
- @apply text-default-foreground-secondary;
- @apply border-2;
-}
diff --git a/tools/ui-components/src/button/button.stories.tsx b/tools/ui-components/src/button/button.stories.tsx
index bff461cf7cb..f47e6cbf93d 100644
--- a/tools/ui-components/src/button/button.stories.tsx
+++ b/tools/ui-components/src/button/button.stories.tsx
@@ -8,7 +8,7 @@ const story = {
component: Button,
parameters: {
controls: {
- include: ['children', 'variant', 'size']
+ include: ['children', 'variant', 'size', 'disabled', 'block']
}
},
argTypes: {
@@ -17,6 +17,14 @@ const story = {
},
size: {
options: ['small', 'medium', 'large']
+ },
+ disabled: {
+ options: [true, false],
+ control: { type: 'radio' }
+ },
+ block: {
+ options: [true, false],
+ control: { type: 'radio' }
}
}
};
@@ -54,4 +62,16 @@ Small.args = {
children: 'Button'
};
+export const Disabled = Template.bind({});
+Disabled.args = {
+ children: 'Button',
+ disabled: true
+};
+
+export const FullWidth = Template.bind({});
+FullWidth.args = {
+ children: 'Button',
+ block: true
+};
+
export default story;
diff --git a/tools/ui-components/src/button/button.test.tsx b/tools/ui-components/src/button/button.test.tsx
index a88012339c5..aff4df5472b 100644
--- a/tools/ui-components/src/button/button.test.tsx
+++ b/tools/ui-components/src/button/button.test.tsx
@@ -1,3 +1,7 @@
+// Silence the `jest-dom/prefer-enabled-disabled` rule as the rule looks for the `disabled` attribute
+// while the Button component doesn't use it.
+/* eslint-disable jest-dom/prefer-enabled-disabled */
+
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import React from 'react';
@@ -40,4 +44,31 @@ describe('Button', () => {
expect(onClick).toHaveBeenCalledTimes(1);
});
+
+ it('should reflect the disabled state using the aria-disabled attribute', () => {
+ render();
+
+ const button = screen.getByRole('button', { name: /hello world/i });
+
+ expect(button).toHaveAttribute('aria-disabled', 'true');
+
+ // Ensure that the `disabled` attribute is not used.
+ expect(button).not.toHaveAttribute('disabled', 'true');
+ });
+
+ it('should not trigger the onClick prop if the button is disabled', () => {
+ const onClick = jest.fn();
+
+ render(
+
+ );
+
+ const button = screen.getByRole('button', { name: /hello world/i });
+
+ userEvent.click(button);
+
+ expect(onClick).not.toBeCalled();
+ });
});
diff --git a/tools/ui-components/src/button/button.tsx b/tools/ui-components/src/button/button.tsx
index b0f6ddc5f3f..e85e1200997 100644
--- a/tools/ui-components/src/button/button.tsx
+++ b/tools/ui-components/src/button/button.tsx
@@ -1,17 +1,43 @@
-import React, { useMemo } from 'react';
+import React, { useCallback, useMemo } from 'react';
import { ButtonProps, ButtonSize, ButtonVariant } from './types';
-const defaultClassNames = ['cursor-pointer', 'inline-block', 'border-3'];
+const defaultClassNames = [
+ 'relative',
+ 'cursor-pointer',
+ 'inline-block',
+ 'border-3',
+ 'active:before:w-full',
+ 'active:before:h-full',
+ 'active:before:absolute',
+ 'active:before:inset-0',
+ 'active:before:border-3',
+ 'active:before:border-transparent',
+ 'active:before:bg-gray-900',
+ 'active:before:opacity-20',
+ 'aria-disabled:cursor-not-allowed',
+ 'aria-disabled:opacity-50',
+ 'focus:outline-none', // Hide the default browser outline
+ 'focus:ring',
+ 'focus:ring-focus-outline-color'
+];
const computeClassNames = ({
size,
- variant
+ variant,
+ disabled,
+ block
}: {
size: ButtonSize;
variant: ButtonVariant;
+ disabled?: boolean;
+ block?: boolean;
}) => {
const classNames = [...defaultClassNames];
+ if (block) {
+ classNames.push('block', 'w-full');
+ }
+
// TODO: support 'link' variant
switch (variant) {
case 'danger':
@@ -19,8 +45,12 @@ const computeClassNames = ({
'border-default-foreground-danger',
'bg-default-background-danger',
'text-default-foreground-danger',
- 'hover:bg-default-background-danger-hover',
- 'hover:text-default-foreground-danger-hover'
+ ...(disabled
+ ? []
+ : [
+ 'hover:bg-default-background-danger-hover',
+ 'hover:text-default-foreground-danger-hover'
+ ])
);
break;
case 'info':
@@ -28,8 +58,12 @@ const computeClassNames = ({
'border-default-foreground-info',
'bg-default-background-info',
'text-default-foreground-info',
- 'hover:bg-default-background-info-hover',
- 'hover:text-default-foreground-info-hover'
+ ...(disabled
+ ? []
+ : [
+ 'hover:bg-default-background-info-hover',
+ 'hover:text-default-foreground-info-hover'
+ ])
);
break;
// default variant is 'primary'
@@ -38,8 +72,12 @@ const computeClassNames = ({
'border-default-foreground-secondary',
'bg-default-background-quaternary',
'text-default-foreground-secondary',
- 'hover:bg-default-background-primary-hover',
- 'hover:text-default-foreground-primary-hover'
+ ...(disabled
+ ? []
+ : [
+ 'hover:bg-default-background-primary-hover',
+ 'hover:text-default-foreground-primary-hover'
+ ])
);
}
@@ -65,17 +103,39 @@ export const Button = React.forwardRef(
size = 'medium',
type = 'button',
onClick,
- children
+ children,
+ disabled,
+ block
},
ref
) => {
const classes = useMemo(
- () => computeClassNames({ size, variant }),
- [size, variant]
+ () => computeClassNames({ size, variant, disabled, block }),
+ [size, variant, disabled, block]
+ );
+
+ // Manually prevent click event if the button is disabled
+ // as `aria-disabled` marks the element disabled but still registers the click event.
+ // Ref: https://css-tricks.com/making-disabled-buttons-more-inclusive/#aa-the-difference-between-disabled-and-aria-disabled
+ const handleClick = useCallback(
+ (event: React.MouseEvent) => {
+ const ariaDisabled = event.currentTarget.getAttribute('aria-disabled');
+
+ if (!ariaDisabled && onClick) {
+ onClick(event);
+ }
+ },
+ [onClick]
);
return (
-