[DT-1087] Date Picker initial implementation with tests. #2749

otchet-broad · 2024-12-09T22:11:00Z

Addresses

DT-1087

Summary

Adds a form field type of a date picker to Duos UI components per mockups from UX. Visible in tests only.

Have you read Terra's Contributing Guide lately? If not, do that first.

Label PR with a Jira ticket number and include a link to the ticket
Label PR with a security risk modifier [no, low, medium, high]
PR describes scope of changes
Get a minimum of one thumbs worth of review, preferably two if enough team members are available
Get PO sign-off for all non-trivial UI or workflow changes
Verify all tests go green
Test this change deployed correctly and works on dev environment after deployment

…est more reliable in CI/CD.

…yping a bad date.

otchet-broad · 2024-12-10T19:15:09Z

There's an observed issue with Github Actions and the MUI date picker (generic) component rendering in "mobile" mode when being exercised in cypress under a github action. To account for this, I opted to guarantee the Desktop date component since a Data Submitter working on a mobile device is not fully supported at this point.
See https://stackoverflow.com/questions/69799310/cypress-testing-a-material-ui-datepicker-is-not-working-on-github-actions.

src/components/DuosDatePicker.tsx

rushtong

Thank you for the linting and test cleanup, looks reasonable to me 👍🏽

s-rubenstein · 2024-12-11T15:38:51Z

src/components/DuosDatePicker.tsx

+            color: '#000',
+            fontSize: '13px',
+            fontWeight: '400',
+            padding: '7px 20px 7px 20px',


Nit: I think this could be 7px 20px

s-rubenstein · 2024-12-11T15:39:31Z

src/components/DuosDatePicker.tsx

+            fontWeight: '400',
+            padding: '7px 20px 7px 20px',
+            '&.Mui-selected': {
+              backgroundColor: '#216FB4',


Could we make an intermediate variable for this?

@s-rubenstein - Do you mean a CSS variable that calculates the value or a variable that assigns the backgroundColor to the theme?

s-rubenstein · 2024-12-11T15:39:53Z

src/components/DuosDatePicker.tsx

+        styleOverrides:{
+          weekDayLabel:{
+            fontFamily: 'Montserrat',
+            color: '#000',


How would you feel about standardizing this on black/using the color names where we have them?

I don't have strong feelings about this and would be happy to make that change, however we should be on the same page about colors in the context of MUI. Do you expect to see MUI color palettes or CSS ? For this PR, do you expect that standardization across all of DUOS-UI or just in the context of the DuosDatePicker?

One minor thing I've observed in just using the CSS color 'black' instead of '#000' is that the IDE removes the option for the color picker in the gutter. Does that matter to you?

For black I don't strongly care - if we need to change it we can always move it to get the color picker back.

I would personally say let's steer towards unity across the codebase, so if the rest of the codebase uses #000 then lets do that. However, I think right now there's existing inconsistency?

s-rubenstein · 2024-12-11T15:41:09Z

src/components/DuosDatePicker.tsx

+        label={'Select a date'}
+        format={inputFormat}
+        value={value}
+        onAccept={(value)=>{onChange(value);}}


Nit: I think you can just provide the method reference here?

I'll tweak the types on both onChange and onError from Function to any and update these.

Sorry for the back and forth edits on this. I've got the types tweaked for this now to be just a method reference. In making and testing, I found a bug with date validation in a test that I needed to fix. I was passing in the wrong validator in the test.

s-rubenstein · 2024-12-11T15:41:21Z

src/components/DuosDatePicker.tsx

+        format={inputFormat}
+        value={value}
+        onAccept={(value)=>{onChange(value);}}
+        onError={(error, value)=> onError(error, value)}


Same comment about the method reference

rjohanek · 2024-12-12T16:06:36Z

src/components/forms/formValidation.js

+export const dayJSValidator = {
+  id: 'dayjs',
+  isValid: (val) => isValidDayjsDate(val),
+  msg: 'Please select a valid date.',


Wondering about if there is a way to make this error message more helpful, if it is a pop up calendar, what is the case where a user selects an invalid date?

Great question! The control includes a text field that users can type into. The control does a good job at guarding against invalid dates, but they are still theoretically possible. For example, you can type in 2/31/2025.

rjohanek · 2024-12-12T16:11:42Z

src/components/DuosDatePicker.tsx

+
+  const CancelSelectActionBar = (props:PickersActionBarProps) => {
+    // Quirk of this control's usage pattern is the need to destructure the unused onSetToday and onClear from 'other'
+    // props.  This is in part because per mockup, this control does not support 'clear' or 'go to today' style buttons.


can you link the mocks? we don't need a clear button because there is cancel? and we don't need go to today because today is the default value? is that right?

Yes!
Mocks:

Rendered component:

rjohanek

the changes look good, but I'm lacking context from the ticket or the PR description. I know you said it's only visible in tests rn, but I'd love to see what it looks like and where it is used.

rjohanek

the date picker looks good and love the linting thank you!

fboulnois

tests and component look good 👍

s-rubenstein

LGTM

otchet-broad added 7 commits December 9, 2024 17:04

Date Picker initial implementation with tests.

2eccb7a

Merge branch 'develop' into otchet/DT-1087-date-picker-component

4777a3b

Test presence of button element instead of SVG to see if that makes t…

442f1a1

…est more reliable in CI/CD.

See if we can initialize into a good state in CI/CD and still allow t…

d74f747

…yping a bad date.

Try manipulating date in a different way.

76604b9

Force the date picker into desktop mode.

b9504c1

Add in Greg's test refactoring for non-picker-tests.

86d1d06

otchet-broad marked this pull request as ready for review December 10, 2024 19:10

otchet-broad requested a review from a team as a code owner December 10, 2024 19:10

otchet-broad requested review from pshapiro4broad and rjohanek and removed request for a team December 10, 2024 19:10

otchet-broad requested review from rushtong, s-rubenstein and fboulnois December 10, 2024 19:16

rushtong reviewed Dec 10, 2024

View reviewed changes

src/components/DuosDatePicker.tsx Show resolved Hide resolved

rushtong approved these changes Dec 10, 2024

View reviewed changes

s-rubenstein reviewed Dec 11, 2024

View reviewed changes

otchet-broad added 2 commits December 11, 2024 16:00

Updates per feedback from Sky.

93866d3

Use a variable to hold the color of duos blue.

f33e4d1

rjohanek reviewed Dec 12, 2024

View reviewed changes

rjohanek approved these changes Dec 12, 2024

View reviewed changes

otchet-broad requested a review from s-rubenstein December 13, 2024 14:22

fboulnois approved these changes Dec 13, 2024

View reviewed changes

s-rubenstein approved these changes Dec 16, 2024

View reviewed changes

Merge develop and resolve conflicts.

568861b

otchet-broad merged commit 8c494e2 into develop Dec 16, 2024
10 checks passed

otchet-broad deleted the otchet/DT-1087-date-picker-component branch December 16, 2024 22:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DT-1087] Date Picker initial implementation with tests. #2749

[DT-1087] Date Picker initial implementation with tests. #2749

otchet-broad commented Dec 9, 2024 •

edited by jira bot

Loading

otchet-broad commented Dec 10, 2024 •

edited

Loading

rushtong left a comment

s-rubenstein Dec 11, 2024

s-rubenstein Dec 11, 2024

otchet-broad Dec 11, 2024

s-rubenstein Dec 11, 2024

otchet-broad Dec 11, 2024

otchet-broad Dec 11, 2024

s-rubenstein Dec 16, 2024

s-rubenstein Dec 11, 2024

otchet-broad Dec 11, 2024 •

edited

Loading

s-rubenstein Dec 11, 2024

rjohanek Dec 12, 2024

otchet-broad Dec 12, 2024

rjohanek Dec 12, 2024

otchet-broad Dec 12, 2024

rjohanek left a comment

rjohanek left a comment

fboulnois left a comment

s-rubenstein left a comment

[DT-1087] Date Picker initial implementation with tests. #2749

[DT-1087] Date Picker initial implementation with tests. #2749

Conversation

otchet-broad commented Dec 9, 2024 • edited by jira bot Loading

Addresses

Summary

otchet-broad commented Dec 10, 2024 • edited Loading

rushtong left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

otchet-broad Dec 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rjohanek left a comment

Choose a reason for hiding this comment

rjohanek left a comment

Choose a reason for hiding this comment

fboulnois left a comment

Choose a reason for hiding this comment

s-rubenstein left a comment

Choose a reason for hiding this comment

otchet-broad commented Dec 9, 2024 •

edited by jira bot

Loading

otchet-broad commented Dec 10, 2024 •

edited

Loading

otchet-broad Dec 11, 2024 •

edited

Loading