-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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
fix(datetime): hide footer when month-year picker is open #25205
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.
Behavior and changes look good to me.
Question around accessibility. Now that the actions are no longer available, should we add secondary behavior to help screen reader users get back to the previous datetime state or do we expect them to traverse backwards to the expander, to collapse the month/year selection? UX that I am thinking of is ESC
key to collapse the section, but also know we use that interaction within overlays, so that may not be ideal.
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.
These changes look good. Re: Sean's accessibility question, we could change the month/year item in the top left (the button that says "April 2022") to read "hide year picker" when it is expanded and "show year picker" when it is collapsed. This is what iOS w/ VoiceOver does.
I played around with it some, and while I was able to get it reading "show year picker"/"hide year picker" after the month/year, the usability still isn't great 🤔 I'm going to merge this PR as-is and open a separate issue in a few, since there might be some deeper design discussions around it. |
First of all, thanks for the great work! |
Our users also reported that they would like a Done button on the Month / Year selector. Since the buttons are removed, it created confusion. From an UX point of view I believe what you want is to simply have |
Pull request checklist
Please check if your PR fulfills the following requirements:
npm run build
) was run locally and any changes were pushednpm run lint
) has passed locally and any fixes were made for failuresPull request type
Please check the type of change your PR introduces:
What is the current behavior?
When the month-year dropdown/picker is open, the cancel/done buttons look like they apply to the picker, rather than the whole datetime. This leads to confusing UI where the user might click Done to select a new month/year, but it closes the dialog instead and discards their choice.
Issue URL: #24998, #24962
What is the new behavior?
When the month/year picker is open, the datetime's footer is hidden. The user must collapse the picker to access the buttons again. They can still cancel their choice by clicking outside the overlay or pressing Escape.
Behavior is unchanged for the
month
,year
, andmonth-year
presentations; the footer will remain visible.Does this introduce a breaking change?
Other information
Native behavior is inconsistent across platforms:
We decided to go with this PR's behavior across platforms for better consistency and ease of maintenance. It clarifies the UI without adding extra layers to the component's behavior, i.e. changing what the
confirm
method does depending on the datetime's state.Side note: Technically,
monthYearPickerOpen
is redundant sinceshowMonthAndYear
would suffice, but I found this easier to reason about. I can tweak things if preferred.