Independent Daterangepicker months #575

leebyp · 2017-01-30T19:29:33Z

Fixes #432

Fix existing tests
Add new tests
Update documentation
DateRangePicker months set to be contiguous through prop
Extracted date comparison logic into separate class for readability

blueprint-bot · 2017-01-30T19:35:13Z

turn on all tests

Preview: docs | landing | table
Coverage: core | datetime

llorca · 2017-01-31T06:25:49Z

Feels pretty good. Nice to show the bounds of > 2 months ranges on each calendar through the shortcuts 👍

Worth asking: should we disallow "go next" in left calendar / "go prev" in right calendar when the 2 months are contiguous?

cmslewis · 2017-01-31T21:00:44Z

@llorca I guess it depends how valuable the interaction of shuffling both months together might be; that's the capability that would go away if we hide the buttons when the months are contiguous. I don't have a good sense for that at this point.

EDIT: But there is definitely some awkwardness/non-clarity there at this point. I could easily see this confusing some users as is.

llorca · 2017-01-31T22:47:05Z

@thisisalessandro would love to get your design input

sonovawolf · 2017-02-01T00:22:56Z

A few notes:

Disabling the arrows might be equally confusing: the mental gap between noticing that the arrows are disabled and understanding that it's because the months are contingent is big. Having the months "move" together is at least practical
Maybe the first click should always be interpreted as a start date. Right now if you pick a date and then pick another one that precedes the former, the first one counted as an end date and the second one as a start date. I think the most common case for pick a date prior to the one you just selected is because you picked a wrong start date, not because you are thinking in reverse
Related to☝️ : when we print the dates we represent the first click as a start date
Not super convinced by the fact that disabled days don't have a gray background. Especially in cases like this:

leebyp · 2017-02-01T00:52:03Z

re- @thisisalessandro 's points

Yeah that was my worry too. I think this is intrinsically linked with the decision of whether you can move the left date ahead of the right one directly with the dropdown.
This is related but should definitely not be in this PR - related to things @cmslewis is doing.
...
With the increased flexibility in the component now, I'm actually down to just set showOutsideDays to false - then it should be obvious which are disabled.

llorca · 2017-02-01T02:34:45Z

I'm not opposed to having showOutsideDays to false by default if you feel strongly 👍

cmslewis · 2017-02-01T02:39:29Z

@llorca @leebyp @thisisalessandro - just touched on outside days in my first PR on the road to merging my DateRangeInput work. I kind of think we should just get rid of them. Reasoning here:
https://github.com/palantir/blueprint/pull/586/files#r98813760

…ndent-daterangepicker-months

blueprint-bot · 2017-02-04T00:53:03Z

add comments to explain logic behind updating displayed months

Preview: docs | landing | table
Coverage: core | datetime

llorca · 2017-02-10T18:44:17Z

This is cool, we want it. Can we have it as a simple prop/option on the date range picker?

* Create new class for DateRangePicker state internals

blueprint-bot · 2017-02-13T19:36:01Z

