Skip to content

Conversation

ghiculescu
Copy link

Calculate the correct number of rows for each month in a multi-month datepicker.

This means that different months might have different number of rows, which is in keeping with the overall style of the datepicker.

…423 - Datepicker: displaying multiple months shows an extra row of dates on months subsequent to a "long" month
@scottgonzalez
Copy link
Member

Sorry, but this is an unusable diff, with 1845 additions, 1846 deletions.

…Fixes #8423 - Datepicker: displaying multiple months shows an extra row of dates on months subsequent to a "long" month"

This reverts commit c843b45.
…423 - Datepicker: displaying multiple months shows an extra row of dates on months subsequent to a "long" month
@ghiculescu
Copy link
Author

Ooops, sorry. Reverted and re-committed properly.

@scottgonzalez
Copy link
Member

This is the first fix that we were trying to avoid. All visible calendars should be the same height. Honestly, I think the original behavior of always showing 6 rows was better because it avoids a height jump as you go from only months with 5 rows to any month with 6 rows.

@scottgonzalez
Copy link
Member

You can see the discussion about the first time this solution was used: #223

@ghiculescu
Copy link
Author

I guess the problem is that row counts are not consistent now. See for example the screenshot on bug #8423. We've gone with the solution here for my company's use of the datepicker, I think it would be good for jquery-ui to make some sort of fix because the current method definitely isn't perfect.

@scottgonzalez
Copy link
Member

I agree. As I've said, the correct solution is for all displayed months to have the same height, which would be the max of whatever is currently displayed. If you're interested in implementing that, we can land it, otherwise I think it's best to go back to the original implementation of always showing 6 rows.

@ghiculescu
Copy link
Author

I'll have a play with that over the next few days. Thanks for your patience.

@ghiculescu
Copy link
Author

I'm going to close this pull request for now because I haven't been able to think of a good way of implementing this.

@ghiculescu ghiculescu closed this Jul 9, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants