Custom content in CalendarDay with #renderDay

[evandavis]

Replaces #283

/**
 * @param day - the Moment object representing the day
 * @returns {string|element} - a single text or JSX node
 */
 // example
 // print a happy cat on Fridays
 <CalendarDay
   renderDay={day => (day.day() % 6 === 5 ? '😻' : day.format('D'))}
 />

[evandavis]

@majapw @ljharb Children.only doesn't accept strings, so I had to work around it a bit.

[majapw]

Okay, after more exploration, we kind of realized that we should probably just return the result directly because Children.only breaks on null, string, number, undefined... etc. X_x I thought this function did something slightly different then it does.

Basically we should just have {renderDay ? renderDay(day) : day.format('D')}directly in the` instead.

[coveralls]

Coverage increased (+0.09%) to 86.594% when pulling 1aa3fe3 on relayrides:issue-127_renderDay into 8a0c56d on airbnb:master.

[majapw]

Just the one comment, unfortunately, backtracking on our previous recommendation and then this is ready to go.

If you can squash your commits into just one, that would also be appreciated. Thank you!

[ljharb]

an alternative would be to get rid of the instance method entirely, and just define the defaultProp value for renderDay to be day => day.format('D'), and do {renderDay(day)} inline here?

[evandavis]

@ljharb the linter requires that we have default props at every level, so we would have to use renderDay: undefined instead of null in the wrapping components, and it seems like that pattern is discouraged. I think the instance method (or the ternary directly in render) is easier to reason about given that requirement.

[ljharb]

That's a good point, altho defaulting to undefined elsewhere would be fine too.

[evandavis]

@majapw updated, rebased, and squashed

[coveralls]

Coverage remained the same at 86.501% when pulling 8627454 on relayrides:issue-127_renderDay into 4c2c604 on airbnb:master.

[zachfeldman]

@evandavis I'm trying this out and returning the day, then some content in a span. Works pretty decently but the span gets rendered as plaintext (<, etc). Not sure if this is intentional but once again just throwing in my two cents!

[evandavis]

@zachfeldman can you post your renderDay function? It sounds like you are returning a string instead of a React node.

[zachfeldman]

@evandavis you're right :). Fixed it to a React node and now we're good! Thanks!

[evandavis]

@zachfeldman 👍 return an array if you need adjacent nodes: [day.format('D'), <span />]

[Bamieh]

it would be better to have the default renderDay function as (day) => day.format('D') and just call it always, no need for the if statement

[evandavis]

@Bamieh we already discussed that here: #307 (review)