[DatePicker] time support: timePrecision & timePickerProps #2856

giladgray · 2018-08-20T23:59:47Z

Changes proposed in this pull request:

add timePrecision and timePickerProps to shared date picker props
- and remove them from DateInputProps
DatePicker now includes time selection if one of the above props is defined
❌ Deprecate DateTimePicker since DatePicker can do it all now

blueprint-bot · 2018-08-21T00:03:07Z

fix dateinput test

Preview: documentation | landing | table

cmslewis · 2018-08-21T16:41:07Z

@giladgray nit: worth showing the time portion of the selected value beneath the DateInput in the example?

cmslewis

@giladgray You write some of the most elegant Enzyme tests this side of the Mississippi. Left some thoughts.

cmslewis · 2018-08-21T16:44:39Z

packages/datetime/src/_timepicker.scss

+  &:last-child {
+    margin-bottom: $datepicker-padding;
+  }
+}


I would consider adding 5px of margin above the time picker. Feels a little cramped right now, and with 5px more of separation, a selected date on the bottom row yields a symmetric 10px of white space both above and below the time picker.

cmslewis · 2018-08-21T16:48:36Z

packages/datetime/src/datePicker.tsx

@@ -264,6 +283,12 @@ export class DatePicker extends AbstractPureComponent<IDatePickerProps, IDatePic
        this.updateValue(value, true);
    };

+    private handleTimeChange = (time: Date) => {
+        Utils.safeInvoke(this.props.timePickerProps.onChange, time);
+        const newValue = DateUtils.getDateTime(this.state.value || new Date(), time);


nit: I prefer explicit boolean expressions. Makes the intent clear, and false-y values like 0 can throw off these shorthand expressions (less of an issue with timestamps that will likely never be 0 on purpose, but still).

e.g.

const { value } = this.state.value; const date = value != null ? value : new Date(); const newValue = DateUtils.getDateTime(date, time);

cmslewis · 2018-08-21T16:49:39Z

packages/datetime/src/datePickerCore.tsx

+     * `onChange` and `value` are ignored in favor of the corresponding
+     * top-level props on this component.
+     *
+     * Passing any defined value to this prop (even `{}`) will cause the


"even {}"

Good to note. 👍

cmslewis · 2018-08-21T16:50:36Z

packages/datetime/test/datePickerTests.tsx

 import { assertDatesEqual, assertDayDisabled, assertDayHidden } from "./common/dateTestUtils";

-describe("<DatePicker>", () => {
+describe.only("<DatePicker>", () => {


remove .only

cmslewis · 2018-08-21T16:53:03Z

packages/docs-app/src/examples/datetime-examples/common/momentDate.tsx

+export const MomentDate: React.SFC<{ date: Date; format?: string; withTime?: boolean }> = ({
+    date,
+    withTime = false,
+    format = withTime ? FORMAT_TIME : FORMAT,


Interesting. Didn't know you could use an earlier destructured value in the default value of another.

oh heck yeah you can! order matters.

cmslewis · 2018-08-21T16:55:18Z

packages/docs-app/src/examples/datetime-examples/common/precisionSelect.tsx

     */
-    allowEmpty?: boolean;
+    allowNone?: boolean;


Can we add a toggle for this in the timePickerExample? Just for completeness. Curious what it feels like in the wild.

oh it's not supported in TimePicker. if you're using that element then there's no concept of "none".

it's only for DatePicker where the TimePicker is conditionally rendered

blueprint-bot · 2018-08-21T18:19:30Z

strict boolean, no only

Preview: documentation | landing | table

blueprint-bot · 2018-08-21T18:20:51Z

top margin on timepicker

Preview: documentation | landing | table

themadcreator

1 question, otherwise approved

themadcreator · 2018-08-21T18:42:21Z

packages/datetime/src/datePicker.tsx

@@ -264,6 +283,13 @@ export class DatePicker extends AbstractPureComponent<IDatePickerProps, IDatePic
        this.updateValue(value, true);
    };

+    private handleTimeChange = (time: Date) => {
+        Utils.safeInvoke(this.props.timePickerProps.onChange, time);
+        const { value } = this.state;


is it theoretically possible that state.value would be changed as a result of invoking this.props.timePickerProps.onChange ? Also what if timePickerProps === null ???

null is not a valid value. it's X | undefined

* 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 13 commits August 20, 2018 16:04

add TimePicker props right in DatePicker!

cc5020b

getDateTime() to merge date and time

c487b98

remove "none" from allowed timePrecision

825c980

add style for TP in DP

831b8ff

refactors to MomentDate and PrecisionSelect to support time

e97ba44

add PrecisionSelect to DP and DRP examples

57da87d

import types, update styles (no divider)

00be0df

revert some example changes

d4abdb7

check null in caption

db61020

revert DRP example change

c6c7f95

DateInput only renders DatePicker

7e0ee9b

deprecate DateTimePicker

34c9206

fix dateinput test

c339984

giladgray requested review from cmslewis, themadcreator and adidahiya August 20, 2018 23:59

tests for time!

25647d9

cmslewis reviewed Aug 21, 2018

View reviewed changes

Gilad Gray added 2 commits August 21, 2018 11:11

strict boolean, no only

15276ff

top margin on timepicker

45b0bcb

themadcreator approved these changes Aug 21, 2018

View reviewed changes

giladgray merged commit 478e7a8 into develop Aug 21, 2018

giladgray deleted the gg/datepicker+time branch August 21, 2018 18:59

adidahiya mentioned this pull request Jun 18, 2019

Wrong maxTime displayed in DateRangeInput with timePickerProps #3509

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DatePicker] time support: timePrecision & timePickerProps #2856

[DatePicker] time support: timePrecision & timePickerProps #2856

giladgray commented Aug 20, 2018

blueprint-bot commented Aug 21, 2018

cmslewis commented Aug 21, 2018

cmslewis left a comment

cmslewis Aug 21, 2018

giladgray Aug 21, 2018

cmslewis Aug 21, 2018

cmslewis Aug 21, 2018

cmslewis Aug 21, 2018

cmslewis Aug 21, 2018

giladgray Aug 21, 2018

cmslewis Aug 21, 2018

giladgray Aug 21, 2018

giladgray Aug 21, 2018

blueprint-bot commented Aug 21, 2018

blueprint-bot commented Aug 21, 2018

themadcreator left a comment

themadcreator Aug 21, 2018

giladgray Aug 21, 2018

[DatePicker] time support: timePrecision & timePickerProps #2856

[DatePicker] time support: timePrecision & timePickerProps #2856

Conversation

giladgray commented Aug 20, 2018

Changes proposed in this pull request:

blueprint-bot commented Aug 21, 2018

fix dateinput test

cmslewis commented Aug 21, 2018

cmslewis left a 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

Choose a reason for hiding this comment

blueprint-bot commented Aug 21, 2018

strict boolean, no only

blueprint-bot commented Aug 21, 2018

top margin on timepicker

themadcreator left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment