Skip to content

Commit

Permalink
Remove disabled attribute from button (#412)
Browse files Browse the repository at this point in the history 
* Remove disabled attribute from button, add aria-disabled, prevent onClick if disabled is true

* merge unit tests
  • Loading branch information
@MateuszRomek
MateuszRomek authored Sep 8, 2022
1 parent eef1c67 commit 89db30f
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 56 deletions.
106 changes: 53 additions & 53 deletions packages/react-components/src/components/Button/Button.module.scss
View file
Original file line number Diff line number Diff line change
@@ -1,24 +1,24 @@
$base-class: 'btn';

.#{$base-class} {
display: inline-flex;
position: relative;
align-items: center;
border-radius: 4px;
border-style: solid;
justify-content: center;
transition-duration: 200ms;
transition-property: opacity, border, color, background-color, box-shadow;
transition-timing-function: cubic-bezier(0.64, 0, 0.35, 1);
border-width: 1px;
border-style: solid;
border-radius: 4px;
cursor: pointer;
display: inline-flex;
font-size: 15px;
font-weight: 600;
height: 36px;
justify-content: center;
min-width: 36px;
padding: 8px 16px;
position: relative;
min-width: 36px;
height: 36px;
text-align: center;
text-decoration: none;
transition-duration: 200ms;
transition-property: opacity, border, color, background-color, box-shadow;
transition-timing-function: cubic-bezier(0.64, 0, 0.35, 1);
font-size: 15px;
font-weight: 600;
user-select: none;

&:focus,
Expand All @@ -28,7 +28,7 @@ $base-class: 'btn';
}

&:focus {
box-shadow: 0 0 1px 2px rgba(var(--color-action-default-rgb), 0.7);
box-shadow: 0 0 1px 2px rgb(var(--color-action-default-rgb) 0.7);
}

&--disabled {
Expand All @@ -49,8 +49,8 @@ $base-class: 'btn';
}