Create new class for DateRangePicker state internals (#619)

Preview: docs | landing | table
Coverage: core | datetime

leebyp · 2017-02-13T22:59:15Z

@llorca the optional prop is implemented in #678

#678) * Add tests * set indepedent months feature through prop * update docs * update example in docs

blueprint-bot · 2017-02-15T16:39:08Z

Set independent months in DateRangePicker through props, and add tests (#678)

Preview: docs | landing | table
Coverage: core | datetime

cmslewis

Ooh, I'm going with "Request changes" instead of "Comment" for the first time. Left some questions and corrections. LMK when you're ready for review, @leebyp.

cmslewis · 2017-02-16T22:59:55Z

packages/datetime/src/_daterangepicker.scss

@@ -55,11 +55,26 @@ Styleguide components.datetime.daterangepicker.js
 .pt-daterangepicker {
  white-space: nowrap;

+  &.pt-daterangepicker-sequential {
+    .DayPicker {
+      min-width: $pt-grid-size * 43;


43 looks so arbitrary, haha. What's the width of each content column here? Can we make variables for (1) the width of each calendar and (2) the width of the shortcuts list, then add them in place here?

Extracted out the width for DayPicker since it's used to calculate the other widths. And made the css a bit more verbose to show what's going on. I haven't moved the shortcuts out, since it is arbitrary.

cmslewis · 2017-02-16T23:16:36Z

packages/datetime/src/_daterangepicker.scss

+
+    ~ .DayPicker {
+      border-left: 1px solid $pt-divider-black;
+      min-width: $pt-grid-size * 44;


Why are the min-widths different?

cmslewis · 2017-02-16T23:17:25Z

packages/datetime/src/common/classes.ts

@@ -18,6 +18,7 @@ export const DATEPICKER_MONTH_SELECT = "pt-datepicker-month-select";
 export const DATEPICKER_YEAR_SELECT = "pt-datepicker-year-select";

 export const DATERANGEPICKER = "pt-daterangepicker";
+export const DATERANGEPICKER_SEQUENTIAL = "pt-daterangepicker-sequential";


This class still abides by the old isSequential naming scheme. Can we change to pt-daterangepicker-contiguous or something?

cmslewis · 2017-02-16T23:18:09Z

packages/datetime/src/common/dateUtils.ts

+
+export function getDatePreviousMonth(date: Date): Date {
+    const monthIsJanuary = date.getMonth() === Months.JANUARY;
+    if (monthIsJanuary) {


nit: IMO the equality expression above is clear enough without this extra variable.

cmslewis · 2017-02-16T23:21:28Z

packages/datetime/src/common/monthAndYear.ts

+}
+
+// returns -ve if left < right
+// returns +ve if left > right


nit: the ...ve suffix took a sec for me to interpret. I guess I'm not mathy enough. Mild preference for negative and positive.

cmslewis · 2017-02-16T23:23:09Z

packages/datetime/test/dateRangePickerTests.tsx

+            renderDateRangePicker({ contiguousCalendarMonths, maxDate, minDate });
+            assert.lengthOf(document.getElementsByClassName("DayPicker"), 1);
+            assert.lengthOf(document.getElementsByClassName(".DayPicker-NavButton--prev"), 0);
+            assert.lengthOf(document.getElementsByClassName(".DayPicker-NavButton--next"), 0);


We don't want the dots in the class name when using document.getElementsByClassName. Won't return anything.

haha oops...

cmslewis · 2017-02-16T23:24:31Z

packages/datetime/test/dateRangePickerTests.tsx

        });

        it("is a day between minDate and maxDate if only maxDate/minDate set and today is not in range", () => {
            const maxDate = new Date(2005, Months.JANUARY);
            const minDate = new Date(2000, Months.JANUARY);
            renderDateRangePicker({ maxDate, minDate });
-            const { displayMonth, displayYear } = dateRangePicker.state;
-            assert.isTrue(DateUtils.isDayInRange(new Date(displayYear, displayMonth), [minDate, maxDate]));
+            const leftDisplayMonth = dateRangePicker.state.leftView;


nit: Pondering to myself it const leftView = ... would be clearer to me than const leftDisplayMonth = ..., since the display month includes a year. Thoughts @leebyp?

Ah yes thanks, this was leftover from refactors - updated a couple of other places i missed too.

blueprint-bot · 2017-02-17T20:34:51Z

more verbose css and add toggle to example

Preview: docs | landing | table
Coverage: core | datetime

blueprint-bot · 2017-02-17T20:39:02Z

missed capital letter

Preview: docs | landing | table
Coverage: core | datetime

leebyp · 2017-02-17T21:14:15Z

@cmslewis should be easier to read now

cmslewis · 2017-02-17T22:59:22Z

@leebyp love the "Show shortcuts" toggle and the CSS variables! The monthAndYear variables are still a little tricky, but I'm okay with them as is. My only remaining nit is to give the toggle labels a little more room if possible:

Otherwise, looks good IMO.

blueprint-bot · 2017-02-17T23:40:11Z

docs css

Preview: docs | landing | table
Coverage: core | datetime

llorca · 2017-02-20T19:00:27Z

Shouldn't it be "Constrain" instead of "Contrain"?

leebyp added 11 commits January 23, 2017 00:32

initial individual daterangepicker

f4e34af

hide arrow buttons when only showing one daterangepicker

016cf30

stagger left and right datepicker limits by one month

e7e1153

uncomment edge case

30abd94

delete old year select change

142214d

fix checking same year for #289 edge case

e9005d2

refactors

cd5a660

fix most tests

8307e93

reimplement minimal change of months

d03eac9

reset example

efae300

turn on all tests

440b301

leebyp added Status: in discussion Status: work in progress labels Jan 30, 2017

leebyp added 5 commits February 1, 2017 13:05

Merge branch 'master' of github.com:palantir/blueprint into bl/indepe…

a57051f

…ndent-daterangepicker-months

remove outside days

75f7793

css stylings

a483439

slight code cleanup

5338530

add comments to explain logic behind updating displayed months

8c5927c

leebyp mentioned this pull request Feb 4, 2017

Create new class for DateRangePicker state internals #619

Merged

llorca removed the Status: in discussion label Feb 10, 2017

Create new class for DateRangePicker state internals (#619)

cc3d5e4

* Create new class for DateRangePicker state internals

Set independent months in DateRangePicker through props, and add tests (

c01b7a2

#678) * Add tests * set indepedent months feature through prop * update docs * update example in docs

leebyp changed the title ~~(WIP) Independent Daterangepicker months~~ Independent Daterangepicker months Feb 15, 2017

leebyp requested review from cmslewis and giladgray February 15, 2017 16:38

leebyp removed the Status: work in progress label Feb 15, 2017

cmslewis suggested changes Feb 16, 2017

View reviewed changes

leebyp added 3 commits February 17, 2017 11:47

leftover refactor naming

fa890d6

more verbose css and add toggle to example

8c4b7f2

missed capital letter

ac2e906

cmslewis approved these changes Feb 17, 2017

View reviewed changes

docs css

baaaf19

leebyp merged commit f667b2c into master Feb 20, 2017

leebyp deleted the bl/independent-daterangepicker-months branch February 20, 2017 04:12

giladgray mentioned this pull request Feb 24, 2017

Prepare Release v1.10.0 #730

Merged

neevany mentioned this pull request Feb 28, 2017

Feature request: <DateRangeInput/> #249

Closed

10 tasks

cmslewis mentioned this pull request Mar 2, 2017

[Date Range Input] - Part 9: Obey and control the focused field #757

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Independent Daterangepicker months #575

Independent Daterangepicker months #575

leebyp commented Jan 30, 2017 •

edited

Loading

blueprint-bot commented Jan 30, 2017

llorca commented Jan 31, 2017

cmslewis commented Jan 31, 2017 •

edited

Loading

llorca commented Jan 31, 2017

sonovawolf commented Feb 1, 2017 •

edited

Loading

leebyp commented Feb 1, 2017 •

edited

Loading

llorca commented Feb 1, 2017

cmslewis commented Feb 1, 2017

blueprint-bot commented Feb 4, 2017

llorca commented Feb 10, 2017

blueprint-bot commented Feb 13, 2017

leebyp commented Feb 13, 2017

blueprint-bot commented Feb 15, 2017

cmslewis left a comment

cmslewis Feb 16, 2017 •

edited

Loading

leebyp Feb 17, 2017

cmslewis Feb 16, 2017

cmslewis Feb 16, 2017

cmslewis Feb 16, 2017

cmslewis Feb 16, 2017

cmslewis Feb 16, 2017

leebyp Feb 17, 2017

cmslewis Feb 16, 2017

leebyp Feb 17, 2017

blueprint-bot commented Feb 17, 2017

blueprint-bot commented Feb 17, 2017

leebyp commented Feb 17, 2017

cmslewis commented Feb 17, 2017 •

edited

Loading

blueprint-bot commented Feb 17, 2017

llorca commented Feb 20, 2017

Independent Daterangepicker months #575

Independent Daterangepicker months #575

Conversation

leebyp commented Jan 30, 2017 • edited Loading

Fixes #432

blueprint-bot commented Jan 30, 2017

turn on all tests

llorca commented Jan 31, 2017

cmslewis commented Jan 31, 2017 • edited Loading

llorca commented Jan 31, 2017

sonovawolf commented Feb 1, 2017 • edited Loading

leebyp commented Feb 1, 2017 • edited Loading

llorca commented Feb 1, 2017

cmslewis commented Feb 1, 2017

blueprint-bot commented Feb 4, 2017

add comments to explain logic behind updating displayed months

llorca commented Feb 10, 2017

blueprint-bot commented Feb 13, 2017

Create new class for DateRangePicker state internals (#619)

leebyp commented Feb 13, 2017

blueprint-bot commented Feb 15, 2017

Set independent months in DateRangePicker through props, and add tests (#678)

cmslewis left a comment

Choose a reason for hiding this comment

cmslewis Feb 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blueprint-bot commented Feb 17, 2017

more verbose css and add toggle to example

blueprint-bot commented Feb 17, 2017

missed capital letter

leebyp commented Feb 17, 2017

cmslewis commented Feb 17, 2017 • edited Loading

blueprint-bot commented Feb 17, 2017

docs css

llorca commented Feb 20, 2017

leebyp commented Jan 30, 2017 •

edited

Loading

cmslewis commented Jan 31, 2017 •

edited

Loading

sonovawolf commented Feb 1, 2017 •

edited

Loading

leebyp commented Feb 1, 2017 •

edited

Loading

cmslewis Feb 16, 2017 •

edited

Loading

cmslewis commented Feb 17, 2017 •

edited

Loading