Skip to content
This repository has been archived by the owner on Jun 3, 2024. It is now read-only.

DatePickerSingle prop typos #361

Merged
merged 3 commits into from
Dec 11, 2018
Merged

DatePickerSingle prop typos #361

merged 3 commits into from
Dec 11, 2018

Conversation

tcbegley
Copy link
Contributor

@tcbegley tcbegley commented Nov 2, 2018

This simple PR corrects a couple of typos in props being passed to SingleDatePicker from react-dates inside the definition of DatePickerSingle.

See here and here

In particular this means the show_outside_days prop of DatePickerSingle from dash-core-components has the desired effect.

@tcbegley tcbegley changed the title Datepickersingle prop typos DatePickerSingle prop typos Nov 2, 2018
Copy link
Contributor

@T4rk1n T4rk1n left a comment

Choose a reason for hiding this comment

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

💃

@rmarren1
Copy link
Contributor

rmarren1 commented Nov 2, 2018

💃

@rmarren1 rmarren1 self-requested a review November 2, 2018 15:01
@Marc-Andre-Rivet
Copy link
Contributor

@tcbegley I see this PR has been sitting here for a few weeks now. All that's missing is a merge from master. Do you want to bring it through the finish line or can we do it ourselves? Whatever your preference, thanks for your contribution! 😄

@tcbegley
Copy link
Contributor Author

tcbegley commented Dec 11, 2018

Hey @Marc-Andre-Rivet, sorry, I had been meaning to chase this up actually. I don't have permission to merge, so please do go ahead and merge for me.

What's your preferred way of resolving the conflicts? I can rebase and rebuild, or maybe the bundles don't even really need to be included in this PR anyway? Let me know if you want me to sort it out before merging.

@Marc-Andre-Rivet
Copy link
Contributor

Marc-Andre-Rivet commented Dec 11, 2018

I don't have permission to merge
Ah! Didn't think of that. I guess that makes a lot of sense.

@tcbegley The contribution guideline is missing a few key points that were not raised during the review.

What's needed:

  1. resolve conflicts (just accept what comes from master and rebuild, push the rebuild's result -- the build has changed so there will be additional files generated)
  • bump patch version in package.json and version.py (we're currently at 0.40.3, so 0.40.4)
  1. update changelog
## [0.40.4] - 2018-12-11
### Fixed
- Fix typos in DatePickerSingle props [#361](https://github.com/plotly/dash-core-components/pull/361)
  1. updated build artefacts (yes, the build artefacts need to be updated in the PR with our current process)

If you'd rather I do the changes in the fork I'm more than willing too. Don't hesitate if you have additional questions.

@tcbegley
Copy link
Contributor Author

@Marc-Andre-Rivet That makes sense, happy to take a pass at that now and push for you to review.

@Marc-Andre-Rivet
Copy link
Contributor

@tcbegley Sorry if this follow up is a bit haphazard, this is the first community PR I pick up here 😧

@tcbegley
Copy link
Contributor Author

@Marc-Andre-Rivet no worries at all! You've been very helpful so far.

I've made the changes you suggested (with the exception of bumping to 0.40.5 as 0.40.4 appears to already be on master and PyPI, is that right?).

@Marc-Andre-Rivet
Copy link
Contributor

@tcbegley Yup, 0.40.5 is where it's at. There was a visual diff but it was not significant. Will squash and merge and follow up with the publish in a short while.

Copy link
Contributor

@Marc-Andre-Rivet Marc-Andre-Rivet left a comment

Choose a reason for hiding this comment

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

💃

@Marc-Andre-Rivet Marc-Andre-Rivet merged commit 636045f into plotly:master Dec 11, 2018
@tcbegley
Copy link
Contributor Author

I was wondering about that. I'm guessing to do with the fact that show_outside_days previously defaulted to true but didn't actually do anything because it was passed to enableOutSideDays which wasn't recognised by react-dates.

Anyway, thanks for the help and for merging!

@tcbegley tcbegley deleted the datepickersingle-prop-typos branch December 11, 2018 14:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants