update props, comments as discussed, update test and doc accordingly

NYPL · Oct 12, 2023 · 1e634b1 · 1e634b1
1 parent d99e940
commit 1e634b1
Show file tree

Hide file tree

Showing 6 changed files with 125 additions and 81 deletions.
diff --git a/src/components/NewsletterSignup/NewsletterSignup.mdx b/src/components/NewsletterSignup/NewsletterSignup.mdx
@@ -2,6 +2,8 @@ import { ArgTypes, Canvas, Description, Meta, Source } from "@storybook/blocks";
 
 import * as NewsletterSignupStories from "./NewsletterSignup.stories";
 import Link from "../Link/Link";
+import ComponentChangelogTable from "../../utils/ComponentChangelogTable";
+import { changelogData } from "./newsletterSignupChangelogData";
 
 <Meta of={NewsletterSignupStories} />
 
@@ -17,6 +19,7 @@ import Link from "../Link/Link";
 - {<Link href="#overview" target="_self">Overview</Link>}
 - {<Link href="#component-props" target="_self">Component Props</Link>}
 - {<Link href="#accessibility" target="_self">Accessibility</Link>}
+- {<Link href="#changelog" target="_self">Changelog</Link>}
 
 ## Overview
 
@@ -33,6 +36,9 @@ import Link from "../Link/Link";
 The `NewsletterSignup` component is a complex component built from various Reservoir
 DS and Chakra components.
 
+The `title` prop of the `NewsletterSignup` component expects a `HTML Element` or a `React Component`. By default it renderes a `h2` tag but it is
+the responsibility of the consuming app to pass the heading tag (`h*`) that aligns with the page structure and ensures accessibility.
+
 Within the `NewsletterSignup` component, the DS `form` component wraps around two DS `FormField` components.
 Those`FormField` components hold a DS `TextInput` component of `type="email"` and a DS `Button` component of `type="submit"` respectively.
 Each of these components has their own accessibility features documented in their respective Storybook pages.
@@ -42,6 +48,7 @@ error message if an error occurs.
 
 Resources:
 
+- [W3C WAI Headings](https://www.w3.org/WAI/tutorials/page-structure/headings/)
 - [DS Form Accessibility](../?path=/docs/components-form-elements-form--docs#accessibility)
 - [DS Button Accessibility](../?path=/docs/components-form-elements-button--docs#accessibility)
 - [DS TextInput Accessibility](../?path=/docs/components-form-elements-textinput--docs#accessibility)
@@ -150,3 +157,7 @@ function NewsletterSignupOnSubmitExampleComponent() {
 ## Component States
 
 <Canvas of={NewsletterSignupStories.ComponentStates} />
+
+## Changelog
+
+<ComponentChangelogTable changelogData={changelogData} />
diff --git a/src/components/NewsletterSignup/NewsletterSignup.stories.tsx b/src/components/NewsletterSignup/NewsletterSignup.stories.tsx
@@ -68,10 +68,17 @@ const meta: Meta<typeof NewsletterSignup> = {
       },
     },
     title: {
-      control: "text",
+      control: false,
+      mapping: {
+        default: (
+          <Heading noSpace size="heading3" text="Sign Up for Our Newsletter" />
+        ),
+      },
       table: {
+        control: "text",
         defaultValue: {
-          summary: "Sign Up for Our Newsletter!",
+          summary:
+            '<Heading noSpace size="heading3" text="Sign Up for Our Newsletter" />',
         },
       },
     },
@@ -115,9 +122,10 @@ export const WithControls: Story = {
       event.preventDefault();
       action("onSubmit")(event.target[0].value);
     },
-    privacyPolicyLink:
-      "https://www.nypl.org/help/about-nypl/legal-notices/privacy-policy",
-    title: undefined,
+    privacyPolicyLink: undefined,
+    title: (
+      <Heading noSpace size="heading3" text="Sign Up for Our Newsletter" />
+    ),
     valueEmail: undefined,
     view: undefined,
   },
@@ -286,7 +294,9 @@ export const ComponentStates: Story = {
 };
 
 /* To fix focus issue where the page focuses on the last NewsletterSignup 
-component example */
+component example. 
+Note: This behavior only effects the storybook doc and is caused by rendering
+a list of the component in different states. This issue should not happen on a consuming app page*/
 const setFocus = () => {
   const heading = document.getElementById(
     "anchor--components-form-elements-newslettersignup--with-controls"
@@ -295,4 +305,4 @@ const setFocus = () => {
   heading.scrollIntoView({ behavior: "smooth" });
 };
 
-setTimeout(setFocus, 1000);
+setTimeout(setFocus, 2000);
diff --git a/src/components/NewsletterSignup/NewsletterSignup.test.tsx b/src/components/NewsletterSignup/NewsletterSignup.test.tsx
@@ -18,7 +18,6 @@ describe("NewsletterSignup Accessibility", () => {
       <NewsletterSignup
         className="my-class"
         id="my-id"
-        title="Testing"
         formHelperText="Form helper"
         onSubmit={onSubmit}
         onChange={onChange}
@@ -35,7 +34,6 @@ describe("NewsletterSignup Accessibility", () => {
       <NewsletterSignup
         className="my-class"
         id="my-id"
-        title="Testing"
         formHelperText="Form helper"
         onSubmit={onSubmit}
         onChange={onChange}
@@ -53,7 +51,6 @@ describe("NewsletterSignup Accessibility", () => {
       <NewsletterSignup
         className="my-class"
         id="my-id"
-        title="Testing"
         formHelperText="Form helper."
         onSubmit={onSubmit}
         onChange={onChange}
@@ -71,7 +68,6 @@ describe("NewsletterSignup Accessibility", () => {
       <NewsletterSignup
         className="my-class"
         id="my-id"
-        title="Testing"
         formHelperText="Form helper"
         onSubmit={onSubmit}
         onChange={onChange}
@@ -89,7 +85,6 @@ describe("NewsletterSignup Accessibility", () => {
       <NewsletterSignup
         className="my-class"
         id="my-id"
-        title="Testing"
         formHelperText="Form helper"
         onSubmit={onSubmit}
         onChange={onChange}
@@ -115,7 +110,6 @@ describe("NewsletterSignup Unit Tests", () => {
   it("Renders the Minimum Required Elements for the Form", () => {
     render(
       <NewsletterSignup
-        title="Testing"
         onSubmit={onSubmit}
         onChange={onChange}
         confirmationHeading="Thank you for signing up!"
@@ -125,7 +119,7 @@ describe("NewsletterSignup Unit Tests", () => {
     expect(screen.getByRole("form")).toBeInTheDocument();
     expect(screen.getByRole("textbox")).toBeInTheDocument();
     expect(screen.getByRole("button", { name: "Submit" })).toBeInTheDocument();
-    expect(screen.getByText(/Testing/i)).toBeInTheDocument();
+    expect(screen.getByRole("heading", { level: 2 })).toBeInTheDocument();
     expect(screen.getByText(/Privacy Policy/i)).toBeInTheDocument();
   });
 

diff --git a/src/components/NewsletterSignup/NewsletterSignup.tsx b/src/components/NewsletterSignup/NewsletterSignup.tsx
@@ -38,16 +38,18 @@ interface NewsletterSignupProps {
   id?: string;
   /** Toggles the invalid state for the email field. */
   isInvalidEmail?: boolean;
-  /** Value to determine the section color highlight */
+  /** Value to determine the section color highlight. The default is set to "blogs" as it uses the
+   * "ui.border.deafult" color.
+   */
   newsletterSignupType?: SectionTypes;
   /** A handler function that will be called when the form is submitted. */
   onSubmit: (event: React.FormEvent<any>) => void;
   /** A handler function that will be called when the text input changes. */
   onChange: (event: React.ChangeEvent<HTMLInputElement>) => void;
   /** Link to the relevant privacy policy page. */
   privacyPolicyLink?: string;
-  /** Used to populate the `<h3>` header title. */
-  title?: string;
+  /** Used to populate the title of the Component*/
+  title?: JSX.Element;
   /** The value of the email text input field. */
   valueEmail?: string;
   /** Used to specify what is displayed in the component form/feedback area. */
@@ -86,7 +88,9 @@ export const NewsletterSignup = chakra(
         onSubmit,
         privacyPolicyLink = "https://www.nypl.org/help/about-nypl/legal-notices/privacy-policy",
         valueEmail,
-        title = "Sign Up for Our Newsletter!",
+        title = (
+          <Heading noSpace size="heading3" text="Sign Up for Our Newsletter" />
+        ),
         view = "form",
         ...rest
       },
@@ -114,10 +118,10 @@ export const NewsletterSignup = chakra(
           {...rest}
         >
           <VStack __css={styles.pitch} alignItems="flex-start">
-            {title && <Heading level="h3" text={title} /*margin="unset"*/ />}
+            {title}
             {descriptionText ? (
               typeof descriptionText === "string" ? (
-                <Text /*margin="unset"*/>{descriptionText}</Text>
+                <Text noSpace>{descriptionText}</Text>
               ) : (
                 descriptionText
               )
@@ -126,7 +130,6 @@ export const NewsletterSignup = chakra(
             <Link
               href={privacyPolicyLink}
               type="external"
-              // margin="unset"
               isUnderlined={false}
               __css={styles.privacy}
             >