Merge pull request #1726 from NYPL/development

Release v3.5.2
NYPL · Jan 16, 2025 · 6140923 · 6140923
2 parents 984d4b1 + 35334d8
commit 6140923
Show file tree

Hide file tree

Showing 65 changed files with 1,837 additions and 422 deletions.
diff --git a/.storybook/preview.tsx b/.storybook/preview.tsx
@@ -100,6 +100,28 @@ const parameters = {
         "Chakra UI",
         "Development Guide",
         "Style Guide",
+        [
+          "*",
+          [
+            "Overview",
+            "Page composition",
+            "Page elements",
+            [
+              "Outline",
+              "Keyword search field",
+              "Filter bar",
+              "Active filters list",
+              "Total results heading",
+              "Sorting menu",
+              "Results list",
+              "Pagination menu",
+            ],
+            "Applying & clearing filters",
+            "Accessibility",
+            "JS-disabled",
+          ],
+          "*",
+        ],
         "Accessibility Guide",
         "Components",
         "Hooks",

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,22 @@ Currently, this repo is in Prerelease. When it is released, this project will ad
 
 ## Prerelease
 
+## 3.5.2 (January 16, 2025)
+
+### Adds
+
+- Adds the `Filtering` subsection to the `Style Guide`.
+
+### Updates
+
+- Updates the `Form` component to remove the `aria-label`. The previous value of `"form"` was redundant and not required.
+- Updates the `RadioGroup` component to replace the `fieldset` and `legend` wrappers with a div and span for the title. The main div wrapper around the `Radio` components already has a `role="radiogroup"` values which makes the `fieldset` redundant.
+- Updates the `Pagination` component to apply new visual styles.
+
+### Fixes
+
+- Fixes the infinite loop in the `Image` component caused when there is no `fallbackSrc` prop value.
+
 ## 3.5.1 (December 19, 2024)
 
 ### Adds

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@nypl/design-system-react-components",
-  "version": "3.5.1",
+  "version": "3.5.2",
   "description": "NYPL Reservoir Design System React Components",
   "repository": {
     "type": "git",

diff --git a/src/components/Card/__snapshots__/Card.test.tsx.snap b/src/components/Card/__snapshots__/Card.test.tsx.snap
@@ -15,7 +15,6 @@ exports[`Card Renders the UI snapshot correctly 1`] = `
         alt="Alt text"
         className="css-0"
         id="img-id-regularCard"
-        onError={[Function]}
         src="https://images.nypl.org/index.php?id=swope_243048&t=r"
       />
     </div>
@@ -66,7 +65,6 @@ exports[`Card Renders the UI snapshot correctly 2`] = `
         alt="Alt text"
         className="css-0"
         id="img-id-cardWithExtendedStyles"
-        onError={[Function]}
         src="https://images.nypl.org/index.php?id=swope_243048&t=r"
       />
     </div>
@@ -144,7 +142,6 @@ exports[`Card Renders the UI snapshot correctly 3`] = `
         alt="Alt text"
         className="css-0"
         id="img-id-cardWithNoCTAs"
-        onError={[Function]}
         src="https://images.nypl.org/index.php?id=swope_243048&t=r"
       />
     </div>
@@ -191,7 +188,6 @@ exports[`Card Renders the UI snapshot correctly 4`] = `
         alt="Alt text"
         className="css-0"
         id="img-id-cardWithNoContent"
-        onError={[Function]}
         src="https://images.nypl.org/index.php?id=swope_243048&t=r"
       />
     </div>
@@ -327,7 +323,6 @@ exports[`Card Renders the UI snapshot correctly 6`] = `
         alt="Alt text"
         className="css-0"
         id="img-id-fullclick"
-        onError={[Function]}
         src="https://images.nypl.org/index.php?id=swope_243048&t=r"
       />
     </div>
@@ -383,7 +378,6 @@ exports[`Card Renders the UI snapshot correctly 7`] = `
         alt="Alt text"
         className="css-0"
         id="img-id-cardWithRightActions"
-        onError={[Function]}
         src="https://images.nypl.org/index.php?id=swope_243048&t=r"
       />
     </div>
@@ -446,7 +440,6 @@ exports[`Card Renders the UI snapshot correctly 8`] = `
         alt="Alt text"
         className="css-0"
         id="img-id-chakraProps"
-        onError={[Function]}
         src="https://images.nypl.org/index.php?id=swope_243048&t=r"
       />
     </div>
@@ -498,7 +491,6 @@ exports[`Card Renders the UI snapshot correctly 9`] = `
         alt="Alt text"
         className="css-0"
         id="img-id-otherProps"
-        onError={[Function]}
         src="https://images.nypl.org/index.php?id=swope_243048&t=r"
       />
     </div>

diff --git a/src/components/FeaturedContent/__snapshots__/FeaturedContent.test.tsx.snap b/src/components/FeaturedContent/__snapshots__/FeaturedContent.test.tsx.snap
@@ -19,7 +19,6 @@ exports[`Renders the UI snapshot correctly 1`] = `
           alt="Image example"
           className="css-0"
           id={null}
-          onError={[Function]}
           src="https://images.nypl.org/index.php?id=swope_243048&t=r"
         />
       </div>
@@ -82,7 +81,6 @@ exports[`Renders the UI snapshot correctly 2`] = `
           alt="Image example"
           className="css-0"
           id={null}
-          onError={[Function]}
           src="https://images.nypl.org/index.php?id=swope_243048&t=r"
         />
       </div>
@@ -145,7 +143,6 @@ exports[`Renders the UI snapshot correctly 3`] = `
           alt="Image example"
           className="css-0"
           id={null}
-          onError={[Function]}
           src="https://images.nypl.org/index.php?id=swope_243048&t=r"
         />
       </div>
@@ -208,7 +205,6 @@ exports[`Renders the UI snapshot correctly 4`] = `
           alt="Image example"
           className="css-0"
           id={null}
-          onError={[Function]}
           src="https://images.nypl.org/index.php?id=swope_243048&t=r"
         />
       </div>
@@ -271,7 +267,6 @@ exports[`Renders the UI snapshot correctly 5`] = `
           alt="Image example"
           className="css-0"
           id={null}
-          onError={[Function]}
           src="https://images.nypl.org/index.php?id=swope_243048&t=r"
         />
       </div>
@@ -334,7 +329,6 @@ exports[`Renders the UI snapshot correctly 6`] = `
           alt="Image example"
           className="css-0"
           id={null}
-          onError={[Function]}
           src="https://images.nypl.org/index.php?id=swope_243048&t=r"
         />
       </div>

diff --git a/src/components/Form/Form.mdx b/src/components/Form/Form.mdx
@@ -12,7 +12,7 @@ import Link from "../Link/Link";
 | Component Version | DS Version |
 | ----------------- | ---------- |
 | Added             | `0.23.2`   |
-| Latest            | `3.0.0`    |
+| Latest            | `3.5.2`    |
 
 ## Table of Contents
 
@@ -64,6 +64,8 @@ The `Form` component renders a standard `<form>` element and should be used to
 wrap all form elements. A `Form` should have a `Button` component with a `type`
 of `submit` to submit the form to a server through the `Form`'s `action` prop.
 
+Note: An `aria-label` is not required. If one is needed, make sure it is descriptive.
+
 Resources:
 
 - [WebAIM Creating Accessible Forms](https://webaim.org/techniques/forms/)

diff --git a/src/components/Form/Form.test.tsx b/src/components/Form/Form.test.tsx
@@ -26,61 +26,24 @@ describe("Form Accessibility", () => {
   });
 });
 
-describe("Form Snapshot", () => {
-  it("Renders the UI snapshot correctly", () => {
-    const tree = renderer
-      .create(
-        <Form id="snapshot-form">
-          <FormRow>
-            <FormField>Form Field 1</FormField>
-            <FormField>Form Field 2</FormField>
-            <FormField>Form Field 3</FormField>
-          </FormRow>
-        </Form>
-      )
-      .toJSON();
-    const withChakraProps = renderer
-      .create(
-        <Form id="chakra" p="20px" color="ui.error.primary">
-          <FormRow>
-            <FormField>Form Field 1</FormField>
-            <FormField>Form Field 2</FormField>
-            <FormField>Form Field 3</FormField>
-          </FormRow>
-        </Form>
-      )
-      .toJSON();
-    const withOtherProps = renderer
-      .create(
-        <Form id="props" data-testid="props">
-          <FormRow>
-            <FormField>Form Field 1</FormField>
-            <FormField>Form Field 2</FormField>
-            <FormField>Form Field 3</FormField>
-          </FormRow>
-        </Form>
-      )
-      .toJSON();
-
-    expect(tree).toMatchSnapshot();
-    expect(withChakraProps).toMatchSnapshot();
-    expect(withOtherProps).toMatchSnapshot();
-  });
-});
-
 describe("Form", () => {
-  it("Renders a <form> element", () => {
+  it("renders a <form> element", () => {
     render(<Form id="form" />);
-    expect(screen.getByRole("form")).toBeInTheDocument();
+    expect(screen.getByTestId("ds-form")).toBeInTheDocument();
+  });
+
+  it("renders an aria-label attribute", () => {
+    render(<Form id="form" aria-label="form-test" />);
+    expect(screen.getByLabelText("form-test")).toBeInTheDocument();
   });
 
-  it("Renders a <form> element with child FormRow element", () => {
+  it("renders a <form> element with child FormRow element", () => {
     render(
       <Form id="form">
         <FormRow />
       </Form>
     );
-    const form = screen.getByRole("form");
+    const form = screen.getByTestId("ds-form");
     const formRow = form.firstChild;
     expect(form).toBeInTheDocument();
     expect(formRow).toBeInTheDocument();
@@ -89,13 +52,13 @@ describe("Form", () => {
     });
   });
 
-  it("Renders a <form> element with child FormField element", () => {
+  it("renders a <form> element with child FormField element", () => {
     render(
       <Form id="form">
         <FormField />
       </Form>
     );
-    const form = screen.getByRole("form");
+    const form = screen.getByTestId("ds-form");
     const formField = form.firstChild;
     expect(form).toBeInTheDocument();
     expect(formField).toBeInTheDocument();
@@ -104,7 +67,7 @@ describe("Form", () => {
     });
   });
 
-  it("Renders a <form> element with FormRow, FormField and input elements properly nested", () => {
+  it("renders a <form> element with FormRow, FormField and input elements properly nested", () => {
     render(
       <Form id="form">
         <FormRow>
@@ -114,7 +77,7 @@ describe("Form", () => {
         </FormRow>
       </Form>
     );
-    const form = screen.getByRole("form");
+    const form = screen.getByTestId("ds-form");
     const formRow = form.firstChild;
     const formField = formRow?.firstChild;
     const textInput = screen.getByRole("textbox");
@@ -130,9 +93,9 @@ describe("Form", () => {
     expect(textInput).toBeInTheDocument();
   });
 
-  it("Renders a <form> element with custom `action` and `method` attributes", () => {
+  it("renders a <form> element with custom `action` and `method` attributes", () => {
     render(<Form id="form" action="/end/point" method="get" />);
-    const form = screen.getByRole("form");
+    const form = screen.getByTestId("ds-form");
     expect(form).toBeInTheDocument();
     expect(form).toHaveAttribute("action", "/end/point");
     expect(form).toHaveAttribute("method", "get");
@@ -208,13 +171,13 @@ describe("Form", () => {
         </FormRow>
       </Form>
     );
-    const form = screen.getByRole("form");
+    const form = screen.getByTestId("ds-form");
     expect(onSubmit).toHaveBeenCalledTimes(0);
     fireEvent.submit(form);
     expect(onSubmit).toHaveBeenCalledTimes(1);
   });
 
-  it("Logs a warning when there is no `id` passed", () => {
+  it("logs a warning when there is no `id` passed", () => {
     const warn = jest.spyOn(console, "warn");
     render(
       // @ts-ignore: Typescript complains when a required prop is not passed, but
@@ -247,3 +210,45 @@ describe("Form", () => {
     expect(container.querySelector("form")).toBe(ref.current);
   });
 });
+
+describe("Form Snapshot", () => {
+  it("renders the UI snapshot correctly", () => {
+    const tree = renderer
+      .create(
+        <Form id="snapshot-form">
+          <FormRow>
+            <FormField>Form Field 1</FormField>
+            <FormField>Form Field 2</FormField>
+            <FormField>Form Field 3</FormField>
+          </FormRow>
+        </Form>
+      )
+      .toJSON();
+    const withChakraProps = renderer
+      .create(
+        <Form id="chakra" p="20px" color="ui.error.primary">
+          <FormRow>
+            <FormField>Form Field 1</FormField>
+            <FormField>Form Field 2</FormField>
+            <FormField>Form Field 3</FormField>
+          </FormRow>
+        </Form>
+      )
+      .toJSON();
+    const withOtherProps = renderer
+      .create(
+        <Form id="props" data-testid="props">
+          <FormRow>
+            <FormField>Form Field 1</FormField>
+            <FormField>Form Field 2</FormField>
+            <FormField>Form Field 3</FormField>
+          </FormRow>
+        </Form>
+      )
+      .toJSON();
+
+    expect(tree).toMatchSnapshot();
+    expect(withChakraProps).toMatchSnapshot();
+    expect(withOtherProps).toMatchSnapshot();
+  });
+});
diff --git a/src/components/Form/Form.tsx b/src/components/Form/Form.tsx
@@ -125,8 +125,8 @@ export const Form: ChakraComponent<
     return (
       <Box
         as="form"
-        aria-label="form"
         className={className}
+        data-testid="ds-form"
         id={id}
         onSubmit={onSubmit}
         ref={ref}