&--basic {
background-color: var(--surface-basic-default);
border-color: var(--color-action-default);
background-color: var(--surface-basic-default);
color: var(--color-action-default);

&:hover {
Expand All @@ -63,92 +63,92 @@ $base-class: 'btn';
color: var(--color-action-hover);
}

&[disabled] {
background-color: inherit;
&[aria-disabled='true'] {
border-color: var(--color-action-disabled);
background-color: inherit;
color: var(--color-action-disabled);
}
}

&--primary {
background-color: var(--color-action-default);
border-color: var(--color-action-default);
background-color: var(--color-action-default);
color: var(--content-invert-default);

&:hover {
background-color: var(--color-action-hover);
border-color: var(--color-action-hover);
background-color: var(--color-action-hover);
}

&:active {
background-color: var(--color-action-active);
border-color: var(--color-action-active);
background-color: var(--color-action-active);
}

&[disabled] {
background-color: var(--color-action-disabled);
&[aria-disabled='true'] {
border-color: var(--color-action-disabled);
background-color: var(--color-action-disabled);
}
}

&--secondary {
background-color: var(--surface-basic-default);
border-color: var(--surface-secondary-default);
background-color: var(--surface-basic-default);
color: var(--content-default);

&:hover {
background-color: var(--surface-basic-hover);
border-color: var(--surface-secondary-hover);
background-color: var(--surface-basic-hover);
}

&:active {
background-color: var(--surface-feedback-info);
border-color: var(--color-action-default);
background-color: var(--surface-feedback-info);
color: var(--color-action-default);
}

&[disabled] {
background-color: inherit;
&[aria-disabled='true'] {
border-color: var(--border-disabled);
background-color: inherit;
color: var(--content-disabled);
}
}

&--destructive {
background-color: var(--color-negative-default);
border-color: var(--color-negative-default);
background-color: var(--color-negative-default);
color: var(--content-invert-default);

&:hover {
background-color: var(--color-negative-hover);
border-color: var(--color-negative-hover);
background-color: var(--color-negative-hover);
}

&:active {
background-color: var(--color-negative-active);
border-color: var(--color-negative-active);
background-color: var(--color-negative-active);
}

&[disabled] {
background-color: var(--color-negative-disabled);
&[aria-disabled='true'] {
border-color: var(--color-negative-disabled);
background-color: var(--color-negative-disabled);
}
}

&--compact {
height: 32px;
min-width: 32px;
padding: 6px 16px;
min-width: 32px;
height: 32px;

&.#{$base-class}--icon-only {
padding: 4px;
}
}

&--large {
height: 42px;
min-width: 42px;
padding: 11px 16px;
min-width: 42px;
height: 42px;

&.#{$base-class}--icon-only {
padding: 9px;
Expand All @@ -158,15 +158,15 @@ $base-class: 'btn';
&--text,
&--plain,
&--plain-light {
background-color: transparent;
border-color: transparent;
background-color: transparent;
color: var(--color-action-default);

&:hover {
color: var(--color-action-hover);
}

&[disabled] {
&[aria-disabled='true'] {
color: var(--color-action-disabled);
}
}
Expand All @@ -182,9 +182,9 @@ $base-class: 'btn';
&--plain,
&--plain-light {
border: 0;
height: auto;
min-width: auto;
padding: 0;
min-width: auto;
height: auto;

&:hover,
&:focus {
Expand All @@ -198,34 +198,34 @@ $base-class: 'btn';
}

&--loading {
cursor: progress;
transition: all 0.2s cubic-bezier(0.64, 0, 0.35, 1);
cursor: progress;

> * {
opacity: 0;
}

.#{$base-class}__loader {
align-items: center;
box-sizing: border-box;
display: flex;
height: 100%;
justify-content: center;
position: absolute;
top: 0;
left: 0;
align-items: center;
justify-content: center;
opacity: 1;
padding: inherit;
position: absolute;
top: 0;
width: 100%;
height: 100%;

> * {
color: inherit;
font-weight: inherit;
margin: 0 5px;
overflow: hidden;
text-align: left;
text-overflow: ellipsis;
white-space: nowrap;
color: inherit;
font-weight: inherit;
}
}
}
Expand All @@ -236,22 +236,22 @@ $base-class: 'btn';
}

&__icon {
color: inherit;
display: inline-block;
width: 20px;
height: 20px;
line-height: 1;
width: 20px;
color: inherit;

&--left {
margin-left: -4px;
margin-right: 5px;
order: -1;
margin-right: 5px;
margin-left: -4px;
}

&--right {
margin-left: 5px;
margin-right: -4px;
order: 1;
margin-right: -4px;
margin-left: 5px;
}
}
}
14 changes: 12 additions & 2 deletions packages/react-components/src/components/Button/Button.spec.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as React from 'react';

import { render } from 'test-utils';
import { render, fireEvent, vi } from 'test-utils';
import { Icon } from '../Icon';

import { Button } from './Button';
Expand All @@ -9,7 +9,6 @@ import styles from './Button.module.scss';
describe('<Button> component', () => {
function renderButton(props = {}) {
const result = render(<Button {...props} />);

return {
...result,
btnEl: result.container.firstChild,
Expand Down Expand Up @@ -107,4 +106,15 @@ describe('<Button> component', () => {
expect((btnEl as HTMLElement).tagName).toBe('BUTTON');
expect((linkEl as HTMLElement).tagName).toBe('A');
});

it('should set aria-disabled if we pass disabled prop and should not fire onClick callback if disabled is true', () => {
const onClick = vi.fn();
const { btnEl } = renderButton({ disabled: true, onClick });

expect(btnEl).toHaveAttribute('aria-disabled', 'true');

fireEvent.click(btnEl as Element);

expect(onClick).not.toHaveBeenCalled();
});
});
4 changes: 3 additions & 1 deletion packages/react-components/src/components/Button/Button.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export const Button: React.FC<ButtonProps> = ({
className,
children,
href,
onClick,
...props
}) => {
const isDisabled = loading || disabled;
Expand All @@ -63,9 +64,10 @@ export const Button: React.FC<ButtonProps> = ({
return (
<Component
className={mergedClassNames}
disabled={isDisabled}
aria-disabled={isDisabled}
type={type}
href={isDisabled ? undefined : href}
onClick={isDisabled ? undefined : onClick}
{...props}
>
{loading && (
Expand Down

0 comments on commit 89db30f

Please sign in to comment.