Skip to content

Commit e38de0e

Browse files
ethanashawCopilot
andauthored
feat(ActionButton): use button component for action button (#1358)
Co-authored-by: Copilot <copilot@github.com>
1 parent fcc85c6 commit e38de0e

6 files changed

Lines changed: 35 additions & 62 deletions

File tree

src/components/ActionButton/ActionButton.tsx

Lines changed: 14 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
import classNames from "classnames";
2-
import React, { MouseEventHandler, useEffect, useRef, useState } from "react";
3-
import type { ButtonHTMLAttributes, ReactNode } from "react";
2+
import React, { useEffect, useRef, useState } from "react";
43

54
import type { ButtonProps } from "../Button";
65
import Icon from "../Icon";
76

8-
import type { ClassName, PropsWithSpread } from "types";
7+
import type { PropsWithSpread } from "types";
8+
import Button from "../Button";
99

1010
export const LOADER_MIN_DURATION = 400; // minimium duration (ms) loader displays
1111
export const SUCCESS_DURATION = 2000; // duration (ms) success tick is displayed
@@ -17,41 +17,16 @@ export enum Label {
1717

1818
export type Props = PropsWithSpread<
1919
{
20-
/**
21-
* The appearance of the button.
22-
*/
23-
appearance?: ButtonProps["appearance"];
24-
/**
25-
* The content of the button.
26-
*/
27-
children?: ReactNode;
28-
/**
29-
* Optional class(es) to pass to the button element.
30-
*/
31-
className?: ClassName;
32-
/**
33-
* Whether the button should be disabled.
34-
*/
35-
disabled?: boolean;
36-
/**
37-
* Whether the button should display inline.
38-
*/
39-
inline?: boolean;
4020
/**
4121
* Whether the button should be in the loading state.
4222
*/
4323
loading?: boolean;
44-
/**
45-
* Function for handling button click event.
46-
*/
47-
onClick?: MouseEventHandler<HTMLButtonElement>;
4824
/**
4925
* Whether the button should be in the success state.
5026
*/
51-
5227
success?: boolean;
5328
},
54-
ButtonHTMLAttributes<HTMLButtonElement>
29+
ButtonProps
5530
>;
5631

5732
/**
@@ -62,12 +37,9 @@ export type Props = PropsWithSpread<
6237
* props table:
6338
*/
6439
const ActionButton = ({
65-
appearance,
6640
children,
6741
className,
68-
onClick,
6942
disabled = null,
70-
inline = false,
7143
loading = false,
7244
success = false,
7345
...buttonProps
@@ -150,33 +122,20 @@ const ActionButton = ({
150122
return () => window.clearTimeout(successTimeout);
151123
}, [showSuccess]);
152124

153-
const buttonClasses = classNames(
154-
className,
155-
"p-action-button",
156-
appearance ? `p-button--${appearance}` : "p-button",
157-
{
158-
"is-processing": showLoader || showSuccess,
159-
"is-disabled": disabled === null ? showLoader : disabled,
160-
"is-inline": inline,
161-
},
162-
);
125+
const buttonClasses = classNames(className, "p-action-button", {
126+
"is-processing": showLoader || showSuccess,
127+
});
163128
const showIcon = showLoader || showSuccess;
164-
const isDisabled = disabled === null ? showLoader : disabled;
165129
const icon = (showLoader && "spinner") || (showSuccess && "success") || null;
166-
const iconLight = appearance === "positive" || appearance === "negative";
167-
const onClickDisabled: MouseEventHandler<HTMLButtonElement> = (e) =>
168-
e.preventDefault();
169-
170-
// This component uses the base button element instead of the Button component
171-
// as the button requires a ref and Button would have to be updated to use
172-
// forwardRef which is not currently supported by components that use
173-
// typescript generics.
130+
const iconLight =
131+
buttonProps.appearance === "positive" ||
132+
buttonProps.appearance === "negative";
133+
174134
return (
175-
<button
135+
<Button
176136
className={buttonClasses}
177137
ref={ref}
178-
onClick={isDisabled ? onClickDisabled : onClick}
179-
aria-disabled={isDisabled || undefined}
138+
disabled={disabled === null ? showLoader : disabled}
180139
style={
181140
height && width
182141
? {
@@ -197,7 +156,7 @@ const ActionButton = ({
197156
) : (
198157
children
199158
)}
200-
</button>
159+
</Button>
201160
);
202161
};
203162

src/components/ActionButton/__snapshots__/ActionButton.test.tsx.snap

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

33
exports[`ActionButton matches loading snapshot 1`] = `
44
<button
55
aria-disabled="true"
6-
class="p-action-button p-button is-processing is-disabled"
6+
class="p-button is-disabled p-action-button is-processing"
77
>
88
<i
99
aria-label="Waiting for action to complete"
@@ -14,7 +14,7 @@ exports[`ActionButton matches loading snapshot 1`] = `
1414

1515
exports[`ActionButton matches success snapshot 1`] = `
1616
<button
17-
class="p-action-button p-button is-processing"
17+
class="p-button p-action-button is-processing"
1818
>
1919
<i
2020
aria-label="Action completed"

src/components/Button/Button.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import type {
66
ElementType,
77
MouseEventHandler,
88
ReactNode,
9+
Ref,
910
} from "react";
1011

1112
import type { ClassName, ValueOf } from "types";
@@ -64,6 +65,10 @@ export type Props<P = null> = {
6465
* Whether the button should be small.
6566
*/
6667
small?: boolean;
68+
/**
69+
* A ref to the button.
70+
*/
71+
ref?: Ref<HTMLButtonElement>;
6772
} & (Omit<ButtonHTMLAttributes<HTMLButtonElement>, "onClick"> | P);
6873

6974
/**

src/components/ConfirmationModal/ConfirmationModal.stories.tsx

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { Meta, StoryObj } from "@storybook/react";
44

55
import ConfirmationModal from "./ConfirmationModal";
66
import Input from "../Input";
7+
import Icon from "components/Icon";
78

89
const doNothing = () => {};
910

@@ -30,9 +31,17 @@ export const Default: Story = {
3031
{modalOpen ? (
3132
<ConfirmationModal
3233
title="Confirm delete"
33-
confirmButtonLabel="Delete"
34+
confirmButtonLabel={
35+
<>
36+
<Icon name="delete" light />
37+
<span>Delete</span>
38+
</>
39+
}
3440
onConfirm={doNothing}
3541
close={closeHandler}
42+
confirmButtonProps={{
43+
hasIcon: true,
44+
}}
3645
>
3746
<p>
3847
This will permanently delete the user "Simon".

src/components/Modal/Modal.stories.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ export const Focus: Story = {
8585
render: ({ closeOnOutsideClick }) => {
8686
/* eslint-disable react-hooks/rules-of-hooks */
8787
const [modalOpen, setModalOpen] = useState(true);
88-
const buttonRef = useRef<HTMLElement>(null);
88+
const buttonRef = useRef<HTMLButtonElement>(null);
8989
/* eslint-enable react-hooks/rules-of-hooks */
9090

9191
const closeHandler = () => setModalOpen(false);

src/components/SummaryButton/__snapshots__/SummaryButton.test.tsx.snap

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

33
exports[`<SummaryButton /> renders and matches the snapshot 1`] = `
44
<small>
@@ -8,7 +8,7 @@ exports[`<SummaryButton /> renders and matches the snapshot 1`] = `
88
Showing some items
99
</span>
1010
<button
11-
class="is-small is-dense is-inline p-action-button p-button"
11+
class="p-button is-small is-dense is-inline p-action-button"
1212
>
1313
Show more
1414
</button>

0 commit comments

Comments
 (0)