[datetime] fix(TimePicker): allow more natural text entry (#3762)

packages/datetime/src/_timepicker.scss

@@ -70,6 +70,16 @@ $timepicker-control-width: $pt-grid-size * 3.3 !default;
     &:focus {
       box-shadow: input-transition-shadow($input-shadow-color-focus, true);
     }
+
+    @each $intent, $color in $pt-intent-colors {
+      &.#{$ns}-intent-#{$intent} {
+        @include pt-input-intent($color);
+
+        .#{$ns}-dark & {
+          @include pt-dark-input-intent($color);
+        }
+      }
+    }
   }
 
   .#{$ns}-timepicker-ampm-select {

packages/datetime/src/timePicker.tsx

@@ -19,6 +19,7 @@ import {
     DISPLAYNAME_PREFIX,
     HTMLSelect,
     Icon,
+    Intent,
     IProps,
     Keys,
     Utils as BlueprintUtils,
@@ -229,9 +230,15 @@ export class TimePicker extends React.Component<ITimePickerProps, ITimePickerSta
     }
 
     private renderInput(className: string, unit: TimeUnit, value: string) {
+        const isValid = isTimeUnitValid(unit, parseInt(value, 10));
+
         return (
             <input
-                className={classNames(Classes.TIMEPICKER_INPUT, className)}
+                className={classNames(
+                    Classes.TIMEPICKER_INPUT,
+                    { [CoreClasses.intentClass(Intent.DANGER)]: !isValid },
+                    className,
+                )}
                 onBlur={this.getInputBlurHandler(unit)}
                 onChange={this.getInputChangeHandler(unit)}
                 onFocus={this.handleFocus}
@@ -261,12 +268,26 @@ export class TimePicker extends React.Component<ITimePickerProps, ITimePickerSta
 
     // begin method definitions: event handlers
 
-    private getInputBlurHandler = (unit: TimeUnit) => (e: React.SyntheticEvent<HTMLInputElement>) => {
+    private getInputChangeHandler = (unit: TimeUnit) => (e: React.SyntheticEvent<HTMLInputElement>) => {
         const text = getStringValueFromInputEvent(e);
-        this.updateTime(parseInt(text, 10), unit);
+        switch (unit) {
+            case TimeUnit.HOUR_12:
+            case TimeUnit.HOUR_24:
+                this.setState({ hourText: text });
+                break;
+            case TimeUnit.MINUTE:
+                this.setState({ minuteText: text });
+                break;
+            case TimeUnit.SECOND:
+                this.setState({ secondText: text });
+                break;
+            case TimeUnit.MS:
+                this.setState({ millisecondText: text });
+                break;
+        }
     };
 
-    private getInputChangeHandler = (unit: TimeUnit) => (e: React.SyntheticEvent<HTMLInputElement>) => {
+    private getInputBlurHandler = (unit: TimeUnit) => (e: React.SyntheticEvent<HTMLInputElement>) => {
         const text = getStringValueFromInputEvent(e);
         this.updateTime(parseInt(text, 10), unit);
     };

packages/datetime/test/dateRangePickerTests.tsx

@@ -1208,20 +1208,24 @@ describe("<DateRangePicker>", () => {
         });
 
         it("changing time does not change date", () => {
-            render({ timePrecision: "minute", defaultValue: defaultRange }).setTimeInput("minute", 10, "left");
+            render({ timePrecision: "minute", defaultValue: defaultRange }).setTimeInput("minute", "left", 10);
             assert.isTrue(DateUtils.areSameDay(onChangeSpy.firstCall.args[0][0] as Date, defaultRange[0]));
         });
 
         it("hovering over date does not change entered time", () => {
             const harness = render({ timePrecision: "minute", defaultValue: defaultRange });
-            harness.changeTimeInput("minute", 10, "left");
+            const newLeftMinute = 10;
+            harness.setTimeInput("minute", "left", newLeftMinute);
+            onChangeSpy.resetHistory();
             const { left } = harness;
             left.mouseEnterDay(5);
-            assert.equal((onChangeSpy.firstCall.args[0][0] as Date).getMinutes(), 10);
+            assert.isTrue(onChangeSpy.notCalled);
+            const minuteInputText = harness.getTimeInput("minute", "left");
+            assert.equal(parseInt(minuteInputText, 10), newLeftMinute);
         });
 
         it("changing time without date uses today", () => {
-            render({ timePrecision: "minute" }).setTimeInput("minute", 45, "left");
+            render({ timePrecision: "minute" }).setTimeInput("minute", "left", 45);
             assert.isTrue(DateUtils.areSameDay(onChangeSpy.firstCall.args[0][0] as Date, new Date()));
         });
 
@@ -1268,6 +1272,10 @@ describe("<DateRangePicker>", () => {
 
     function wrap(datepicker: JSX.Element) {
         const wrapper = mount<IDateRangePickerProps, IDateRangePickerState>(datepicker);
+
+        const findTimeInput = (precision: TimePrecision | "hour", which: "left" | "right") =>
+            wrapper.find(`.${DateClasses.TIMEPICKER}-${precision}`).at(which === "left" ? 0 : 1);
+
         // Don't cache the left/right day pickers into variables in this scope,
         // because as of Enzyme 3.0 they can get stale if the views change.
         const harness = {
@@ -1294,11 +1302,8 @@ describe("<DateRangePicker>", () => {
                     );
                 }
             },
-            changeTimeInput: (precision: TimePrecision | "hour", value: number, which: "left" | "right") =>
-                harness.wrapper
-                    .find(`.${DateClasses.TIMEPICKER}-${precision}`)
-                    .at(which === "left" ? 0 : 1)
-                    .simulate("change", { target: { value } }),
+            changeTimeInput: (precision: TimePrecision | "hour", which: "left" | "right", value: number) =>
+                findTimeInput(precision, which).simulate("change", { target: { value } }),
             clickNavButton: (which: "next" | "prev", navIndex = 0) => {
                 wrapper
                     .find(DatePickerNavbar)
@@ -1318,11 +1323,10 @@ describe("<DateRangePicker>", () => {
             getDays: (className: string) => {
                 return wrapper.find(`.${className}`).filterWhere(dayNotOutside);
             },
-            setTimeInput: (precision: TimePrecision | "hour", value: number, which: "left" | "right") =>
-                harness.wrapper
-                    .find(`.${DateClasses.TIMEPICKER}-${precision}`)
-                    .at(which === "left" ? 0 : 1)
-                    .simulate("blur", { target: { value } }),
+            getTimeInput: (precision: TimePrecision | "hour", which: "left" | "right") =>
+                findTimeInput(precision, which).props().value as string,
+            setTimeInput: (precision: TimePrecision | "hour", which: "left" | "right", value: number) =>
+                findTimeInput(precision, which).simulate("blur", { target: { value } }),
         };
         return harness;
     }

packages/datetime/test/timePickerTests.tsx

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { Keys } from "@blueprintjs/core";
+import { Classes as CoreClasses, Intent, Keys } from "@blueprintjs/core";
 import { assert } from "chai";
 import { mount } from "enzyme";
 import * as React from "react";
@@ -145,13 +145,25 @@ describe("<TimePicker>", () => {
         assert.strictEqual(hourInput.value, "2");
     });
 
-    it("does not allow invalid text entry", () => {
+    it("allows invalid text entry, but shows visual indicator", () => {
         renderTimePicker();
         const hourInput = findInputElement(Classes.TIMEPICKER_HOUR);
         assert.strictEqual(hourInput.value, "0");
 
         hourInput.value = "ab";
         TestUtils.Simulate.change(hourInput);
+        assert.strictEqual(hourInput.value, "ab");
+        assert.isTrue(hourInput.classList.contains(CoreClasses.intentClass(Intent.DANGER)));
+    });
+
+    it("reverts to saved value after invalid text entry is blurred", () => {
+        renderTimePicker();
+        const hourInput = findInputElement(Classes.TIMEPICKER_HOUR);
+        assert.strictEqual(hourInput.value, "0");
+
+        hourInput.value = "ab";
+        TestUtils.Simulate.change(hourInput);
+        TestUtils.Simulate.blur(hourInput);
         assert.strictEqual(hourInput.value, "0");
     });