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

feat(datepicker): keep day/month when adding/removing month/year in ngb-calendar #1277

Closed

Conversation

divdavem
Copy link
Member

@divdavem divdavem commented Jan 31, 2017

This pull request contains a change I am suggesting: I think calendar.getNext()/calendar.getPrev() with the period parameter set as 'm'/'y' should not automatically return the first day of the month / the first day of the year.
I think the day in the month and the month in the year of the input date should be kept as much as possible in the returned value. In case the destination month is is not long enough to contain a day with the same number, the last day of the month should be used.

For example:
calendar.getNext(new NgbDate(2016, 7, 22), 'm'))
previously returned: new NgbDate(2016, 8, 1))
with this PR, it returns: new NgbDate(2016, 8, 22))

calendar.getNext(new NgbDate(2016, 7, 22), 'y'))
previously returned: new NgbDate(2017, 1, 1))
with this PR, it returns: new NgbDate(2017, 7, 22))

calendar.getNext(new NgbDate(2016, 1, 30), 'm'))
previously returned: new NgbDate(2016, 2, 1))
with this PR, it returns: new NgbDate(2016, 2, 29))
(because 2016-02-30 is not a valid date)

Especially, I think this feature is useful for #1273 when pressing page up/page down to change the month or shift+page up/shift+page down to change the year.

@maxokorokov: What do you think?

…gb-calendar

BREAKING CHANGES: ngb-calendar getNext/getPrev specifications changed

`calendar.getNext()`/`calendar.getPrev()` with the period parameter set
as `'m'`/`'y'` no longer automatically returns the first day of the
month / the first day of the year. The day in the month and the month in
the year of the input date are now kept as much as possible.
In case the destination month is is not long enough to contain a day with
the same number, the last day of the month is used.

For example:

`calendar.getNext(new NgbDate(2016, 7, 22), 'm'))`
 previously returned: `new NgbDate(2016, 8, 1))`
  and now it returns: `new NgbDate(2016, 8, 22))`

`calendar.getNext(new NgbDate(2016, 7, 22), 'y'))`
 previously returned: `new NgbDate(2017, 1, 1))`
  and now it returns: `new NgbDate(2017, 7, 22))`

`calendar.getNext(new NgbDate(2016, 1, 30), 'm'))`
 previously returned: `new NgbDate(2016, 2, 1))`
  and now it returns: `new NgbDate(2016, 2, 29))`
 (because 2016-02-30 is not a valid date)
Copy link
Member

@maxokorokov maxokorokov left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!
We'll have to make sure that IslamicCalendar we have behaves the same way though...

Copy link
Member

@maxokorokov maxokorokov left a comment

Choose a reason for hiding this comment

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

I think we'll have to fix the IslamicCalendar to have consistent behaviour before merging this one

@pkozlowski-opensource
Copy link
Member

@maxokorokov @divdavem what is the status of this one? Is it still relevant?

@pkozlowski-opensource pkozlowski-opensource added this to the Purgatory milestone Mar 9, 2018
@pkozlowski-opensource pkozlowski-opensource removed this from the Purgatory milestone Aug 3, 2018
@maxokorokov maxokorokov modified the milestones: 3.1.0, 3.2.0 Aug 10, 2018
@maxokorokov maxokorokov self-assigned this Aug 28, 2018
@pkozlowski-opensource pkozlowski-opensource modified the milestones: 3.2.0, 3.3.0 Aug 30, 2018
@pkozlowski-opensource pkozlowski-opensource modified the milestones: 3.3.0, 3.4.0 Oct 5, 2018
@pkozlowski-opensource pkozlowski-opensource removed this from the 3.4.0 milestone Oct 19, 2018
@maxokorokov maxokorokov added this to the 5.1.2 milestone Sep 27, 2019
@maxokorokov
Copy link
Member

This commit was cherrypicked and merged in #3355

@maxokorokov maxokorokov closed this Oct 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants