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

DatePickerSingle prop typos #361

Merged
merged 3 commits into from Dec 11, 2018

Conversation

Projects
None yet
4 participants
@tcbegley
Copy link
Contributor

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

@T4rk1n

T4rk1n approved these changes Nov 2, 2018

Copy link
Contributor

left a comment

💃

@rmarren1

This comment has been minimized.

Copy link
Contributor

commented Nov 2, 2018

💃

@rmarren1 rmarren1 self-requested a review Nov 2, 2018

@Marc-Andre-Rivet

This comment has been minimized.

Copy link
Contributor

commented Dec 11, 2018

@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

This comment has been minimized.

Copy link
Contributor Author

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Contributor Author

commented Dec 11, 2018

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

@Marc-Andre-Rivet

This comment has been minimized.

Copy link
Contributor

commented Dec 11, 2018

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

@tcbegley tcbegley force-pushed the tcbegley:datepickersingle-prop-typos branch from 72581b3 to e260e99 Dec 11, 2018

@tcbegley

This comment has been minimized.

Copy link
Contributor Author

commented Dec 11, 2018

@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

This comment has been minimized.

Copy link
Contributor

commented Dec 11, 2018

@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.

@Marc-Andre-Rivet
Copy link
Contributor

left a comment

💃

@Marc-Andre-Rivet Marc-Andre-Rivet merged commit 636045f into plotly:master Dec 11, 2018

4 checks passed

ci/circleci: python-2.7 Your tests passed on CircleCI!
Details
ci/circleci: python-3.6 Your tests passed on CircleCI!
Details
ci/circleci: python-3.7 Your tests passed on CircleCI!
Details
percy/dash-core-components Visual review approved by Marc-André Rivet
Details
@tcbegley

This comment has been minimized.

Copy link
Contributor Author

commented Dec 11, 2018

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 tcbegley:datepickersingle-prop-typos branch Dec 11, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.