[DatePicker] reuse existing components in caption #2792

giladgray · 2018-08-10T19:54:36Z

Fixes #2801

Changes proposed in this pull request:

use minimal HTMLSelect for month/year dropdowns.
use minimal Button for next/previous buttons.
use navbarElement prop to render custom next/prev buttons instead of restyling the existing classes.
- caption is responsible for month/year selects
- navbar is responsible for prev/next buttons.
DatePicker uses single handleMonthChange for all DayPicker events
- caption (month/year select) and navbar (prev/next buttons) now all use the same logic for changes!
use Divider for all the internal separators
add HTMLSelect iconProps prop so we can position the icon

Screenshot

DatePicker

DateRangePicker

DateRangePicker contiguousCalendarMonths

massive reduction in styles

use Button for another reduction in styles

caption (month/year select) and navbar (prev/next buttons) now all use the same logic for changes!

assertSelectedDays() helper in DP tests replaces getSelectedDays()

blueprint-bot · 2018-08-10T19:56:15Z

fix & refactor caption & DP tests

Preview: documentation | landing | table

blueprint-bot · 2018-08-10T22:27:17Z

dateinput tests

Preview: documentation | landing | table

giladgray · 2018-08-10T22:54:36Z

#2793 will fix the failing DRP tests here.

cmslewis · 2018-08-13T16:54:54Z

packages/datetime/src/_datepicker.scss

+    display: flex;
+    justify-content: space-between;
+    margin-top: $pt-grid-size;
+    margin-bottom: -$datepicker-padding;


nit: Is there a way around using a negative margin?

cmslewis · 2018-08-13T16:56:03Z

packages/datetime/src/_datepicker.scss

-    border-top: 1px solid $pt-divider-black;
-    padding-top: $pt-grid-size / 2;
+    + .#{$ns}-icon {
+      right: 2px;


nit: should this be a multiple of $pt-grid-size?

in this case i think not. i really dislike using non-2 multiples of grid size because they're not reliable integers.

cmslewis · 2018-08-13T16:57:44Z

packages/datetime/src/_daterangepicker.scss

-  margin-left: -$pt-grid-size / 2;
-  min-width: $pt-grid-size * 15;
+  margin-top: -$datepicker-padding;
+  margin-left: -$datepicker-padding;


nit: same question regarding avoiding negative margins? Though I see we already had some in place before.

cmslewis · 2018-08-13T17:00:14Z

packages/datetime/src/datePickerCaption.tsx

-                />
-            </div>
+            <HTMLSelect
+                iconProps={{ style: { left: this.state.monthWidth } }}


Interesting. What's going on here? An icon is absolutely positioned inside the <select>? Oh, or is this referring to the caret? Can we just let the select morph to the width of the widest <option>, or does our style customization make that tricky?

EDIT: Took a look at the GIFs. Looks odd to me when the caret is right next to the text but the hoverable area goes way beyond the icon to the right. Can we just follow the style of default <select>s and leave the caret flush on the right side of the hoverable region?

@cmslewis i've got back and forth on this myself. you can disable the left property in the browser. i think it looks weirder with the two icons on either side of the year.

cmslewis · 2018-08-13T17:05:00Z

packages/datetime/src/datePickerCaption.tsx

            this.displayedMonthText,
            Classes.DATEPICKER_CAPTION_MEASURE,
            this.containerElement,
        );
-        this.setState({ monthWidth });
+        if (monthWidth > this.containerElement.firstElementChild.clientWidth - Icon.SIZE_STANDARD) {


~~Can you post some screenshots here to demonstrate each case by example?~~ Never mind.

cmslewis · 2018-08-13T17:05:43Z

packages/datetime/src/datePickerNavbar.tsx

@@ -0,0 +1,46 @@
+/*
+ * Copyright 2015 Palantir Technologies, Inc. All rights reserved.


cmslewis · 2018-08-13T17:05:55Z

packages/datetime/src/datePickerNavbar.tsx

+    minDate: Date;
+}
+
+export class DatePickerNavbar extends React.PureComponent<IDatePickerNavbarProps> {


yeah! love a good new feature.

cmslewis · 2018-08-13T17:07:26Z

packages/datetime/src/datePickerNavbar.tsx

+        const { classNames: classes, month, maxDate, minDate } = this.props;
+
+        return (
+            <div className={classNames(Classes.DATEPICKER_NAVBAR, classes.navBar)}>


Where does classes.navBar come from / what is its value? Not seeing it in the changed lines of the parent component(s). I assume it's just the ReactDayPicker default, and we need to provide it in order for basic ReactDayPicker styles to be applied in addition to our style overrides?

DayPicker itself. it's the actual .DayPicker-whatever class.

RDP does support customizing these classes, tho i don't think we should actually support that.

cmslewis · 2018-08-13T17:11:19Z

packages/datetime/test/dateInputTests.tsx

@@ -557,6 +564,7 @@ describe("<DateInput>", () => {

            wrapper
                .find(`.${Classes.DATEPICKER_MONTH_SELECT}`)
+                .find("select")
                .simulate("change", { value: Months.FEBRUARY.toString() });


Maybe add a changeSelectOption() helper?

function changeSelectOption(wrapper: ReactWrapper<any, any>, selectCssClass: string, value: string) { wrapper .find(`.${selectCssClass}`) .find("select") .simulate("change", { value }); }

Or more generically:

function changeSelectOption(wrapper: ReactWrapper<any, any>, selectSelector: string, value: string) {...}

Oops, meant to just Comment.

qcz · 2018-08-14T19:51:06Z

The month picker looks bad in Firefox on Windows:

In Edge the icon is still closer to the month name than in Chrome, but at least it does not overlap the name.

…icker-navbar2

also remove all negative margins

blueprint-bot · 2018-08-17T18:51:49Z

copyright, test helper

Preview: documentation | landing | table

giladgray · 2018-08-17T19:36:59Z

packages/datetime/src/_datepicker.scss

@@ -17,13 +17,24 @@ $header-margin: ($header-height - $pt-input-height) / 2 !default;
  line-height: 1;
 }

+.#{$ns}-divider {


TODO: gonna pull this out to its own component in a separate PR, in progress now.

…icker-navbar2

blueprint-bot · 2018-08-20T22:02:41Z

fix DRP tests

Preview: documentation | landing | table

blueprint-bot · 2018-08-20T22:24:52Z

adjust month select icon position

Preview: documentation | landing | table

themadcreator · 2018-08-20T22:36:20Z

packages/datetime/src/datePickerCaption.tsx

+                onChange={this.handleMonthSelectChange}
+                value={displayMonth}
+                options={monthOptionElements}
+            />


well this is great

themadcreator · 2018-08-20T22:38:52Z

packages/datetime/src/datePickerCaption.tsx

            this.displayedMonthText,
            Classes.DATEPICKER_CAPTION_MEASURE,
            this.containerElement,
        );
-        this.setState({ monthWidth });
+        const monthSelectWidth = this.containerElement.firstElementChild.clientWidth;


reaching into first child element for client width? makes me nervous that we're not doing more null checks

also this will trigger a layout

blueprint-bot · 2018-08-20T22:46:41Z

fix R15 tests

Preview: documentation | landing | table

* Fix tabs querySelector on jsdom (palantir#2761) * match event handler prop name with code and docs (palantir#2760) * matched icons with the original design (palantir#2780) * [Popover] captureDismiss=false by default (palantir#2776) * disable captureDismiss by default as it breaks links * fix tests * redirect old v1, v2 sites to new URLs (palantir#2773) * Remove `HACKHACK` (palantir#2790) BP requires `"@types/react": "^16.0.34"` and it is pinned by `yarn.lock`. For this version of typings, it is provided an overload, then the `HACKHACK` does not persist. ```ts function createElement<P>( type: SFC<P> | ComponentClass<P> | string, props?: Attributes & P, ...children: ReactNode[]): ReactElement<P>; ``` See issue: palantir#2785 * [RadioGroup] [HtmlSelect] options support className and disabled (palantir#2783) * Fix: <RadioGroup> passes custom className to options (+ test) * Fix: <HtmlSelect> passes custom className to options (+ test) * HTMLSelect supports disabled * test all options props * remove className test * [DatePicker] ❤️ (palantir#2789) * tests for DateUtils.getDateTime() * replace setStateWithValueIfUncontrolled with setState & updateValue helper refactor handlers to reduce let vars and clarify logic * refactor constructor into helper functions * merge disabled checks * fix test for initial state * massage imports * little bug fixes - ignoring next month change when null - correct day calculation - comments! * bump react-day-picker * areSameDay calls areSameMonth * reduce nesting * fix tests * Add Popover support for "auto-start" and "auto-end" (palantir#2772) * palantir#2770: Popover now accepts 'auto-start' and 'auto-end' positions * Update example * Update docs * Oops, undo unintended changes to example * [Button] Utils.isEmptyReactNode solves icon regression (palantir#2775) * Utils.isEmptyReactNode solves button regression * naming and test * rename in tests * Publish - @blueprintjs/core@3.3.0 * updated Sketch file (palantir#2813) * [TagInput] On paste, don't tag-ify text if no separator included (palantir#2804) * [TagInput] Leave a delimiter-less value in the input on paste * Update tests * Update docs * [DateRangePicker] all tests use enzyme (palantir#2793) * DRP tests use enzyme everywhere with a cool harness * name clash * fix tests in React 15 by using accessor to find latest element when needed * [new] Divider component (palantir#2854) * add Divider component * example * docs edits * replace all modifiers with borders * ignore coverage * remove fill from example * english is hard * [DatePicker] reuse existing components in caption (palantir#2792) * HTMLSelect: add iconProps, fix dark icon color * use HTMLSelect in caption massive reduction in styles * DatePickerNavbar renders prev/next Buttons use Button for another reduction in styles * cache month widths * DatePicker uses single handleMonthChange for all DayPicker events caption (month/year select) and navbar (prev/next buttons) now all use the same logic for changes! * add DPNavbar to DRP * fix & refactor caption & DP tests assertSelectedDays() helper in DP tests replaces getSelectedDays() * add $datepicker-padding variable * adjust paddings to use standard buttons in navbar * month icon won't exceed select bounds * dateinput tests * renames * replace borders with Divider elements also remove all negative margins * copyright, test helper * use Divider component * fix DRP tests * adjust month select icon position * margin only on caption * fix R15 tests * [DatePicker] time support: timePrecision & timePickerProps (palantir#2856) * add TimePicker props right in DatePicker! * getDateTime() to merge date and time * remove "none" from allowed timePrecision * add style for TP in DP * refactors to MomentDate and PrecisionSelect to support time * add PrecisionSelect to DP and DRP examples * import types, update styles (no divider) * revert some example changes * check null in caption * revert DRP example change * DateInput only renders DatePicker * deprecate DateTimePicker * fix dateinput test * tests for time! * strict boolean, no only * top margin on timepicker * refactors to reduce some complexity (palantir#2858) * [DateRangePicker] Shortcuts component and renderCalendars method (palantir#2859) * Shortcuts component and renderCalendars method to greatly simplify render() * bind handleNextState * Publish - @blueprintjs/icons@3.1.0 * remove dependencies section * [table] fix invisible table menu icon (palantir#2866) Fixes palantir#2865 * [Spinner] restore IE support (palantir#2868) * fix Spinner in IE by adding HTML wrapper tag for the animation * fix loading button spinner position * add tagName test * attempt to fix changing value on IE * added latest version (palantir#2862) * Publish - @blueprintjs/core@3.4.0 - @blueprintjs/table@3.1.1 * sketch file updated date * sandbox link in readme * remove quotes on $ns variable value (palantir#2881) * [Icon] render HTML element & tagName prop (palantir#2884) * Icon tagName prop and set `.bp3-icon-{name}` on element * refactor icon styles so svg is child - only render font glyph if the icon element is :empty * icon docs * color prop becomes fill attribute on svg, overrides css colors * fix tests * oops fix icon classes on non-icon elements (like callouts & buttons) * fix text ref (palantir#2888) * Skeleton: fix FF support! (palantir#2887) and refactor styles for simpler keyframes * [OverflowList] fix browser zoom behavior (palantir#2886) * fix OverflowList when zoomed * less magical number * [docs] better version tag styles (palantir#2889) * [docs] better version tag styles * remove obsolete flex override on tag icons * center docs-nav-buttons and key-combo * [Skeleton] Increase animation contrast (palantir#2885) * [Spinner] add additional child element to isolate spinner from parent (palantir#2890) * add spinner-animation element to isolate spinner from parent * comments about elements * Publish - @blueprintjs/core@3.5.0 * [Icon] revert to inline-block (palantir#2896) * clean up comments * take no chances * revert to inline-block and set block on svg instead of relying on flex child fixes all noted regressions * Publish - @blueprintjs/core@3.5.1 * Incorrect argument name (palantir#2898) Copied this over and realized that the argument should not be item but rather film. * [Suggest] Added selectedItem prop (palantir#2874) * Added selected item prop on the ISuggestProps def. * Init the Suggest state with the selected item prop. * Fixed Suggest support for controlled mode. * Fixed a tslint coma issue. * Made the state the only source of truth, added tests. * Added more tests, improved controlled mode support Now the Suggest does not update its underlying state in controlled mode, just like the EditableText component. * Added support for controlled empty selection. * prop docs * whitespace * [TagInput] Use $input-padding-horizontal when empty for consistency with <InputGroup> (palantir#2900) * Use -padding-horizontal in empty <TagInput> * Remove -empty class, use pure CSS approach * Add left-icon toggle to example * 🔧 switch to tree-sitter-typescript (palantir#2908) * switch to tree-sitter-typescript, move syntax pkgs to docs-data * update syntax tokens * 🔧 switch to circle-github-bot (palantir#2907) * add circle-github-bot * new preview script * restore cache in circle job * delete old scripts * [timepicker] Fix allowing to type time that exceeds time bounds (palantir#2795) * [DateRangePicker] support time selection (palantir#2895) * add time selection unit tests * add maybeRender placeholder * add wrapper div for calendars + time * add time precision to DRP example * reorder function * DateRangePicker now has time selection field * Updated DateRangePicker example to show time when precision is selected * Updated props to include timePickerProps and fixed/cleaned up tests for DateRangePicker * renamed timepicker classes appropiately, and code cleanup * updated momentTime to pass props * clicking on a shortcut doesn't change the time * removed only from describe in tests * removed unneeded CSS property and added test for making sure that time is preserved when un-selecting / reselecting date * Um/fix collapse animation (palantir#2911) * fixes collapse opening animation on first open * fix documentation * Add condensed property to HTML tables (palantir#2904) * Add condensed property to HTML tables * Move description to small instead of condensed * Deprecate small property on HTML tables * comment format * move collapse animation state docs onto the enum values, CLOSING_END -> CLOSING (palantir#2914) * bump sass-inline-svg (palantir#2915) * [docs] Modifiers & update DTP deprecation (palantir#2909) * add modifiers docs * update DTP deprecation notice * back to present tense since we're ready to ship * [Switch] fix switch styles variables for dark theme (palantir#2912) * [Select] Add resetOnQuery prop (palantir#2894) * [Select] Add resetActiveItemOnQuery prop * Switch prop name from resetActiveItemOnQuery to resetOnQuery and default prop to true * Match documentation to current functionality * Move default prop down to the lowest level, queryList

Gilad Gray added 7 commits August 10, 2018 12:46

HTMLSelect: add iconProps, fix dark icon color

f2a934b

use HTMLSelect in caption

ed2255d

massive reduction in styles

DatePickerNavbar renders prev/next Buttons

3162e53

use Button for another reduction in styles

cache month widths

752ef71

DatePicker uses single handleMonthChange for all DayPicker events

6fc7d0a

caption (month/year select) and navbar (prev/next buttons) now all use the same logic for changes!

add DPNavbar to DRP

5dbf33e

fix & refactor caption & DP tests

9201d55

assertSelectedDays() helper in DP tests replaces getSelectedDays()

giladgray added the Type: refactor label Aug 10, 2018

giladgray requested review from cmslewis and adidahiya August 10, 2018 19:54

Gilad Gray added 4 commits August 10, 2018 15:16

add $datepicker-padding variable

585e44c

adjust paddings to use standard buttons in navbar

20cc99b

month icon won't exceed select bounds

ee1aefe

dateinput tests

1446108

cmslewis previously approved these changes Aug 13, 2018

View reviewed changes

giladgray mentioned this pull request Aug 14, 2018

[DateTime] Drop dependency on Icon fonts #2801

Closed

Gilad Gray added 4 commits August 17, 2018 11:15

Merge branch 'develop' of github.com:palantir/blueprint into gg/datep…

c007d02

…icker-navbar2

renames

8596355

replace borders with Divider elements

ab916b5

also remove all negative margins

copyright, test helper

2be34c7

giladgray commented Aug 17, 2018

View reviewed changes

Gilad Gray added 2 commits August 20, 2018 14:42

Merge branch 'develop' of github.com:palantir/blueprint into gg/datep…

358de54

…icker-navbar2

use Divider component

8688d92

Gilad Gray added 2 commits August 20, 2018 15:18

fix DRP tests

205183d

adjust month select icon position

c49c583

giladgray force-pushed the gg/datepicker-navbar2 branch from 6a971fb to c49c583 Compare August 20, 2018 22:19

margin only on caption

e368464

themadcreator approved these changes Aug 20, 2018

View reviewed changes

fix R15 tests

915e290

giladgray merged commit 15cd895 into develop Aug 20, 2018

giladgray deleted the gg/datepicker-navbar2 branch August 20, 2018 23:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DatePicker] reuse existing components in caption #2792

[DatePicker] reuse existing components in caption #2792

giladgray commented Aug 10, 2018 •

edited

Loading

blueprint-bot commented Aug 10, 2018

blueprint-bot commented Aug 10, 2018

giladgray commented Aug 10, 2018

cmslewis Aug 13, 2018

cmslewis Aug 13, 2018

giladgray Aug 13, 2018

cmslewis Aug 13, 2018

cmslewis Aug 13, 2018

giladgray Aug 13, 2018

cmslewis Aug 13, 2018

cmslewis Aug 13, 2018

cmslewis Aug 13, 2018

giladgray Aug 13, 2018

cmslewis Aug 13, 2018

giladgray Aug 13, 2018 •

edited

Loading

cmslewis Aug 13, 2018

giladgray Aug 13, 2018

qcz commented Aug 14, 2018

blueprint-bot commented Aug 17, 2018

giladgray Aug 17, 2018

blueprint-bot commented Aug 20, 2018

blueprint-bot commented Aug 20, 2018

themadcreator Aug 20, 2018

themadcreator Aug 20, 2018

themadcreator Aug 20, 2018

blueprint-bot commented Aug 20, 2018

		@@ -0,0 +1,46 @@
		/*
		* Copyright 2015 Palantir Technologies, Inc. All rights reserved.

[DatePicker] reuse existing components in caption #2792

[DatePicker] reuse existing components in caption #2792

Conversation

giladgray commented Aug 10, 2018 • edited Loading

Fixes #2801

Changes proposed in this pull request:

Screenshot

blueprint-bot commented Aug 10, 2018

fix & refactor caption & DP tests

blueprint-bot commented Aug 10, 2018

dateinput tests

giladgray commented Aug 10, 2018

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

Choose a reason for hiding this comment

giladgray Aug 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qcz commented Aug 14, 2018

blueprint-bot commented Aug 17, 2018

copyright, test helper

Choose a reason for hiding this comment

blueprint-bot commented Aug 20, 2018

fix DRP tests

blueprint-bot commented Aug 20, 2018

adjust month select icon position

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blueprint-bot commented Aug 20, 2018

fix R15 tests

giladgray commented Aug 10, 2018 •

edited

Loading

giladgray Aug 13, 2018 •

edited

Loading