From a88e811b50df1d2ed2c575c1f12957d0708427c8 Mon Sep 17 00:00:00 2001
From: Peter Makowski <peter.makowski@canonical.com>
Date: Wed, 10 Jan 2024 09:28:19 +0100
Subject: [PATCH] fix: input label without specifying id (#1012)

---
 src/components/Field/Field.test.tsx |  8 ++++----
 src/components/Input/Input.test.tsx | 20 +++++++++++++++-----
 src/components/Input/Input.tsx      |  6 ++++--
 3 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/src/components/Field/Field.test.tsx b/src/components/Field/Field.test.tsx
index 1578ae157..f0e7e81d6 100644
--- a/src/components/Field/Field.test.tsx
+++ b/src/components/Field/Field.test.tsx
@@ -26,7 +26,7 @@ describe("Field ", () => {
         <input aria-errormessage="id-1" aria-invalid="true" />
       </Field>
     );
-    expect(screen.getByRole("textbox")).toHaveErrorMessage(
+    expect(screen.getByRole("textbox")).toHaveAccessibleErrorMessage(
       "Caution: Are you sure?"
     );
     expect(screen.getByTestId("field")).toHaveClass("is-caution");
@@ -42,7 +42,7 @@ describe("Field ", () => {
         <input aria-errormessage="id-1" aria-invalid="true" />
       </Field>
     );
-    expect(screen.getByRole("textbox")).toHaveErrorMessage(
+    expect(screen.getByRole("textbox")).toHaveAccessibleErrorMessage(
       "Caution: Are you sure?"
     );
     expect(screen.getByTestId("field")).toHaveClass("is-caution");
@@ -54,7 +54,7 @@ describe("Field ", () => {
         <input aria-errormessage="id-1" aria-invalid="true" />
       </Field>
     );
-    expect(screen.getByRole("textbox")).toHaveErrorMessage(
+    expect(screen.getByRole("textbox")).toHaveAccessibleErrorMessage(
       "Error: You can't do that"
     );
     expect(screen.getByTestId("field")).toHaveClass("is-error");
@@ -70,7 +70,7 @@ describe("Field ", () => {
         <input aria-errormessage="id-1" aria-invalid="true" />
       </Field>
     );
-    expect(screen.getByRole("textbox")).toHaveErrorMessage(
+    expect(screen.getByRole("textbox")).toHaveAccessibleErrorMessage(
       "Error: You can't do that"
     );
     expect(screen.getByTestId("field")).toHaveClass("is-error");
diff --git a/src/components/Input/Input.test.tsx b/src/components/Input/Input.test.tsx
index 194779f87..bfb90b1aa 100644
--- a/src/components/Input/Input.test.tsx
+++ b/src/components/Input/Input.test.tsx
@@ -7,14 +7,17 @@ describe("Input", () => {
   it("displays the field label for text inputs", () => {
     render(<Input type="text" label="text label" />);
     expect(screen.getByText("text label")).toHaveClass("p-form__label");
+    expect(screen.getByRole("textbox")).toHaveAccessibleName("text label");
   });
 
   it("moves the label for radio buttons", () => {
-    render(<Input type="radio" />);
+    render(<Input type="radio" label="text label" />);
     expect(
       // eslint-disable-next-line testing-library/no-node-access
       document.querySelector(".p-radio__label")
     ).toBeInTheDocument();
+
+    expect(screen.getByRole("radio")).toHaveAccessibleName("text label");
     expect(
       // eslint-disable-next-line testing-library/no-node-access
       document.querySelector(".p-form__label")
@@ -22,7 +25,8 @@ describe("Input", () => {
   });
 
   it("moves the label for checkboxes", () => {
-    render(<Input type="checkbox" />);
+    render(<Input type="checkbox" label="text label" />);
+    expect(screen.getByRole("checkbox")).toHaveAccessibleName("text label");
     expect(
       // eslint-disable-next-line testing-library/no-node-access
       document.querySelector(".p-checkbox__label")
@@ -93,17 +97,23 @@ describe("Input", () => {
 
   it("can display an error for a text input", async () => {
     render(<Input error="Uh oh!" type="text" />);
-    expect(screen.getByRole("textbox")).toHaveErrorMessage("Error: Uh oh!");
+    expect(screen.getByRole("textbox")).toHaveAccessibleErrorMessage(
+      "Error: Uh oh!"
+    );
   });
 
   it("can display an error for a radiobutton", async () => {
     render(<Input error="Uh oh!" type="radio" />);
-    expect(screen.getByRole("radio")).toHaveErrorMessage("Error: Uh oh!");
+    expect(screen.getByRole("radio")).toHaveAccessibleErrorMessage(
+      "Error: Uh oh!"
+    );
   });
 
   it("can display an error for a checkbox", async () => {
     render(<Input error="Uh oh!" type="checkbox" />);
-    expect(screen.getByRole("checkbox")).toHaveErrorMessage("Error: Uh oh!");
+    expect(screen.getByRole("checkbox")).toHaveAccessibleErrorMessage(
+      "Error: Uh oh!"
+    );
   });
 
   it("can display help for a text input", async () => {
diff --git a/src/components/Input/Input.tsx b/src/components/Input/Input.tsx
index c4da04bc4..41e46b8ab 100644
--- a/src/components/Input/Input.tsx
+++ b/src/components/Input/Input.tsx
@@ -94,6 +94,8 @@ const Input = ({
 }: Props): JSX.Element => {
   const inputRef = useRef(null);
   const fieldLabel = !["checkbox", "radio"].includes(type) ? label : "";
+  const defaultInputId = useId();
+  const inputId = id || defaultInputId;
   const validationId = useId();
   const helpId = useId();
   const hasError = !!error;
@@ -104,7 +106,7 @@ const Input = ({
       .join(" "),
     "aria-errormessage": hasError ? validationId : null,
     "aria-invalid": hasError,
-    id: id,
+    id: inputId,
     label: label,
     required: required,
     ...inputProps,
@@ -152,7 +154,7 @@ const Input = ({
       caution={caution}
       className={wrapperClassName}
       error={error}
-      forId={id}
+      forId={inputId}
       help={help}
       helpClassName={helpClassName + ""}
       helpId={helpId}