feat(datepicker): autoAdvanceCalendar - advance multimonth on select #2656
Conversation
foodforarabbit
requested review from
chasestarr,
gergelyke,
nadiia,
sandgraham and
tajo
as code owners
|
This pull request is being automatically deployed with ZEIT Now (learn more). 🔍 Inspect: https://zeit.co/uber-ui-platform/baseweb/3vt6gmxtf |
|
Nice feature!
|
| this.setState({ | ||
| date: this.getDateInView(), | ||
| }); | ||
| if (this.props.autoAdvanceCalendar) { |
There was a problem hiding this comment.
This condition here will prevent displaying the currently selected date when date typed in and autoAdvanceCalendar set to false only to fix the shift in a multi-month view.
I don't think in the current implementation adding autoAdvanceCalendar should be used as a fix for the #2471. I'm thinking we should add something like isInView check for an updated value.
As for the autoAdvanceCalendar independently from the bugfix, could you provide an example when would we want to have the opened calendar show a different that selected date value on open? Also what would be that different date?
There was a problem hiding this comment.
hmm the way I imagined the feature, the two modes of calendar would be
a) calendar view / months visible is "automatically" adjusted after it is open. The logic for adjusting the months is a separate issue (e.g. actually, we can do isInView check to fix #2471 it is still "automatically" adjusting the months if somehow the selected date was not visible)
b) calendar view / months visible is set automatically when opening the datepicker / popover, but after it is open, it is entirely manually controlled regardless of what else happens.
You are correct though, this is not really a direct solution for #2471 - more like I gave an alternate solution to prevent automatic control
Of course, we might still want to add isInView check so that calendar can have default autoAdvanceCalendar behaviour (i.e. original behaviour with view updating based on input value) but also not change views in multi-month when days are selected by clicking
There was a problem hiding this comment.
oh one more thought, is having the months advance in multi-month calendar a desired feature? Do we need another prop to switch between just the issue of months changing in multi-month mode (i.e. advance if not isInView feature)?
Please let me know what the best way to organize these features is!
There was a problem hiding this comment.
I think shifting calendar when a date is selected in a multi-month view creates not good user experience and we should just fix the shifting bug with something like isInView approach, I don't think it's something that should be customizable.
I can see where the very last day of the last visible month is selected and the shift would probably save an additional click to show later dates. But after checking a few popular services with calendars (non does that shift) and considering the second date in a range can be selected before or after the first selected date in our calendar implementation we should avoid that shift issue.
There was a problem hiding this comment.
ok that sounds reasonable, I will open up new PR with just that feature
|
Closing this one in favor of #2672 |
Fixes #2471
Description
Added option
autoAdvanceCalendar?: boolean (default: true)This option controls whether to shift months so that the first selected date appears in the first month (more relevant in multi-month calendar views)
Added yard config for option
Fixed autoFocusCalendar yard type (duplicates #2654)
Scope