Skip to content

fix(Button): state props should override Action state #1720

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 9, 2025
Merged

fix(Button): state props should override Action state #1720

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 82 additions & 19 deletions packages/components/src/components/Action/Action.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { render, screen } from "@testing-library/react";
import React, { act } from "react";
import { act, type FC } from "react";
import Action from "@/components/Action";
import { Button } from "@/components/Button";
import { Button, type ButtonProps } from "@/components/Button";
import type { Mock } from "vitest";
import userEvent from "@/lib/dev/vitestUserEvent";

Expand Down Expand Up @@ -44,7 +44,10 @@ afterEach(() => {
vitest.useRealTimers();
});

const button = <Button data-testid="button" />;
const TestButton: FC<ButtonProps> = (p) => (
<Button data-testid="button" {...p} />
);

const getButton = () => screen.getByTestId("button");

const clickTrigger = async () => {
Expand All @@ -56,18 +59,30 @@ const advanceTime = async (ms: number) => {
};

test("Sync Action is called when trigger is clicked", async () => {
render(<Action action={syncAction1}>{button}</Action>);
render(
<Action action={syncAction1}>
<TestButton />
</Action>,
);
await clickTrigger();
expect(syncAction1).toHaveBeenCalledOnce();
});

test("Action function is updated when action prop changes", async () => {
const r = render(<Action action={syncAction1}>{button}</Action>);
const r = render(
<Action action={syncAction1}>
<TestButton />
</Action>,
);
await clickTrigger();
expect(syncAction1).toHaveBeenCalledOnce();
expect(syncAction2).not.toHaveBeenCalledOnce();

r.rerender(<Action action={syncAction2}>{button}</Action>);
r.rerender(
<Action action={syncAction2}>
<TestButton />
</Action>,
);
await clickTrigger();
expect(syncAction1).toHaveBeenCalledOnce();
expect(syncAction2).toHaveBeenCalledOnce();
Expand All @@ -76,7 +91,9 @@ test("Action function is updated when action prop changes", async () => {
test("Nested sync actions are called when trigger is clicked", async () => {
render(
<Action action={syncAction2}>
<Action action={syncAction1}>{button}</Action>
<Action action={syncAction1}>
<TestButton />
</Action>
</Action>,
);
await clickTrigger();
Expand All @@ -88,7 +105,9 @@ test("Nested sync actions are not called when break action is used", async () =>
render(
<Action action={syncAction2}>
<Action break>
<Action action={syncAction1}>{button}</Action>
<Action action={syncAction1}>
<TestButton />
</Action>
</Action>
</Action>,
);
Expand All @@ -102,7 +121,9 @@ test("Nested sync actions are not called when skipped", async () => {
<Action action={syncAction2}>
<Action action={syncAction2}>
<Action skip>
<Action action={syncAction1}>{button}</Action>
<Action action={syncAction1}>
<TestButton />
</Action>
</Action>
</Action>
</Action>,
Expand All @@ -117,7 +138,9 @@ test("Nested sync actions are not called when multiple skipped", async () => {
<Action action={syncAction2}>
<Action action={syncAction2}>
<Action skip={2}>
<Action action={syncAction1}>{button}</Action>
<Action action={syncAction1}>
<TestButton />
</Action>
</Action>
</Action>
</Action>,
Expand All @@ -130,7 +153,9 @@ test("Nested sync actions are not called when multiple skipped", async () => {
test("When nested sync actions, the inner action is called first", async () => {
render(
<Action action={syncAction2}>
<Action action={syncAction1}>{button}</Action>
<Action action={syncAction1}>
<TestButton />
</Action>
</Action>,
);

Expand All @@ -139,7 +164,11 @@ test("When nested sync actions, the inner action is called first", async () => {
});

test("Button is enabled again when async action has completed", async () => {
render(<Action action={asyncAction1}>{button}</Action>);
render(
<Action action={asyncAction1}>
<TestButton />
</Action>,
);
await clickTrigger();
await advanceTime(asyncActionDuration);
expect(getButton()).not.toBeDisabled();
Expand All @@ -148,7 +177,9 @@ test("Button is enabled again when async action has completed", async () => {
test("When nested async actions, the outer action is called after the first has completed", async () => {
render(
<Action action={asyncAction2}>
<Action action={asyncAction1}>{button}</Action>
<Action action={asyncAction1}>
<TestButton />
</Action>
</Action>,
);
await clickTrigger();
Expand Down Expand Up @@ -181,20 +212,35 @@ describe("Feedback", () => {
test("is shown when sync action succeeds", async () => {
render(
<Action action={syncAction1} showFeedback>
{button}
<TestButton />
</Action>,
);
await clickTrigger();
expectIconInDom("check");
});

test("is shown when set in props", async () => {
const dom = render(
<Action action={syncAction1} showFeedback>
<TestButton isSucceeded />
</Action>,
);
expectIconInDom("check");
dom.rerender(
<Action action={syncAction1} showFeedback>
<TestButton isFailed />
</Action>,
);
expectIconInDom("x");
});

test("is shown when sync action fails", async () => {
syncAction1.mockImplementation(() => {
throw new Error("Whoops");
});
render(
<Action action={syncAction1} showFeedback>
{button}
<TestButton />
</Action>,
);
await clickTrigger();
Expand All @@ -204,7 +250,7 @@ describe("Feedback", () => {
test("is hidden after some time", async () => {
render(
<Action action={syncAction1} showFeedback>
{button}
<TestButton />
</Action>,
);
await clickTrigger();
Expand All @@ -222,14 +268,31 @@ describe("Pending state", () => {
});

test("is shown when async action is pending", async () => {
render(<Action action={asyncAction1}>{button}</Action>);
render(
<Action action={asyncAction1}>
<TestButton />
</Action>,
);
await clickTrigger();
await advanceTime(1000);
expectIconInDom("loader-2");
});

test("is shown when set in props", async () => {
render(
<Action action={asyncAction1}>
<TestButton isPending />
</Action>,
);
expectIconInDom("loader-2");
});

test("is not shown when sync action is executed", async () => {
render(<Action action={syncAction1}>{button}</Action>);
render(
<Action action={syncAction1}>
<TestButton />
</Action>,
);
await clickTrigger();
await advanceTime(1000);
expectNoIconInDom();
Expand All @@ -238,7 +301,7 @@ describe("Pending state", () => {
test("is hidden after some time", async () => {
render(
<Action action={asyncAction1} showFeedback>
{button}
<TestButton />
</Action>,
);
await clickTrigger();
Expand Down
23 changes: 16 additions & 7 deletions packages/components/src/components/Action/Action.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import React from "react";
import { ActionModel as ActionModel } from "@/components/Action/models/ActionModel";
import type { PropsContext } from "@/lib/propsContext";
import { dynamic, PropsContextProvider } from "@/lib/propsContext";
Expand All @@ -22,18 +21,28 @@ const actionButtonContext: ComponentPropsContext<"Button"> = {
return isConfirmationButton ? confirmAction.execute : action.execute;
}),

isPending: dynamic((props) => useActionButtonState(props) === "isPending"),
isPending: dynamic((props) => {
const actionState = useActionButtonState(props);
return props.isPending ?? actionState === "isPending";
}),

isSucceeded: dynamic(
(props) => useActionButtonState(props) === "isSucceeded",
),
isSucceeded: dynamic((props) => {
const actionState = useActionButtonState(props);
return props.isSucceeded ?? actionState === "isSucceeded";
}),

isFailed: dynamic((props) => useActionButtonState(props) === "isFailed"),
isFailed: dynamic((props) => {
const actionState = useActionButtonState(props);
return props.isFailed ?? actionState === "isFailed";
}),

"aria-disabled": dynamic((props) => {
const state = useActionButtonState(props);
const someActionInContextIsBusy = useActionStateContext().useIsBusy();
return state === "isExecuting" || someActionInContextIsBusy;
return (
props["aria-disabled"] ??
(state === "isExecuting" || someActionInContextIsBusy)
);
}),
};

Expand Down