From 7d5888bc46c409f41505972e172615692d042d89 Mon Sep 17 00:00:00 2001 From: nuria1110 Date: Fri, 22 Nov 2024 12:19:23 +0000 Subject: [PATCH] chore: address comments --- playwright/components/numeral-date/index.ts | 2 - .../numeral-date/numeral-date.component.tsx | 222 +++++++----------- .../numeral-date/numeral-date.pw.tsx | 35 ++- .../numeral-date/numeral-date.style.ts | 10 +- .../numeral-date/numeral-date.test.tsx | 40 ++-- 5 files changed, 120 insertions(+), 189 deletions(-) diff --git a/playwright/components/numeral-date/index.ts b/playwright/components/numeral-date/index.ts index e8fcf79609..091d949c7e 100644 --- a/playwright/components/numeral-date/index.ts +++ b/playwright/components/numeral-date/index.ts @@ -6,5 +6,3 @@ export const numeralDateComponent = (page: Page) => page.locator(NUMERAL_DATE_COMPONENT); export const numeralDateInput = (page: Page, index: number) => page.locator(DATE_INPUT).nth(index); -export const numeralDateInputLabel = (page: Page, index: number) => - page.locator("p").nth(index); diff --git a/src/components/numeral-date/numeral-date.component.tsx b/src/components/numeral-date/numeral-date.component.tsx index 4070818a44..ae6f2b9df2 100644 --- a/src/components/numeral-date/numeral-date.component.tsx +++ b/src/components/numeral-date/numeral-date.component.tsx @@ -8,7 +8,6 @@ import Events from "../../__internal__/utils/helpers/events"; import { StyledNumeralDate, StyledDateField, - StyledDateLabel, StyledFieldset, } from "./numeral-date.style"; import Textbox from "../textbox"; @@ -263,13 +262,12 @@ export const NumeralDate = ({ const { validationRedesignOptIn } = useContext(NewValidationContext); const { current: uniqueId } = useRef(id || guid()); + const inputIds = useRef({ dd: guid(), mm: guid(), yyyy: guid() }); const isControlled = useRef(value !== undefined); const initialValue = isControlled.current ? value : defaultValue; const refs = useRef<(HTMLInputElement | null)[]>(dateFormat.map(() => null)); - const labelIds = useRef([guid(), guid(), guid()]); - const [internalMessages, setInternalMessages] = useState({ ...(Object.fromEntries( dateFormat.map((datePart) => [datePart, ""]), @@ -414,11 +412,85 @@ export const NumeralDate = ({ } }; + const renderInputs = () => { + const hasInfo = validationRedesignOptIn ? info : undefined; + + return ( + + {dateFormat.map((datePart, index) => { + const isEnd = index === dateFormat.length - 1; + let inputRef: React.ForwardedRef | undefined; + + const validation = internalError || internalWarning || hasInfo; + const validationInField = + !validationRedesignOptIn && typeof validation === "string"; + + switch (datePart.slice(0, 2)) { + case "dd": + inputRef = dayRef; + break; + case "mm": + inputRef = monthRef; + break; + case "yy": + inputRef = yearRef; + break; + /* istanbul ignore next */ + default: + break; + } + + return ( + + + + handleChange(e, datePart as keyof NumeralDateObject) + } + onBlur={handleBlur} + ref={(element) => handleRef(element, index, inputRef)} + {...(isEnd && + !validationOnLabel && + !validationRedesignOptIn && { + error: internalError, + warning: internalWarning, + info, + })} + tooltipPosition={tooltipPosition} + /> + + + ); + })} + + ); + }; + if (!validationRedesignOptIn) { return ( ({ {...filterStyledSystemMarginProps(rest)} > - - {dateFormat.map((datePart, index) => { - const isEnd = index === dateFormat.length - 1; - const labelId = labelIds.current[index]; - let inputRef: React.ForwardedRef | undefined; - - const validation = internalError || internalWarning || info; - const validationInField = typeof validation === "string"; - - switch (datePart.slice(0, 2)) { - case "dd": - inputRef = dayRef; - break; - case "mm": - inputRef = monthRef; - break; - case "yy": - inputRef = yearRef; - break; - /* istanbul ignore next */ - default: - break; - } - - return ( - - - - {getDateLabel(datePart, locale)} - - - handleChange(e, datePart as keyof NumeralDateObject) - } - onBlur={handleBlur} - ref={(element) => handleRef(element, index, inputRef)} - {...(isEnd && - !validationOnLabel && { - error: internalError, - warning: internalWarning, - info, - })} - tooltipPosition={tooltipPosition} - /> - - - ); - })} - + {renderInputs()} {fieldHelp && {fieldHelp}} @@ -525,7 +520,7 @@ export const NumeralDate = ({ return ( ({ )} - - - {dateFormat.map((datePart, index) => { - const labelId = labelIds.current[index]; - let inputRef: React.ForwardedRef | undefined; - - switch (datePart.slice(0, 2)) { - case "dd": - inputRef = dayRef; - break; - case "mm": - inputRef = monthRef; - break; - case "yy": - inputRef = yearRef; - break; - /* istanbul ignore next */ - default: - break; - } - - return ( - - - - {getDateLabel(datePart, locale)} - - - - handleChange(e, datePart as keyof NumeralDateObject) - } - onBlur={handleBlur} - ref={(element) => handleRef(element, index, inputRef)} - /> - - - ); - })} - + {renderInputs()} ); diff --git a/src/components/numeral-date/numeral-date.pw.tsx b/src/components/numeral-date/numeral-date.pw.tsx index 2bea0a5072..d2ec7c27cc 100644 --- a/src/components/numeral-date/numeral-date.pw.tsx +++ b/src/components/numeral-date/numeral-date.pw.tsx @@ -10,7 +10,6 @@ import { import { numeralDateComponent, numeralDateInput, - numeralDateInputLabel, } from "../../../playwright/components/numeral-date"; import Box from "../box"; @@ -81,8 +80,8 @@ test.describe("NumeralDate component", () => { test("should render NumeralDate with id prop", async ({ mount, page }) => { await mount(); - const input = numeralDateInput(page, 0); - await expect(input).toHaveId(CHARACTERS.STANDARD); + const fieldset = page.locator("fieldset"); + await expect(fieldset).toHaveId(CHARACTERS.STANDARD); }); testData.forEach((label) => { @@ -431,9 +430,9 @@ test.describe("NumeralDate component", () => { }) => { await mount(); - await expect(numeralDateInputLabel(page, 0)).toHaveText("Day"); - await expect(numeralDateInputLabel(page, 1)).toHaveText("Month"); - await expect(numeralDateInputLabel(page, 2)).toHaveText("Year"); + await expect(numeralDateInput(page, 0)).toHaveAccessibleName("Day"); + await expect(numeralDateInput(page, 1)).toHaveAccessibleName("Month"); + await expect(numeralDateInput(page, 2)).toHaveAccessibleName("Year"); }); test('should render NumeralDate with `["mm", "dd", "yyyy"]` dateFormat prop', async ({ @@ -442,9 +441,9 @@ test.describe("NumeralDate component", () => { }) => { await mount(); - await expect(numeralDateInputLabel(page, 0)).toHaveText("Month"); - await expect(numeralDateInputLabel(page, 1)).toHaveText("Day"); - await expect(numeralDateInputLabel(page, 2)).toHaveText("Year"); + await expect(numeralDateInput(page, 0)).toHaveAccessibleName("Month"); + await expect(numeralDateInput(page, 1)).toHaveAccessibleName("Day"); + await expect(numeralDateInput(page, 2)).toHaveAccessibleName("Year"); }); test('should render NumeralDate with `["dd", "mm"]` dateFormat prop', async ({ @@ -453,9 +452,9 @@ test.describe("NumeralDate component", () => { }) => { await mount(); - await expect(numeralDateInputLabel(page, 0)).toHaveText("Day"); - await expect(numeralDateInputLabel(page, 1)).toHaveText("Month"); - await expect(numeralDateInputLabel(page, 2)).not.toBeVisible(); + await expect(numeralDateInput(page, 0)).toHaveAccessibleName("Day"); + await expect(numeralDateInput(page, 1)).toHaveAccessibleName("Month"); + await expect(numeralDateInput(page, 2)).not.toBeVisible(); }); test('should render NumeralDate with `["mm", "dd"]` dateFormat prop', async ({ @@ -464,9 +463,9 @@ test.describe("NumeralDate component", () => { }) => { await mount(); - await expect(numeralDateInputLabel(page, 0)).toHaveText("Month"); - await expect(numeralDateInputLabel(page, 1)).toHaveText("Day"); - await expect(numeralDateInputLabel(page, 2)).not.toBeVisible(); + await expect(numeralDateInput(page, 0)).toHaveAccessibleName("Month"); + await expect(numeralDateInput(page, 1)).toHaveAccessibleName("Day"); + await expect(numeralDateInput(page, 2)).not.toBeVisible(); }); test('should render NumeralDate with `["mm", "yyyy"]` dateFormat prop', async ({ @@ -475,9 +474,9 @@ test.describe("NumeralDate component", () => { }) => { await mount(); - await expect(numeralDateInputLabel(page, 0)).toHaveText("Month"); - await expect(numeralDateInputLabel(page, 1)).toHaveText("Year"); - await expect(numeralDateInputLabel(page, 2)).not.toBeVisible(); + await expect(numeralDateInput(page, 0)).toHaveAccessibleName("Month"); + await expect(numeralDateInput(page, 1)).toHaveAccessibleName("Year"); + await expect(numeralDateInput(page, 2)).not.toBeVisible(); }); ( diff --git a/src/components/numeral-date/numeral-date.style.ts b/src/components/numeral-date/numeral-date.style.ts index 21a9cb1a80..d3a8e537d6 100644 --- a/src/components/numeral-date/numeral-date.style.ts +++ b/src/components/numeral-date/numeral-date.style.ts @@ -2,12 +2,10 @@ import styled, { css } from "styled-components"; import StyledIconSpan from "../../__internal__/input-icon-toggle/input-icon-toggle.style"; import StyledInputPresentation from "../../__internal__/input/input-presentation.style"; import StyledInput from "../../__internal__/input/input.style"; -import Typography from "../typography"; import Fieldset from "../../__internal__/fieldset"; import { StyledLegend } from "../../__internal__/fieldset/fieldset.style"; interface StyledDateFieldProps { - isEnd?: boolean; isYearInput?: boolean; hasValidationIconInField?: boolean; size?: "small" | "medium" | "large"; @@ -30,10 +28,6 @@ export const StyledNumeralDate = styled.div<{ name?: string }>` gap: var(--spacing150); `; -export const StyledDateLabel = styled(Typography)` - margin-bottom: var(--spacing050); -`; - export const StyledDateField = styled.div` ${({ isYearInput, hasValidationIconInField, size }) => css` ${size && @@ -53,6 +47,10 @@ export const StyledDateField = styled.div` width: var(--spacing400); z-index: 999; } + + label { + font-weight: var(--fontWeights400); + } `} `; diff --git a/src/components/numeral-date/numeral-date.test.tsx b/src/components/numeral-date/numeral-date.test.tsx index b03a3f233a..39a3bd4624 100644 --- a/src/components/numeral-date/numeral-date.test.tsx +++ b/src/components/numeral-date/numeral-date.test.tsx @@ -1753,11 +1753,11 @@ test("should render the expected inputs when the `dateFormat` is set as 'mmddyyy />, ); - const inputText = screen.getAllByTestId("numeral-date-input-text"); + const inputs = screen.getAllByRole("textbox"); - expect(inputText[0]).toHaveTextContent("Month"); - expect(inputText[1]).toHaveTextContent("Day"); - expect(inputText[2]).toHaveTextContent("Year"); + expect(inputs[0]).toHaveAccessibleName("Month"); + expect(inputs[1]).toHaveAccessibleName("Day"); + expect(inputs[2]).toHaveAccessibleName("Year"); }); test("should render the expected inputs when the `dateFormat` is set as 'yyyymmdd'", () => { @@ -1769,11 +1769,11 @@ test("should render the expected inputs when the `dateFormat` is set as 'yyyymmd />, ); - const inputText = screen.getAllByTestId("numeral-date-input-text"); + const inputs = screen.getAllByRole("textbox"); - expect(inputText[0]).toHaveTextContent("Year"); - expect(inputText[1]).toHaveTextContent("Month"); - expect(inputText[2]).toHaveTextContent("Day"); + expect(inputs[0]).toHaveAccessibleName("Year"); + expect(inputs[1]).toHaveAccessibleName("Month"); + expect(inputs[2]).toHaveAccessibleName("Day"); }); test("should render the expected inputs when the `dateFormat` is set as 'ddmm'", () => { @@ -1785,11 +1785,11 @@ test("should render the expected inputs when the `dateFormat` is set as 'ddmm'", />, ); - const inputText = screen.getAllByTestId("numeral-date-input-text"); + const inputs = screen.getAllByRole("textbox"); - expect(inputText).toHaveLength(2); - expect(inputText[0]).toHaveTextContent("Day"); - expect(inputText[1]).toHaveTextContent("Month"); + expect(inputs).toHaveLength(2); + expect(inputs[0]).toHaveAccessibleName("Day"); + expect(inputs[1]).toHaveAccessibleName("Month"); }); test("should render the expected inputs when the `dateFormat` is set as 'mmdd'", () => { @@ -1801,11 +1801,11 @@ test("should render the expected inputs when the `dateFormat` is set as 'mmdd'", />, ); - const inputText = screen.getAllByTestId("numeral-date-input-text"); + const inputs = screen.getAllByRole("textbox"); - expect(inputText).toHaveLength(2); - expect(inputText[0]).toHaveTextContent("Month"); - expect(inputText[1]).toHaveTextContent("Day"); + expect(inputs).toHaveLength(2); + expect(inputs[0]).toHaveAccessibleName("Month"); + expect(inputs[1]).toHaveAccessibleName("Day"); }); test("should render the expected inputs when the `dateFormat` is set as 'mmyyyy'", () => { @@ -1817,11 +1817,11 @@ test("should render the expected inputs when the `dateFormat` is set as 'mmyyyy' />, ); - const inputText = screen.getAllByTestId("numeral-date-input-text"); + const inputs = screen.getAllByRole("textbox"); - expect(inputText).toHaveLength(2); - expect(inputText[0]).toHaveTextContent("Month"); - expect(inputText[1]).toHaveTextContent("Year"); + expect(inputs).toHaveLength(2); + expect(inputs[0]).toHaveAccessibleName("Month"); + expect(inputs[1]).toHaveAccessibleName("Year"); }); test("should call `onBlur` callback if prop is passed and user clicks outside of inputs", async () => {