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.

Sign up for GitHub

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

Jump to bottom

Implement numberOfMonths #76

Merged
merged 1 commit into from
Apr 7, 2014
Merged

Implement numberOfMonths #76

merged 1 commit into from
Apr 7, 2014

Conversation

MoOx
Copy link
Contributor

@MoOx MoOx commented Jul 8, 2013

Here is a simple option to add more calendars. PR includes one example & README changes.

@MoOx
Copy link
Contributor Author

MoOx commented Jul 8, 2013

Using this new option & the tiny modifications it involves, I created an example page of a datepicker that support date ranges. I can make another PR to share this example (it will just be an example in the appropriate section) if this one is merged & if you are interested.

@MoOx
Copy link
Contributor Author

MoOx commented Jul 8, 2013

It seems I completly miss the numberOfMonths option. Do you want me to update that PR to use that name ?

@MoOx
Copy link
Contributor Author

MoOx commented Jul 17, 2013

Just to let you know I udpate this PR using numberOfMonths for the option name.

@jessegavin
Copy link

I would be super excited to see this pulled in.

@leads
Copy link

leads commented Sep 30, 2013

Great fix this. Here is a preview
http://embed.plnkr.co/fZNGNHyd8qW8kBnNDPZt/preview

I've set the startdate as today. Is there a way to make sure the current month of the start date is the month on the left. As you could have a blank month on the left.

This was referenced Jan 24, 2014
Open
Closed
@MoOx
Copy link
Contributor Author

MoOx commented Jan 24, 2014

I'm currently updating this PR (rebased fix a related bug (select month/year from title <select> broken)).

@MoOx
Copy link
Contributor Author

MoOx commented Jan 24, 2014

Hey guys, I completely rewrote this implementation. It's way better than before (I fix bugs related to prev|nextMonth & select on the title).

:shipit:

@MoOx
Copy link
Contributor Author

MoOx commented Jan 24, 2014

I also add an option to take control of the position as requested in this comment by @leads. This really make sense depending on what kind of date you are going to choose (futur or past).

@MoOx
Copy link
Contributor Author

MoOx commented Jan 24, 2014

Btw, I just saw #24 already did an similar implementation, but I think I got something more solid & in the same spirit of the existing codebase.

@rikkert
Copy link
Member

rikkert commented Jan 27, 2014

Like I mentioned in #24, I'm not sure if we should add this to master.
I appreciate the work you did. I was thinking we can publish this in a branch and mention it in the README.
Similar to what I did with time support, what do you think?

@MoOx
Copy link
Contributor Author

MoOx commented Jan 27, 2014

I think I do not create this lines https://github.com/dbushell/Pikaday/blob/master/pikaday.js#L207-L208
So it should be merged into master ( I don't want to maintain a fork for a "tiny" (not that much I agree) feature like this one. It's a must have imo.
I going to open source a "date ranger picker" based on this Pikaday version, so I would really love to be able to use the "original" Pikaday that include this small modification.

@rikkert
Copy link
Member

rikkert commented Jan 27, 2014

Ok, good point.
I want to release first and than I will look at it.

@MoOx MoOx mentioned this pull request Mar 31, 2014
Merged
@rikkert rikkert mentioned this pull request Mar 31, 2014
Closed
6 tasks
bjbrewster
bjbrewster reviewed Mar 31, 2014
View reviewed changes
examples/calendars.html

<h2>What is this?</h2>

<p>This is a development preview! Feel free to use it however you like but please report any bugs or feature requests to <a href="http://twitter.com/dbushell/">@dbushell</a> (on the GitHub issue tracker), thanks!</p>
Copy link

Choose a reason for hiding this comment

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

Is this comment still valid about being a dev preview?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No it's not. But this PR seems not considered for merge so I don't care for now.

@rikkert
Copy link
Member

rikkert commented Apr 2, 2014

Just merged this PR, but I see this when using 3 calendars:

screen shot 2014-04-02 at 08 31 01

How can we fix this?

@MoOx
Copy link
Contributor Author

MoOx commented Apr 2, 2014

Sorry just woke up, what is the issue here ?
Are you talking about calendar vertical alignment ?

rikkert
rikkert reviewed Apr 2, 2014
View reviewed changes
css/pikaday.css
.pika-lendar {
float: left;
width: 240px;
}
Copy link
Member

Choose a reason for hiding this comment

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

Should we add some default margin between the calendars?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe yes.

@rikkert
Copy link
Member

rikkert commented Apr 2, 2014

Yes, the jumping up of the second month.

@MoOx
Copy link
Contributor Author

MoOx commented Apr 2, 2014

Probably a simple fix. Let me take a look.

@rikkert
Copy link
Member

rikkert commented Apr 2, 2014

Don't worry. It is because of a bug in site.css that is used for the examples:

button {
margin-bottom: 3em;
}

@MoOx
Copy link
Contributor Author

MoOx commented Apr 2, 2014

Np. Good to know :)

@rikkert rikkert merged commit 1338fd5 into Pikaday:master Apr 7, 2014
@elzix elzix mentioned this pull request Jan 25, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@MoOx @jessegavin @leads @rikkert @bjbrewster