Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: 💡 Migrate DatePicker to MUI5 #747

Merged
merged 3 commits into from
Aug 5, 2022
Merged

refactor: 💡 Migrate DatePicker to MUI5 #747

merged 3 commits into from
Aug 5, 2022

Conversation

aharpt
Copy link
Contributor

@aharpt aharpt commented Aug 4, 2022 •
edited
Loading

@aharpt aharpt requested a review from a team August 4, 2022 19:49
@aharpt aharpt requested a review from 20BBrown14 as a code owner August 4, 2022 19:49
aharpt
aharpt commented Aug 4, 2022
View reviewed changes
stories/__tests__/SQFormDatePicker.stories.test.tsx
const dateOptions = within(calendarDialog).getAllByRole('cell');
const selectedDate = dateOptions[0];
const dateOptions = within(calendarDialog).getAllByRole('button');
const selectedDate = dateOptions[4];
Copy link
Contributor Author

@aharpt aharpt Aug 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had issues with this story not passing for me. We needed to actually get all of the buttons within the calendarDialog instead of the cells. Not sure if this a different in the new DatePicker markup or what. The 1st of the month corresponds to the fifth button inside the calendarDialog. A bit hacky, but the best I could figure out. I can revisit if needed, but don't think that it is very likely we would be adding/removing buttons that would cause this hard-coded value not to work.

Copy link
Contributor

@laurelbean laurelbean Aug 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will work for now. I already have a line item to go update the tests for this component once we upgrade to version 5.
See Issue #358

@aharpt
Copy link
Contributor Author

aharpt commented Aug 4, 2022 •
edited
Loading

Note: This PR also migrates the DatePickerWithDateFNS component. I had to do both components in one PR since the DatePickerWithDateFNS component relies on props from the regular date picker which was causing the tests to fail. I have a separate loom for each component if people are interested

This was linked to issues Aug 4, 2022
Migrate SQFormDatePicker to MUIv5 #745
Closed
Migrate SQFormDatePickerWithDateFNS MUIv5 #748
Closed
@aharpt aharpt mentioned this pull request Aug 5, 2022
Merged
@20BBrown14 20BBrown14 merged commit 9de243b into issue-447/MUI5_migration Aug 5, 2022
@20BBrown14 20BBrown14 deleted the issue-745/Migrate_DatePicker_MUI5 branch August 5, 2022 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@laurelbean laurelbean laurelbean approved these changes

@20BBrown14 20BBrown14 20BBrown14 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Migrate SQFormDatePickerWithDateFNS MUIv5 Migrate SQFormDatePicker to MUIv5
3 participants
@aharpt @laurelbean @20BBrown14