-
Notifications
You must be signed in to change notification settings - Fork 17
Removing Material-ui datepicker #1367
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More comments:
- Date picker seems to overlap the text input.
- Each day's font-size is smaller than in the previous date picker.
- Focus behaviour is not working as expected. The
Tabshould move the focus throw the focusable elements inside the calendar dialog. These components are:- Year button.
- Month button.
- Previous month button.
- Next month button.
- Only the selected day. Right now, when you tab, the focus goes throw every day of the month, which is a bit tedious and not correct. Only the selected (active) day should be focusable. See this or any other calendar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary:
- The date picker is been pushed by the error message.
- Depending on the month and its string length, the right arrow (next month) changes its positioning.
The focus is not always working as expected:
- If you open the calendar and then click the date input, the date picker does not close.
- If you close the date picker with the
Esckey, the focus doesn't go back to the date input, as it was in the previous one or the one from W3C. - If you open the calendar, then click on a space (for example, the blank spaces on the right side of the selected month/year) and press any arrow button, the dialogue gains the focus:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review
General
- I think we should update the documentation in another PR. There are still some missing updates we need to do (images, descriptions, etc) and the PR could become too big.
Date Picker
- If the focus leaves the window, the date picker does not close.
- Why does the outline from the previous/next month's arrows have an offset of -2px? It doesn't make sense since it has enough space to appear. The same question goes for the year dropdown button.
- The box-shadow was designed to be complemented with a border (like in the Select) because on certain screens it can seem not visible:
- Sometimes, the focus leaves a kind of visual trace:
I think this is caused by the component dimensions plus the styling itself, maybe the outline offset and the border set to none are provoking this.
Year selector
- Inconsistent scroll: Quick arrow presses can make the scroll work a bit strange and not always centre the visually focused year.
- The focus has some sort of memory, which I think doesn't quite fit the intended functionality. I mean, if the focus goes out from the years' list and comes back, it goes to the last focused item.
Calendar
-
Focus proposal for the Calendar (currently, is not working as expected):
- If there's a day selected for the current month, the focus goes to this one.
- If there's no day selected but the actual date and the actual month are being displayed, the focus goes to the actual day.
- If both previous conditions are not true, the focus goes to the first day of the month.
-
If the day selected is from the previous/next month and it is also displayed in the current view (the light grey color days), it must be displayed as selected. See for example the calendar from Cloudscape
- The days are too close to each other horizontally, but they have a small amount of space vertically. I think both rows and columns should have the same gap.
- Same issue as in the year selector: The focus has some sort of memory, which I don't fully agree with. It makes it a bit unpredictable for the user to know where the focus is going to be placed since sometimes it's memorized and other times not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments:
- If you change the year, the current day appears focused instead of the first one.
- If you click on a day and then press the tab button, the calendar closes instead of going to the previous month's arrow.
- The scroll does not behave well yet. Same thoughts as in the last review and adding to that, we should remove the
smoothbehaviour of it, as other scrollable components are being used. - Tokens need to be updated based on the designer's choices and opinions. We cannot make any changes by ourselves.
- Storybooks:
- We need to check every state and from the buttons of the Year Picker and the Calendar (focus, hover, active, etc.)
- We need to check that the date picker appears over other components with a z-index higher than 0 (check the dropdown example to get an idea).
marcialfps
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
|
This branch is gonna be merged but there is still some work to be done. |











Checklist
(Check off all the items before submitting)
/libdirectory./websiteas needed.Purpose
Material-ui datepicker removed and added month picker to the new calendar
Additional context
Dayjs has been used to format and help to calculate the correct date within the calendar while navigating.
Closes #1368 #855