-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Add DayPickerSingleDatePicker
component
#573
Conversation
Fantastic news! I'll check it out very soon, thanks Maja 👏 |
This is awesome! Thanks so much! |
@ljharb I've added a lot more content to the description to provide context for the changes if you would like to take a look! It does largely just entail moving around a functions that already existed and I do have confidence in the tests catching things. :D |
I just switched to this version in our library and replaced the uses of our custom implementation of I just have one question. |
100d80c
to
5275629
Compare
5275629
to
7d173c1
Compare
7d173c1
to
730acf8
Compare
730acf8
to
229579f
Compare
229579f
to
48dfbd1
Compare
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.
LGTM overall
} | ||
|
||
componentDidMount() { | ||
this.isTouchDevice = isTouchDevice(); |
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.
should this trigger a rerender, if it changes?
} | ||
|
||
getModifiersForDay(day) { | ||
return new Set(Object.keys(this.modifiers).filter(modifier => this.modifiers[modifier](day))); |
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.
should this be new Set(Object.values(this.modifiers).map(x => x(day)))
? (using object.values
ofc)
(fine as-is, too)
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.
values(visibleDays).forEach((days) => { | ||
Object.keys(days).forEach((day) => { | ||
const momentObj = moment(day); | ||
if (isDayBlocked(momentObj)) { |
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.
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.
Whoooops. Man, I knew I should have rebased. :|
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.
Okay, I think this is back now and released in 12.2.2. D:
Fix for #524
I've created a
DayPickerSingleDateController
to match theDayPickerRangeController
component. I've also added stories for it as well.The related PR for ranges is #167. The basic gist of this work is that all calendar related code has been moved into an abstraction layer in between the
DayPicker
and theSingleDatePicker
called theDayPickerSingleDateController
. Specifically this includes the day interaction callbacks (onDayMouseEnter
,onDayMouseLeave
, andonDayClick
) and all code relating to the modifiers.The following graphic should help explain the new structure somewhat (
DayPickerWithRangeControllers
maps to theDayPickerSingleDateController
).In a future PR, I will separate out the code related strictly to the input from the
SingleDatePicker
. After that theSingleDatePicker
will strictly contain code that coordinates focus and visibility between the input and the calendar dropdown.to: @tagoro9 @pokonski @airbnb/webinfra