Merge pull request #198 from element-hq/quenting/edit-in-place/better…

…-errors

EditInPlace: pass error messages as children to allow better messages

src/components/Form/Controls/EditInPlace/EditInPlace.stories.tsx

@@ -19,6 +19,7 @@ import React from "react";
 import { EditInPlace } from "./";
 import { Meta, StoryObj } from "@storybook/react";
 import { expect, userEvent, within } from "@storybook/test";
+import { ErrorMessage } from "../../Message";
 
 type Props = { invalid?: boolean } & React.ComponentProps<typeof EditInPlace>;
 
@@ -32,11 +33,14 @@ export default {
         "onChange",
         "onSave",
         "onCancel",
+        "onClearServerErrors",
         "defaultValue",
         "error",
+        "serverInvalid",
         "savedLabel",
         "saveButtonLabel",
         "cancelButtonLabel",
+        "helpLabel",
         "disabled",
       ],
     },
@@ -58,7 +62,13 @@ export default {
     onCancel: {
       action: "cancelled",
     },
-    error: {
+    onClearServerErrors: {
+      action: "cleared server errors",
+    },
+    serverInvalid: {
+      type: "boolean",
+    },
+    helpLabel: {
       type: "string",
     },
     savedLabel: {
@@ -122,7 +132,8 @@ export const Saving: Story = {
 
 export const WithError: Story = {
   args: {
-    error: "I am a teapot",
+    serverInvalid: true,
+    children: <ErrorMessage>I am a teapot</ErrorMessage>,
   },
 };
 

src/components/Form/Controls/EditInPlace/EditInPlace.test.tsx

@@ -22,31 +22,27 @@ import { userEvent } from "@storybook/test";
 import { act } from "react-dom/test-utils";
 import { TooltipProvider } from "@radix-ui/react-tooltip";
 import { EditInPlace } from "./EditInPlace";
+import { ErrorMessage } from "../../Message";
 
-type EditInPlaceTestProps = {
-  error?: string;
-  defaultValue?: string;
-  onChange?: (e: React.ChangeEvent<HTMLInputElement>) => void;
-  onSave?: () => Promise<void>;
-  onCancel?: () => void;
-  disabled?: boolean;
-};
+type EditInPlaceTestProps = Omit<
+  React.ComponentProps<typeof EditInPlace>,
+  | "saveButtonLabel"
+  | "cancelButtonLabel"
+  | "savedLabel"
+  | "savingLabel"
+  | "label"
+>;
 
 describe("EditInPlace", () => {
   const EditInPlaceTest = (props: EditInPlaceTestProps) => (
     <TooltipProvider>
       <EditInPlace
         label="Edit Me"
-        defaultValue={props.defaultValue}
-        error={props.error}
-        onChange={props.onChange ?? (() => {})}
-        onSave={props.onSave ?? (() => Promise.resolve())}
-        onCancel={props.onCancel ?? (() => {})}
         saveButtonLabel="Save"
         cancelButtonLabel="Cancel"
         savedLabel="Saved"
         savingLabel="Saving..."
-        disabled={props.disabled}
+        {...props}
       />
     </TooltipProvider>
   );
@@ -88,9 +84,11 @@ describe("EditInPlace", () => {
     expect(input).toBeValid();
   });
 
-  it("renders error icon and text if error provided", () => {
+  it("renders error icon and text if passed as children", () => {
     const { asFragment, getByRole, getByText } = render(
-      <EditInPlaceTest error="Missing Left Falangey" />,
+      <EditInPlaceTest serverInvalid>
+        <ErrorMessage>Missing Left Falangey</ErrorMessage>
+      </EditInPlaceTest>,
     );
     expect(asFragment()).toMatchSnapshot();
 
@@ -104,6 +102,88 @@ describe("EditInPlace", () => {
     expect(errorText.id).toEqual(input.getAttribute("aria-describedby"));
   });
 
+  it("uses native form validation logic", async () => {
+    const { asFragment, getByRole, queryByText } = render(
+      <EditInPlaceTest type="email" required>
+        <ErrorMessage match="valueMissing">you did not fill this</ErrorMessage>
+        <ErrorMessage match="typeMismatch">this is not an email</ErrorMessage>
+      </EditInPlaceTest>,
+    );
+    expect(asFragment()).toMatchSnapshot();
+
+    const input = getByRole("textbox");
+    expect(input).toBeInvalid();
+
+    // The message hasn't showed up yet
+    expect(queryByText("you did not fill this")).not.toBeInTheDocument();
+
+    await act(async () => {
+      // Focus and submit the form
+      await userEvent.keyboard("{tab}{enter}");
+    });
+
+    // The message should be there
+    expect(queryByText("you did not fill this")).toBeInTheDocument();
+
+    await act(async () => {
+      // Type an invalid email
+      await userEvent.type(input, "notanemail");
+      await userEvent.keyboard("{enter}");
+    });
+
+    // Another message should be there
+    expect(queryByText("you did not fill this")).not.toBeInTheDocument();
+    expect(queryByText("this is not an email")).toBeInTheDocument();
+
+    await act(async () => {
+      // Type something else to clear out the errors
+      await userEvent.clear(input);
+      await userEvent.type(input, "something else");
+    });
+
+    // The message should be gone
+    expect(queryByText("you did not fill this")).not.toBeInTheDocument();
+    expect(queryByText("this is not an email")).not.toBeInTheDocument();
+  });
+
+  it("uses the custom error messages passed as children", async () => {
+    const { getByRole, queryByText, asFragment } = render(
+      <EditInPlaceTest>
+        <ErrorMessage match={(value) => value !== "hunter2"}>
+          password should be hunter2
+        </ErrorMessage>
+      </EditInPlaceTest>,
+    );
+    expect(asFragment()).toMatchSnapshot();
+
+    const input = getByRole("textbox");
+
+    // The message hasn't showed up yet
+    expect(queryByText("password should be hunter2")).not.toBeInTheDocument();
+    expect(input).toBeValid();
+
+    await act(async () => {
+      // Focus, type and submit the form
+      await userEvent.type(input, "something");
+      await userEvent.keyboard("{tab}");
+    });
+
+    // The message should be there
+    expect(queryByText("password should be hunter2")).toBeInTheDocument();
+    expect(input).toBeInvalid();
+
+    await act(async () => {
+      // Type the correct password
+      await userEvent.clear(input);
+      await userEvent.type(input, "hunter2");
+      await userEvent.keyboard("{tab}");
+    });
+
+    // The message should be gone and the input valid
+    expect(queryByText("password should be hunter2")).not.toBeInTheDocument();
+    expect(input).toBeValid();
+  });
+
   it("should show the buttons when the input or buttons are focused", async () => {
     const { queryByRole } = render(<EditInPlaceTest />);
 
@@ -257,7 +337,7 @@ describe("EditInPlace", () => {
     const user = userEvent.setup({ advanceTimers: vi.advanceTimersByTime });
 
     const { getByRole, getByText, queryByText, asFragment } = render(
-      <EditInPlaceTest />,
+      <EditInPlaceTest onSave={() => new Promise((r) => setTimeout(r, 500))} />,
     );
 
     expect(asFragment()).toMatchSnapshot();
@@ -269,16 +349,17 @@ describe("EditInPlace", () => {
     });
 
     expect(asFragment()).toMatchSnapshot();
-    expect(getByText("Saved")).toBeInTheDocument();
+    expect(getByText("Saving...")).toBeInTheDocument();
 
-    act(() => {
-      vi.advanceTimersByTime(1900);
+    await act(async () => {
+      await vi.advanceTimersByTimeAsync(600);
     });
 
+    expect(asFragment()).toMatchSnapshot();
     expect(queryByText("Saved")).toBeInTheDocument();
 
-    act(() => {
-      vi.advanceTimersByTime(200);
+    await act(async () => {
+      await vi.advanceTimersByTimeAsync(2000);
     });
 
     expect(queryByText("Saved")).not.toBeInTheDocument();
@@ -296,6 +377,71 @@ describe("EditInPlace", () => {
     expect(onSave).not.toHaveBeenCalled();
   });
 
+  it("shows the help label in the right conditions", async () => {
+    vi.useFakeTimers();
+    const user = userEvent.setup({ advanceTimers: vi.advanceTimersByTime });
+    const { getByRole, getByText, queryByText } = render(
+      <EditInPlaceTest
+        helpLabel="Help"
+        required
+        onSave={() => new Promise((r) => setTimeout(r, 500))}
+      >
+        <ErrorMessage match="valueMissing">Required</ErrorMessage>
+      </EditInPlaceTest>,
+    );
+
+    const input = getByRole("textbox");
+    expect(getByText("Help")).toBeInTheDocument();
+
+    // When we start typing, it's still there
+    await act(async () => {
+      await user.type(input, "Changed");
+      await user.keyboard("{tab}");
+    });
+
+    expect(getByText("Help")).toBeInTheDocument();
+
+    // When the form becomes valid, it's gone
+    await act(async () => {
+      await user.clear(input);
+      await user.keyboard("{tab}");
+    });
+
+    expect(queryByText("Help")).not.toBeInTheDocument();
+
+    // When it becomes valid again, it's back
+    await act(async () => {
+      await user.type(input, "Changed");
+      await user.keyboard("{tab}");
+    });
+
+    expect(getByText("Help")).toBeInTheDocument();
+
+    // When we're submitting, it's gone
+    await act(async () => {
+      await user.click(getByRole("button", { name: "Save" }));
+    });
+
+    expect(queryByText("Help")).not.toBeInTheDocument();
+    expect(getByText("Saving...")).toBeInTheDocument();
+
+    // When we're showing 'Saved', it's still gone
+    await act(async () => {
+      await vi.advanceTimersByTimeAsync(600);
+    });
+
+    expect(queryByText("Help")).not.toBeInTheDocument();
+    expect(getByText("Saved")).toBeInTheDocument();
+
+    // When we're done showing 'Saved', it's back
+    await act(async () => {
+      await vi.advanceTimersByTimeAsync(2000);
+    });
+
+    expect(getByText("Help")).toBeInTheDocument();
+    expect(queryByText("Saved")).not.toBeInTheDocument();
+  });
+
   it("disables control when disabled", () => {
     const { getByRole, asFragment } = render(<EditInPlaceTest disabled />);
     expect(asFragment()).toMatchSnapshot();