feat: [M3-8611]- New DatePicker Component #11151
Conversation
This reverts commit b274baf.
|
@coliu-akamai Looks like the issue is with the AutoComplete component. I'm seeing a similar error for RegionSelect, ImageSelect, LinodeSelect as well in the Storybook. I will create a separate ticket to address this. The DatePicker components are working as expected in Storybook when rendered without TimeZoneSelect (which uses AutoComplete). For now, DatePickers in Storybook renders with ShowTimeZone set to false. To test the component with TimeZoneSelect, I recommend using the component in CM. |
There was a problem hiding this comment.
Unexpected behavior when typing in the TextField
If I click into the TextField and type the number 2, the cursor jumps to the end of the Text Field. It feels a bit unexpected. Is this something we can try to improve?
Screen.Recording.2024-12-09.at.3.23.59.PM.mov
Unexpected spacing around the "Ok" button
There is a lot of spacing around this button? Can we do anything about that?
| '& .MuiDayCalendar-weekDayLabel': { | ||
| fontSize: '0.875rem', | ||
| }, | ||
| '& .MuiPickersCalendarHeader-label': { |
There was a problem hiding this comment.
This seems like stuff that belongs in the MUI theme. Can we move styles like this into light.ts? Seems like this might help
There was a problem hiding this comment.
Agreed. Moving these to foundations/themes/light.ts is not reflecting in the Storybook component but is working in the CM app. I will address this in the following PR for the DateRange Picker - M3-8990.
@bnussman-akamai Thank you for the through check.
Chrome: Working as expected in Chrome. DatePicker-Typing.movFollowing PR's will address the below enhancements for the Date Range Picker Components.
|
| const newHour = newTime.hour; | ||
| const newMinute = newTime.minute; | ||
|
|
||
| if (typeof newHour === 'number' && typeof newMinute === 'number') { |
There was a problem hiding this comment.
Curious, are these not being inferred to where we need this check?
There was a problem hiding this comment.
Yes, I may have overthought this and taken a more defensive approach. In this context, we can safely rely on the Luxon DateTime object and expect hour and minute to always be numbers. I've simplified the logic accordingly. -
1ab71f9
| } | ||
| }; | ||
|
|
||
| const handleEndDateTimeChange = (newEnd: DateTime | null) => { |
There was a problem hiding this comment.
At the moment, this is the biggest issue I see at the moment. Context: #11151 (comment)
| const [endDateTime, setEndDateTime] = useState<DateTime | null>( | ||
| endDateTimeValue | ||
| ); | ||
| const [error, setError] = useState<string>(); |
There was a problem hiding this comment.
Yea I agree - it wouldn't actually be too much of a lift if we removed the useState hooks and relied entirely on props for values and change handling. The only downside when it comes to storybook is that we would need some sort of wrapper that manages the state (similar to how things are now) for testing/demos in storybook or something. Context: #11151 (comment)
packages/manager/src/components/DatePicker/DatePicker.stories.tsx
Outdated
Show resolved
Hide resolved
| value: null, | ||
| }; | ||
|
|
||
| describe('DatePicker', () => { |
There was a problem hiding this comment.
Can we add a test for typing the date in the input?
There was a problem hiding this comment.
Yes, I tried to cover this test but faced an issue because the TextField component currently allows free-form typing. Addressing this would require updating our TextField component to support a date type field or exploring alternative approaches to handle date-specific input more effectively. This also, involves in restricting free from typing on the input field. I will create a separate ticket to tackle this.
it.only('should handle typing a date and pressing Enter', async () => {
renderWithTheme(<DatePicker {...props} />);
const datePickerField = screen.getByRole('textbox', {
name: 'Select a date',
});
// Select all text and clear with Backspace
await userEvent.click(datePickerField);
// Type a valid date into the input field
await userEvent.type(datePickerField, '2024-10-25');
// Simulate pressing Enter
await userEvent.keyboard('{Enter}');
// Verify that the field value is updated
expect(datePickerField).toHaveValue('2024-10-25');
});
| value={selectedDateTime || null} | ||
| {...dateCalendarProps} | ||
| // TODO: Move styling customization to global theme styles. | ||
| sx={(theme: Theme) => ({ |
There was a problem hiding this comment.
Can we make the scrollbar thinner with something like scrollbar-width: thin; and remove the scrollbar for the AM/PM column?
Screen.Recording.2024-12-11.at.3.27.17.PM.mov
There was a problem hiding this comment.
This UX is going to change in the followup PR's where it will be one modal with Calendar and time. I will get this cosmetic change included as part of that.
packages/manager/src/components/DatePicker/DateTimeRangePicker.tsx
Outdated
Show resolved
Hide resolved
bnussman-akamai
added
Approved
There was a problem hiding this comment.
@hana-akamai To make it simple we currently disabled type functionality in the DateTimePicker component.
packages/manager/src/components/DatePicker/DateTimeRangePicker.tsx
Outdated
Show resolved
Hide resolved
| value: null, | ||
| }; | ||
|
|
||
| describe('DatePicker', () => { |
There was a problem hiding this comment.
Yes, I tried to cover this test but faced an issue because the TextField component currently allows free-form typing. Addressing this would require updating our TextField component to support a date type field or exploring alternative approaches to handle date-specific input more effectively. This also, involves in restricting free from typing on the input field. I will create a separate ticket to tackle this.
it.only('should handle typing a date and pressing Enter', async () => {
renderWithTheme(<DatePicker {...props} />);
const datePickerField = screen.getByRole('textbox', {
name: 'Select a date',
});
// Select all text and clear with Backspace
await userEvent.click(datePickerField);
// Type a valid date into the input field
await userEvent.type(datePickerField, '2024-10-25');
// Simulate pressing Enter
await userEvent.keyboard('{Enter}');
// Verify that the field value is updated
expect(datePickerField).toHaveValue('2024-10-25');
});
| value={selectedDateTime || null} | ||
| {...dateCalendarProps} | ||
| // TODO: Move styling customization to global theme styles. | ||
| sx={(theme: Theme) => ({ |
There was a problem hiding this comment.
This UX is going to change in the followup PR's where it will be one modal with Calendar and time. I will get this cosmetic change included as part of that.
Cloud Manager UI test results🎉 465 passing tests on test run #35 ↗︎
|
Cloud Manager E2E
|
|||||||||||||||||||||||||||||||||||||||||||||||||
| Project |
Cloud Manager E2E
|
| Branch Review |
develop
|
| Run status |
|
| Run duration | 33m 11s |
| Commit |
|
| Committer | cpathipa |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
2
|
Flaky
|
2
|
Pending
|
2
|
Skipped
|
0
|
Passing
|
467
|
| View all changes introduced in this branch ↗︎ | |
Tests for review
cypress/e2e/core/linodes/rebuild-linode.spec.ts • 2 failed tests
| Test | Artifacts | |
|---|---|---|
| rebuild linode > rebuilds a linode from Community StackScript |
Screenshots
Video
|
|
| rebuild linode > rebuilds a linode from Account StackScript |
Screenshots
Video
|
|
clone-linode.spec.ts • 1 flaky test
| Test | Artifacts | |
|---|---|---|
| clone linode > can clone a Linode from Linode details page |
Screenshots
Video
|
|
linode-config.spec.ts • 1 flaky test
| Test | Artifacts | |
|---|---|---|
| Linode Config management > End-to-End > Clones a config |
Screenshots
Video
|
|
Description 📝
This PR introduces an new Date picker component an abstraction of MUI date picker
Changes 🔄
List any change relevant to the reviewer.
Preview 📷
Include a screenshot or screen recording of the change
How to test 🧪
Verification steps
(How to verify changes)
As an Author I have considered 🤔
Check all that apply