Merge branch 'main' into fix-empty-state

canonical · Apr 26, 2024 · a372645 · a372645
2 parents a412239 + b6067ee
commit a372645
Show file tree

Hide file tree

Showing 18 changed files with 248 additions and 61 deletions.
diff --git a/.github/labeler.yml b/.github/labeler.yml
@@ -3,3 +3,6 @@ Maintenance 🔨:
   - any-glob-to-any-file:
     - package.json
     - .gitignore
+"Review: Percy needed":
+- changed-files:
+  - any-glob-to-any-file: src/**
diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md
@@ -31,6 +31,10 @@ yarn add <path-to-tarball>
 
 - Steps for QA.
 
+### Percy steps
+
+- List any expected visual change in Percy, or write something like "No visual changes expected" if none is expected.
+
 ## Fixes
 
 Fixes: # .
diff --git a/.github/workflows/pr-labels.yml b/.github/workflows/pr-labels.yml
@@ -0,0 +1,23 @@
+name: PR Percy Review Label Required
+on:
+  pull_request:
+    types: [opened, labeled, unlabeled, synchronize]
+jobs:
+  label:
+    runs-on: ubuntu-latest
+    permissions:
+      issues: write
+      pull-requests: write
+    steps:
+      - uses: mheap/github-action-required-labels@v5
+        with:
+          mode: exactly
+          count: 1
+          labels: "Review: Percy +1"
+          add_comment: true
+          message: |
+            This PR is being prevented from merging because it needs to be reviewed on Percy.
+
+            Go to [Percy](https://percy.io/bb49709b/react-components), find the build relevant to this PR and check if it looks as expected.
+
+            Once it's approved, add the label `Review: Percy +1` to this PR.
diff --git a/src/components/ConfirmationModal/ConfirmationModal.test.tsx b/src/components/ConfirmationModal/ConfirmationModal.test.tsx
@@ -55,4 +55,73 @@ describe("ConfirmationModal ", () => {
     await userEvent.click(screen.getByText("Proceed"));
     expect(onConfirm).toHaveBeenCalled();
   });
+
+  it("should stop click event propagation on cancel by default", async () => {
+    const handleExternalClick = jest.fn();
+    render(
+      <div onClick={handleExternalClick}>
+        <ConfirmationModal
+          cancelButtonLabel="Go back"
+          confirmButtonLabel="Proceed"
+          onConfirm={jest.fn()}
+        >
+          Test click propagation
+        </ConfirmationModal>
+      </div>
+    );
+
+    await userEvent.click(screen.getByText("Go back"));
+    expect(handleExternalClick).not.toHaveBeenCalled();
+  });
+
+  it("should propagate click event on cancel", async () => {
+    const handleExternalClick = jest.fn();
+    render(
+      <div onClick={handleExternalClick}>
+        <ConfirmationModal
+          cancelButtonLabel="Go back"
+          confirmButtonLabel="Proceed"
+          onConfirm={jest.fn()}
+          shouldPropagateClickEvent={true}
+        >
+          Test click propagation
+        </ConfirmationModal>
+      </div>
+    );
+
+    await userEvent.click(screen.getByText("Go back"));
+    expect(handleExternalClick).toHaveBeenCalledTimes(1);
+  });
+
+  it("should stop click event propagation on confirm by default", async () => {
+    const handleExternalClick = jest.fn();
+    render(
+      <div onClick={handleExternalClick}>
+        <ConfirmationModal confirmButtonLabel="Proceed" onConfirm={jest.fn()}>
+          Test click propagation
+        </ConfirmationModal>
+      </div>
+    );
+
+    await userEvent.click(screen.getByText("Proceed"));
+    expect(handleExternalClick).not.toHaveBeenCalled();
+  });
+
+  it("should propagate click event on confirm", async () => {
+    const handleExternalClick = jest.fn();
+    render(
+      <div onClick={handleExternalClick}>
+        <ConfirmationModal
+          confirmButtonLabel="Proceed"
+          onConfirm={jest.fn()}
+          shouldPropagateClickEvent={true}
+        >
+          Test click propagation
+        </ConfirmationModal>
+      </div>
+    );
+
+    await userEvent.click(screen.getByText("Proceed"));
+    expect(handleExternalClick).toHaveBeenCalledTimes(1);
+  });
 });
diff --git a/src/components/ConfirmationModal/ConfirmationModal.tsx b/src/components/ConfirmationModal/ConfirmationModal.tsx
@@ -3,6 +3,7 @@ import type { ReactNode } from "react";
 import { PropsWithSpread, ValueOf } from "types";
 import Button, { ButtonAppearance } from "components/Button";
 import Modal, { ModalProps } from "components/Modal";
+import ActionButton from "components/ActionButton";
 
 export type Props = PropsWithSpread<
   {
@@ -29,7 +30,15 @@ export type Props = PropsWithSpread<
     /**
      * Function to perform the action prompted by the modal.
      */
-    onConfirm: (e: MouseEvent<HTMLElement>) => void;
+    onConfirm: (event: MouseEvent<HTMLElement>) => void;
+    /**
+     * Whether the confirm button should be in the loading state.
+     */
+    confirmButtonLoading?: boolean;
+    /**
+     * Whether the confirm button should be disabled.
+     */
+    confirmButtonDisabled?: boolean;
   },
   Omit<ModalProps, "buttonRow">
 >;
@@ -41,23 +50,41 @@ export const ConfirmationModal = ({
   confirmButtonLabel,
   confirmExtra,
   onConfirm,
+  confirmButtonLoading,
+  confirmButtonDisabled,
   ...props
 }: Props): ReactElement => {
+  const handleClick =
+    <A extends Function>(action: A | null | undefined) =>
+    (event: MouseEvent<HTMLButtonElement>) => {
+      if (!props.shouldPropagateClickEvent) {
+        event.stopPropagation();
+      }
+      if (action) {
+        action(event);
+      }
+    };
+
   return (
     <Modal
       buttonRow={
         <>
           {confirmExtra}
-          <Button className="u-no-margin--bottom" onClick={props.close}>
+          <Button
+            className="u-no-margin--bottom"
+            onClick={handleClick(props.close)}
+          >
             {cancelButtonLabel}
           </Button>
-          <Button
+          <ActionButton
             appearance={confirmButtonAppearance}
             className="u-no-margin--bottom"
-            onClick={onConfirm}
+            onClick={handleClick(onConfirm)}
+            loading={confirmButtonLoading}
+            disabled={confirmButtonDisabled}
           >
             {confirmButtonLabel}
-          </Button>
+          </ActionButton>
         </>
       }
       {...props}

diff --git a/src/components/Field/Field.test.tsx b/src/components/Field/Field.test.tsx
@@ -27,7 +27,7 @@ describe("Field ", () => {
       </Field>
     );
     expect(screen.getByRole("textbox")).toHaveAccessibleErrorMessage(
-      "Caution: Are you sure?"
+      "Are you sure?"
     );
     expect(screen.getByTestId("field")).toHaveClass("is-caution");
   });
@@ -43,7 +43,7 @@ describe("Field ", () => {
       </Field>
     );
     expect(screen.getByRole("textbox")).toHaveAccessibleErrorMessage(
-      "Caution: Are you sure?"
+      "Are you sure?"
     );
     expect(screen.getByTestId("field")).toHaveClass("is-caution");
   });
@@ -55,7 +55,7 @@ describe("Field ", () => {
       </Field>
     );
     expect(screen.getByRole("textbox")).toHaveAccessibleErrorMessage(
-      "Error: You can't do that"
+      "You can't do that"
     );
     expect(screen.getByTestId("field")).toHaveClass("is-error");
   });
@@ -71,7 +71,7 @@ describe("Field ", () => {
       </Field>
     );
     expect(screen.getByRole("textbox")).toHaveAccessibleErrorMessage(
-      "Error: You can't do that"
+      "You can't do that"
     );
     expect(screen.getByTestId("field")).toHaveClass("is-error");
   });
@@ -83,7 +83,7 @@ describe("Field ", () => {
       </Field>
     );
     expect(screen.getByRole("textbox")).toHaveAccessibleDescription(
-      "Success: You did it!"
+      "You did it!"
     );
     expect(screen.getByTestId("field")).toHaveClass("is-success");
   });
@@ -99,7 +99,7 @@ describe("Field ", () => {
       </Field>
     );
     expect(screen.getByRole("textbox")).toHaveAccessibleDescription(
-      "Success: You did it!"
+      "You did it!"
     );
     expect(screen.getByTestId("field")).toHaveClass("is-success");
   });

diff --git a/src/components/Field/Field.tsx b/src/components/Field/Field.tsx
@@ -107,11 +107,9 @@ const generateError = (
   if (!error && !caution && !success) {
     return null;
   }
-  const messageType =
-    (error && "Error") || (caution && "Caution") || (success && "Success");
   return (
     <p className="p-form-validation__message" id={validationId}>
-      <strong>{messageType}:</strong> {error || caution || success}
+      {error || caution || success}
     </p>
   );
 };

diff --git a/src/components/FormikField/FormikField.test.tsx b/src/components/FormikField/FormikField.test.tsx
@@ -29,9 +29,7 @@ describe("FormikField", () => {
       </Formik>
     );
 
-    expect(screen.getByRole("textbox")).toHaveAccessibleErrorMessage(
-      "Error: Uh oh!"
-    );
+    expect(screen.getByRole("textbox")).toHaveAccessibleErrorMessage("Uh oh!");
   });
 
   it("can hide the errors", () => {

diff --git a/src/components/Input/Input.test.tsx b/src/components/Input/Input.test.tsx
@@ -103,23 +103,17 @@ describe("Input", () => {
 
   it("can display an error for a text input", async () => {
     render(<Input error="Uh oh!" type="text" />);
-    expect(screen.getByRole("textbox")).toHaveAccessibleErrorMessage(
-      "Error: Uh oh!"
-    );
+    expect(screen.getByRole("textbox")).toHaveAccessibleErrorMessage("Uh oh!");
   });
 
   it("can display an error for a radiobutton", async () => {
     render(<Input error="Uh oh!" type="radio" />);
-    expect(screen.getByRole("radio")).toHaveAccessibleErrorMessage(
-      "Error: Uh oh!"
-    );
+    expect(screen.getByRole("radio")).toHaveAccessibleErrorMessage("Uh oh!");
   });
 
   it("can display an error for a checkbox", async () => {
     render(<Input error="Uh oh!" type="checkbox" />);
-    expect(screen.getByRole("checkbox")).toHaveAccessibleErrorMessage(
-      "Error: Uh oh!"
-    );
+    expect(screen.getByRole("checkbox")).toHaveAccessibleErrorMessage("Uh oh!");
   });
 
   it("can display help for a text input", async () => {

diff --git a/src/components/Modal/Modal.test.tsx b/src/components/Modal/Modal.test.tsx
@@ -139,4 +139,36 @@ describe("Modal ", () => {
     expect(mockHandleModalClose).toHaveBeenCalledTimes(1);
     expect(mockHandleEscPress).not.toHaveBeenCalled();
   });
+
+  it("should stop click event propagation on close by default", async () => {
+    const handleExternalClick = jest.fn();
+    const { container } = render(
+      <div onClick={handleExternalClick}>
+        <Modal title="Test" close={jest.fn()}>
+          Bare bones
+        </Modal>
+      </div>
+    );
+
+    const closeButton = container.querySelector("button.p-modal__close");
+    expect(closeButton).not.toBeNull();
+    await userEvent.click(closeButton!);
+    expect(handleExternalClick).not.toHaveBeenCalled();
+  });
+
+  it("should propagate click event on close", async () => {
+    const handleExternalClick = jest.fn();
+    const { container } = render(
+      <div onClick={handleExternalClick}>
+        <Modal title="Test" close={jest.fn()} shouldPropagateClickEvent={true}>
+          Bare bones
+        </Modal>
+      </div>
+    );
+
+    const closeButton = container.querySelector("button.p-modal__close");
+    expect(closeButton).not.toBeNull();
+    await userEvent.click(closeButton!);
+    expect(handleExternalClick).toHaveBeenCalledTimes(1);
+  });
 });