Daterangepicker nextMonth logic is incorrect #431

Closed
themadcreator opened this Issue Jan 9, 2017 · 4 comments

Projects

None yet

6 participants

@themadcreator
Contributor

Bug report

  • Package version(s): latest
  • Browser and OS versions: 1.5

https://github.com/palantir/blueprint/blob/master/packages/datetime/src/dateRangePicker.tsx#L401

This code uses 12 for december, but should be 11 since it's zero-based.

Correct code should read:

// 0 is january, 11 is december
function getNextMonth([month, year]: DisplayMonth): DisplayMonth {
  return month === 11 ? [0, year + 1] : [month + 1, year];
}

function getPreviousMonth([month, year]: DisplayMonth): DisplayMonth {
  return month === 0 ? [11, year - 1] : [month - 1, year];
}
@llorca llorca added this to the 1.6.0 milestone Jan 9, 2017
@cmslewis
Contributor
cmslewis commented Jan 9, 2017 edited

Would love to use named constants here. I found some of that code a little confusing when I read through it recently—in particular because, IIRC, some date stuff is in fact 1-indexed.

@giladgray
Member
giladgray commented Jan 9, 2017 edited

yeah not a bad idea.

const JANUARY = 0; 
const DECEMBER = 11;
@leebyp leebyp was assigned by giladgray Jan 9, 2017
@adidahiya
Member
adidahiya commented Jan 9, 2017 edited
import { Months } from "@blueprintjs/datetime";

Months.JANUARY; // 0

edit: just like Keys in core

@giladgray
Member

^ 👍 even better

@leebyp leebyp referenced this issue Jan 9, 2017
Merged

Expose months value mapping in datetime package #439

1 of 1 task complete
@giladgray giladgray closed this in #439 Jan 10, 2017
@giladgray giladgray added a commit that referenced this issue Jan 10, 2017
@leebyp @giladgray leebyp + giladgray expose Months value mapping in datetime package (#439)
* expose new `const enum Months`

* replace all month numbers with enum values throughout code and tests

* fixes #431
ab4f6a8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment