Skip to content
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

[Issue 25] Unopinionated month and year selection support #1106

Merged
merged 6 commits into from
Jun 18, 2018

Conversation

GoGoCarl
Copy link
Contributor

@GoGoCarl GoGoCarl commented Apr 6, 2018

After tracking #25, which has been plaguing many users, myself included, I hope to have implemented, minimally, a stopgap solution to allow users to traverse months and years on the calendar, based on the excellent work contributed by @jvanderz22.

From the comments in the issue, it seems that the major sticking point for the past few months has been around design/UX. For this proposed solution, there is no design required from the maintainers; the caller is 100% responsible for the rendering.

To achieve this, this PR adds a single prop, renderCaption, which is expected to be a function that receives a data object as a param and returns a renderable custom caption (i.e. string, number, node). The data object passed to renderCaption contains the following parameters:

  • month -- a moment object for the current month
  • onMonthSelect -- a callback function that takes two params
    • month - the month moment object
    • value - the integer value of the month to change to
  • onYearSelect -- a callback function that takes two params
    • month - the month moment object
    • value - the integer value of the year to change to

Here is some sample code of how this could work with simple select inputs (this sample is also available in the storybook in this PR):

    <div style={{ display: 'flex', justifyContent: 'center' }}>
      <div>
        <select
          value={month.month()}
          onChange={(e) => { onMonthSelect(month, e.target.value); }}
        >
          {moment.months().map((label, value) => (
            <option value={value}>{label}</option>
          ))}
        </select>
      </div>
      <div>
        <select
          value={month.year()}
          onChange={(e) => { onYearSelect(month, e.target.value); }}
        >
          <option value={moment().year() - 1}>Last year</option>
          <option value={moment().year()}>{moment().year()}</option>
          <option value={moment().year() + 1}>Next year</option>
        </select>
      </div>
    </div>

Even if there is a solution later to present a pre-designed interface for selecting months and years, it would still be nice to have something like renderCaption to allow users to provide their own custom implementation.

As of this writing, this PR passes all tests and is updated to version 16.5.0.

@coveralls
Copy link

coveralls commented Apr 6, 2018

Coverage Status

Coverage decreased (-0.6%) to 84.785% when pulling 08c997c on GoGoCarl:month-year-selection-support into 4c641de on airbnb:master.

@majapw
Copy link
Collaborator

majapw commented Apr 12, 2018

This is awesome, and I'll review it a bit more thoroughly today! :)

@ljharb ljharb added the semver-minor: new stuff Any feature or API addition. label Apr 12, 2018
@@ -185,7 +194,8 @@ class CalendarMonth extends React.Component {
verticalScrollable && styles.CalendarMonth_caption__verticalScrollable,
)}
>
<strong>{monthTitle}</strong>
{renderCaption && renderCaption({ month, onMonthSelect, onYearSelect })}
{!renderCaption && <strong>{monthTitle}</strong>}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason not to make this be the default function for renderCaption?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, sounds great, I'll update that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, monthTitle here is potentially derived from the renderMonth props, so can't make this explicitly a defaultProp. But can still certainly clean this bit of code up.

initialMonthSubtraction -= 1;
}
newMonth.set('month', newMonthVal).subtract(initialMonthSubtraction, 'months');
this.props.onMonthChange(newMonth);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please destructure this out, so the “this” value of the function isn’t the props object.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha 👍

initialMonthSubtraction -= 1;
}
newMonth.set('year', newYearVal).subtract(initialMonthSubtraction, 'months');
this.props.onYearChange(newMonth);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please destructure this out, so the “this” value of the function isn’t the props object.

import moment from 'moment';

export default function isNextMonth(a, b) {
if (!moment.isMoment(a) || !moment.isMoment(b)) return false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what non-moment values do you expect in these functions? I’d prefer a throw when it’s an unexpected value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll update this and the other spot, thanks.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is to allow one or both of a and b to be null (startDate and endDate are sometimes null)

import moment from 'moment';

export default function isPrevMonth(a, b) {
if (!moment.isMoment(a) || !moment.isMoment(b)) return false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

Copy link
Contributor Author

@GoGoCarl GoGoCarl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review, I'll push an update to address your comments.

@@ -185,7 +194,8 @@ class CalendarMonth extends React.Component {
verticalScrollable && styles.CalendarMonth_caption__verticalScrollable,
)}
>
<strong>{monthTitle}</strong>
{renderCaption && renderCaption({ month, onMonthSelect, onYearSelect })}
{!renderCaption && <strong>{monthTitle}</strong>}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, sounds great, I'll update that.

initialMonthSubtraction -= 1;
}
newMonth.set('month', newMonthVal).subtract(initialMonthSubtraction, 'months');
this.props.onMonthChange(newMonth);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha 👍

import moment from 'moment';

export default function isNextMonth(a, b) {
if (!moment.isMoment(a) || !moment.isMoment(b)) return false;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll update this and the other spot, thanks.

@GoGoCarl
Copy link
Contributor Author

@ljharb I pushed a change to address a couple of your notes, but after some further review I left the type checks in the utils classes in place. The reason was because all the other similar utility classes, (i.e. https://github.com/airbnb/react-dates/blob/master/src/utils/isAfterDay.js) also do this check, so I think the code and behavior should remain consistent with what's already in place there. Thoughts?

@ljharb
Copy link
Member

ljharb commented Apr 13, 2018

Looks good; I’ll defer to @majapw about those functions (but consistency is probably better)

@kevinnio
Copy link

kevinnio commented Apr 30, 2018

My team is interested in having this PR merged. Can we have an estimate of when is this going to happen?

@jgnieuwhof
Copy link

+1 on this, we have a team here waiting on this feature to go in, is there an estimate on when this will be merged?

@gvincentlh
Copy link

We would really appreciate a merge as well 🙏

Copy link
Collaborator

@majapw majapw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

omg, i am so sorry for how long it has taken me to review this. @GoGoCarl I can address some of the comments I've made if that'll be easier. I think this is reasonable?

There are def some rough things about the react-dates architecture that makes this harder than it should be... :x

README.md Outdated
@@ -147,6 +147,7 @@ numberOfMonths: PropTypes.number,
keepOpenOnDateSelect: PropTypes.bool,
reopenPickerOnClearDates: PropTypes.bool,
renderCalendarInfo: PropTypes.func,
renderCaption: PropTypes.func,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add what these render functions expect to the README? Something like:

renderCaption: PropTypes.func, // ({ month, onMonthSelect, onYearSelect }) => PropTypes.node,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Certainly! I can also document the callbacks.

renderMonth,
renderCalendarDay,
renderDayContents,
renderCaption,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it weird to have both renderCaption and renderMonth? Should we better differentiate between the two?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do agree it is a bit weird.

My original thought was to modify the method signature of renderMonth to be something like renderMonth(month, { onMonthSelect, onYearSelect }). In order to do that, though, and keep compatible with the current UI, the resulting monthTitle would need to be wrapped in a <strong> tag only if it was a string, and I didn't know if adding that type check was the way to go.

So, I went with adding a new function instead mainly to preserve the old functionality of renderMonth without additional type checks. But if you think it makes more sense to do the above, I'm for it.

Otherwise, we can keep the two separate and having something like renderMonth and renderMonthElement, the latter being a rename of renderCaption that emphasizes the expected return value a bit better and linking the two functions.

I'm open to ideas. :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's leave renderMonth for right now and make the change to renderMonthElement in a follow-up PR (so as to separate out the breaking aspects)

@@ -204,6 +216,32 @@ class CalendarMonthGrid extends React.Component {
onMonthTransitionEnd();
}

onMonthSelect(currentMonth, newMonthVal) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

currentMonth is a momentObject, whereas newMonthVal is a zero-indexed integer?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes; similar to how currentMonth passes the full momentObject. So, really, currentMonth is currentDate. newMonthVal is a value compatible with momentObject.month(val) -- which indeed is a zero-indexed integer.

@@ -419,6 +430,30 @@ class DayPicker extends React.Component {
});
}

onMonthChange(currentMonth) {
// Translation value is a hack to force an invisible transition that
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:(

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very much open to suggestions, but the transition (or something like it) is needed for the re-render.

@@ -456,6 +484,10 @@ export default class DayPickerSingleDateController extends React.Component {
return { currentMonth, visibleDays };
}

setDayPickerRef(ref) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we use this ref?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was just added for consistency with other components, but if it's not used, happy to remove it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This must be a relic of some old code that never got cleaned up

import moment from 'moment';

export default function isNextMonth(a, b) {
if (!moment.isMoment(a) || !moment.isMoment(b)) return false;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is to allow one or both of a and b to be null (startDate and endDate are sometimes null)


export default function isNextMonth(a, b) {
if (!moment.isMoment(a) || !moment.isMoment(b)) return false;
return b.isSame(a.clone().add(1, 'month'), 'month');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've found isSame to be slow even when it's got a 'month' or 'day' or whatever argument. Maybe let's do:

return b.month() === a.clone().add(1, 'month').month();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only issue there is that a = 2017-01-01, b = 2018-02-01 would return true when it would be false. May not be a thing based on how the function is being called, but could be an issue down the line. To me, isSame may be slower, but it is safer.

Perhaps, if performance is a concern, checking a.year === b.year and a.month === b.month might help?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good call!


export default function isPrevMonth(a, b) {
if (!moment.isMoment(a) || !moment.isMoment(b)) return false;
return b.isSame(a.clone().subtract(1, 'month'), 'month');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've found isSame to be slow even when it's got a 'month' or 'day' or whatever argument. Maybe let's do:

return b.month() === a.clone().subtract(1, 'month').month();

@@ -0,0 +1,32 @@
import moment from 'moment';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isNextMonth_spec.js (one underscore pls)

Copy link
Contributor Author

@GoGoCarl GoGoCarl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the review @majapw! I am happy to address the changes to the extent that I can, and leave the rest to you to finalize. Please see my additional comments below.

README.md Outdated
@@ -147,6 +147,7 @@ numberOfMonths: PropTypes.number,
keepOpenOnDateSelect: PropTypes.bool,
reopenPickerOnClearDates: PropTypes.bool,
renderCalendarInfo: PropTypes.func,
renderCaption: PropTypes.func,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Certainly! I can also document the callbacks.

renderMonth,
renderCalendarDay,
renderDayContents,
renderCaption,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do agree it is a bit weird.

My original thought was to modify the method signature of renderMonth to be something like renderMonth(month, { onMonthSelect, onYearSelect }). In order to do that, though, and keep compatible with the current UI, the resulting monthTitle would need to be wrapped in a <strong> tag only if it was a string, and I didn't know if adding that type check was the way to go.

So, I went with adding a new function instead mainly to preserve the old functionality of renderMonth without additional type checks. But if you think it makes more sense to do the above, I'm for it.

Otherwise, we can keep the two separate and having something like renderMonth and renderMonthElement, the latter being a rename of renderCaption that emphasizes the expected return value a bit better and linking the two functions.

I'm open to ideas. :)

@@ -204,6 +216,32 @@ class CalendarMonthGrid extends React.Component {
onMonthTransitionEnd();
}

onMonthSelect(currentMonth, newMonthVal) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes; similar to how currentMonth passes the full momentObject. So, really, currentMonth is currentDate. newMonthVal is a value compatible with momentObject.month(val) -- which indeed is a zero-indexed integer.

@@ -419,6 +430,30 @@ class DayPicker extends React.Component {
});
}

onMonthChange(currentMonth) {
// Translation value is a hack to force an invisible transition that
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very much open to suggestions, but the transition (or something like it) is needed for the re-render.

@@ -456,6 +484,10 @@ export default class DayPickerSingleDateController extends React.Component {
return { currentMonth, visibleDays };
}

setDayPickerRef(ref) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was just added for consistency with other components, but if it's not used, happy to remove it.


export default function isNextMonth(a, b) {
if (!moment.isMoment(a) || !moment.isMoment(b)) return false;
return b.isSame(a.clone().add(1, 'month'), 'month');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only issue there is that a = 2017-01-01, b = 2018-02-01 would return true when it would be false. May not be a thing based on how the function is being called, but could be an issue down the line. To me, isSame may be slower, but it is safer.

Perhaps, if performance is a concern, checking a.year === b.year and a.month === b.month might help?

@majapw majapw force-pushed the month-year-selection-support branch from 8460a7e to 4efde5c Compare May 31, 2018 17:55
@GoGoCarl
Copy link
Contributor Author

All sounds good, thanks @majapw!

@majapw
Copy link
Collaborator

majapw commented May 31, 2018

\o/ Man i have been slow at getting to this. @GoGoCarl I don't want to put too much work on you randomly 2 months later :) so i'm gonna try to address my comments completely and get this merged in today.

I've noticed one small issue when the months change heights which I'll try to address before merging in. :)

@GoGoCarl
Copy link
Contributor Author

No worries, I'm just excited to see this getting closer to the finish line! If I can help out or debug just let me know, happy to help however I can.

@majapw majapw force-pushed the month-year-selection-support branch from 4efde5c to adf2cce Compare May 31, 2018 18:20
@majapw majapw force-pushed the month-year-selection-support branch 3 times, most recently from 494787c to a759dfb Compare June 5, 2018 20:28
@majapw
Copy link
Collaborator

majapw commented Jun 7, 2018

Hmm, I'm facing a weird issue where the height animation does not occur between non-neighboring months when using the custom caption. What's weird, is that neighboring months do trigger the animation, even when navigating via the caption. I think it has something to do with the transition time?

@bartkozal
Copy link

Hi 👋. I have already been using code from this PR as a patch in my app. There is one issue that you should be aware of. The caption is rendered "below" weekdays so in some cases (f.e. when custom components are used for select inputs) the weekdays overlap the content of the caption:

screen shot 2018-06-07 at 17 57 22

The way how the DOM tree looks like make it impossible to just use z-index. The current hacky workaround for me is to hide original weekdays with CSS and render them from renderCaption function:

const captionRenderer = ({ month, onMonthSelect, onYearSelect }) => {
  const weekdays = moment.weekdaysMin();
  return (
    <div>
      <div className="DayPicker_weekHeader">
        <ul className="DayPicker_weekHeader_ul">
          {weekdays.map(day => (
            <li key={day} className="DayPicker_weekHeader_li">
              {day}
            </li>
          ))}
        </ul>
      </div>
    // ...
    </div>
  );
}

@GoGoCarl
Copy link
Contributor Author

GoGoCarl commented Jun 8, 2018

@bkzl YES!! Thank you for mentioning this, I thought there was something else I wanted to flag but was convinced that it was the transition issue so I never thought about it again.

I, too, ran into the same issue and solved it the same way. I'll see if I can add a tweak to this that will help, unless you have some ideas on this @majapw

@majapw majapw force-pushed the month-year-selection-support branch from a759dfb to 4b72e5f Compare June 13, 2018 21:02
@majapw
Copy link
Collaborator

majapw commented Jun 13, 2018

Hello! I believe I've fixed the month transition issue. Maybe @GoGoCarl @ljharb, y'all can take a look at my last commit and see if it looks reasonable

@bkzl @GoGoCarl I dug into the z-index issue a bit, but was unable to come up with a satisfactory solution. Perhaps, the best bet right now is to merge this in as is and open up a separate issue to track the z-index challenge?

@majapw majapw force-pushed the month-year-selection-support branch from 4b72e5f to 11c01b2 Compare June 13, 2018 21:38
GoGoCarl and others added 5 commits June 18, 2018 14:05
Destructure a couple props to remove the `this` reference
and tidy up the display call to `renderCaption`.
- add story for daypickerrangecontroller
- document callbacks in readme
- make month comparison utils more performant
This required a slight change in how the height animation is handled. Namely,
the height animation is now always on after the initial render.
@majapw majapw force-pushed the month-year-selection-support branch from 8aa1fa8 to 08c997c Compare June 18, 2018 21:05
@majapw majapw merged commit ca3906b into react-dates:master Jun 18, 2018
@sauldeleon
Copy link
Contributor

Hi

I have downloaded the master branch of the library, as there is no TAG yet to include this PR.
As I have to build the library, I have found an error:

In the build process

> react-dates@16.7.0 build:css app/node_modules/react-dates
> node scripts/buildCSS.js "--optimize"

app/node_modules/react-dates/node_modules/react-outside-click-handler/build/OutsideClickHandler.js:42
  display: _propTypes2['default'].oneOf(Object.values(DISPLAY))TypeError: Object.values is not a function
    at Object.<anonymous> (app/node_modules/react-dates/node_modules/react-outside-click-handler/build/OutsideClickHandler.js:42:48)
    at Module._compile (module.js:570:32)
    at Module._extensions..js (module.js:579:10)
    at Object.require.extensions.(anonymous function) [as .js] (app/node_modules/react-dates/node_modules/babel-register/lib/node.js:152:7)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
    at Module.require (module.js:497:17)
    at require (internal/module.js:20:19)
    at Object.<anonymous> (app/node_modules/react-dates/node_modules/react-outside-click-handler/index.js:2:18)

This piece of code

var propTypes = (0, _airbnbPropTypes.forbidExtraProps)({
  children: _propTypes2['default'].node.isRequired,
  onOutsideClick: _propTypes2['default'].func.isRequired,
  disabled: _propTypes2['default'].bool,
  useCapture: _propTypes2['default'].bool,
  display: _propTypes2['default'].oneOf(Object.values(DISPLAY)),
})

Must be changed to

var propTypes = (0, _airbnbPropTypes.forbidExtraProps)({
  children: _propTypes2['default'].node.isRequired,
  onOutsideClick: _propTypes2['default'].func.isRequired,
  disabled: _propTypes2['default'].bool,
  useCapture: _propTypes2['default'].bool,
  display: _propTypes2['default'].oneOf(
    Object.keys(DISPLAY).map(function(key) {
      return DISPLAY[key]
    })
  ),
})

Or the build will crash.

There is another way of building the app or using this new feature?

Thanks in advance

@ljharb
Copy link
Member

ljharb commented Jun 20, 2018

@sauldeleon it sounds like you’re using an old version of node to do the build; try using node 8 or 10.

@sauldeleon
Copy link
Contributor

sauldeleon commented Jun 21, 2018

@ljharb thanks for the quick response.

You are totally right. Updating Node to the last version did the trick.

In addition, there is a warning regarding this PR

Warning: Failed prop type: DayPickerRangeController: unknown props found: onMonthChange, onYearChange
    in DayPickerRangeController
    in DateRangePicker
    in withStyles(DateRangePicker)
    in div
    in DateRangePickerWrapper
    in div
    in App

EDIT:
Fixed with adding

  onMonthChange: PropTypes.func,
  onYearChange: PropTypes.func,

on lines 88-89. I can do a PR if you want, but for this maybe its easier for you to do it!

EDIT 2: Done. To be reviewed:

here

@huchenme
Copy link

Any plan for next release? excited about this feature

@majapw
Copy link
Collaborator

majapw commented Jun 22, 2018

v17.0.0 coming v. soon (e.g. now :P)

@sauldeleon
Copy link
Contributor

WORKS like a charm! thanks @GoGoCarl @majapw @ljharb !!!!

@ardaplun
Copy link

ardaplun commented Jul 6, 2018

Is there are any explanation to use this year selection feature, please?

@majapw
Copy link
Collaborator

majapw commented Jul 7, 2018

Hi @ardaplun, you can take a look at https://github.com/airbnb/react-dates/blob/master/stories/DayPickerRangeController.js#L151-L182 to see the custom caption code in action!

@stahor
Copy link

stahor commented Jul 9, 2018

What about animation for onMonthChange? It can be the same one that is used for changing single month

@majapw
Copy link
Collaborator

majapw commented Jul 9, 2018

@stahor posting on a closed PR is probably not the best way to get a discussion going! :) Can you please open an issue?

@archy-bold
Copy link

Quick follow up to @bkzl's suggestion. You should ensure you get the days of the week in order relevant for the locale by passing true to weekdaysMin. Otherwise you always get them with Sunday as the first.

const weekdays = moment.weekdaysMin(true);

@whydna
Copy link

whydna commented Sep 17, 2018

@GoGoCarl @majapw I'm a bit confused - is the property to customize the control renderCaption or renderMonthElement? What's the difference?

@majapw
Copy link
Collaborator

majapw commented Sep 23, 2018

@whydna it was renderCaption in the original PR, but it was renaming to renderMonthElement to be a little bit more true to what it is actually doing. I would look at https://github.com/airbnb/react-dates/blob/master/stories/DayPickerRangeController.js#L151-L182 for the source code.

@goranefbl
Copy link

in case someone stumbles upon here, the source code file has been updated few times, so just look for renderMonthElement property, currently @ https://github.com/airbnb/react-dates/blob/master/stories/DayPickerRangeController.js#L210-L233

@gravitymedianet
Copy link

Hi 👋. I have already been using code from this PR as a patch in my app. There is one issue that you should be aware of. The caption is rendered "below" weekdays so in some cases (f.e. when custom components are used for select inputs) the weekdays overlap the content of the caption:

screen shot 2018-06-07 at 17 57 22

The way how the DOM tree looks like make it impossible to just use z-index. The current hacky workaround for me is to hide original weekdays with CSS and render them from renderCaption function:

const captionRenderer = ({ month, onMonthSelect, onYearSelect }) => {
  const weekdays = moment.weekdaysMin();
  return (
    <div>
      <div className="DayPicker_weekHeader">
        <ul className="DayPicker_weekHeader_ul">
          {weekdays.map(day => (
            <li key={day} className="DayPicker_weekHeader_li">
              {day}
            </li>
          ))}
        </ul>
      </div>
    // ...
    </div>
  );
}

Can you post a link to sample code to implement this? Year, month picker?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-minor: new stuff Any feature or API addition.
Projects
None yet
Development

Successfully merging this pull request may close these issues.