[BOOKINGSG-9283][ZZ] refactor: migrate date input to linaria#1169
[BOOKINGSG-9283][ZZ] refactor: migrate date input to linaria#1169ziggyzet wants to merge 10 commits into
Conversation
Co-authored-by: Copilot <copilot@github.com>
…nput Co-authored-by: Copilot <copilot@github.com>
ziggyzet
requested review from
ghazwan-gt,
iant-gds,
qroll,
ryan-nguyen-t and
zhaoyanxzy
| }); | ||
| }); | ||
|
|
||
| test("Mount (dark mode)", async ({ story }) => { |
There was a problem hiding this comment.
Might be more concise to remove "(dark mode)", as it is alr specified in the outer describe block
(same for other sibling blocks)
| test("Mount (dark mode)", async ({ story }) => { | |
| test("Mount", async ({ story }) => { |
There was a problem hiding this comment.
I noticed that for read-only dark mode, the color of the date and dividers do not change. I track it down to the fact that the hoverValue in standalone-date-input.tsx is always undefined, which cause the dividerHover & baseInputHover classes to not be applied.
Is it fine to setHoverDate for read-only mode in date-input.tsx (in the useEffect) like
if (readOnly) {
setHoveredDate(newValue);
}
?
There was a problem hiding this comment.
and then in read-only.e2e.tsx can add the story-background class from global.css to make the background dark
There was a problem hiding this comment.
idt this is only for readOnly though. Will check what's happening here.
There was a problem hiding this comment.
Not related to the changes, but lets add await for the toBeFocused lines on the file
Co-authored-by: Copilot <copilot@github.com>
| await expect(story.page.getByTestId("form-date-input-short")) | ||
| .toMatchAriaSnapshot(` | ||
| - group "Short date input": | ||
| - group: | ||
| - textbox "Date": | ||
| - /placeholder: DD | ||
| - text: / | ||
| - textbox "Month": | ||
| - /placeholder: MM | ||
| - text: / | ||
| - textbox "Year": | ||
| - /placeholder: YYYY | ||
| `); | ||
|
|
||
| await expect(story.page.getByTestId("form-date-input-long")) |
There was a problem hiding this comment.
aria snapshots not needed, they were covered in the earlier form variants!
| }); | ||
| }); | ||
|
|
||
| test("Disabled", async ({ story }) => { |
| }); | ||
| }); | ||
|
|
||
| test("Read-only", async ({ story }) => { |
| }); | ||
| }); | ||
|
|
||
| test.describe("Error", () => { |
| await expect(story.getUnavailableDayCell(1)).toBeVisible(); | ||
| await expect(story.getUnavailableDayCell(25)).toBeVisible(); |
There was a problem hiding this comment.
unnecessary checks, since we have the screenshot
| await expect(story.getUnavailableDayCell(10)).toBeVisible(); | ||
| await expect(story.getUnavailableDayCell(15)).toBeVisible(); |
| locator: story.locators.dateInput, | ||
| }); | ||
| }); | ||
| }); |
| const [selectedDate, setSelectedDate] = useState<string>(""); | ||
|
|
||
| return ( | ||
| <div className={`${styles.story} story-background`}> |
There was a problem hiding this comment.
can exclude this styles.story from the e2e stories, we'll take the default width of the input; grid and mobile will cover the width behaviour
4 participants
Checklist
classNameis chained correctly withclsx- [ ] Updated Storybook documentation- [ ] Added/updated unit tests- [ ] Listed breaking changes