From 33f01bd9495e79ce84a2236d8b9ea04cf72e59b0 Mon Sep 17 00:00:00 2001 From: Chris Lewis <clewis@palantir.com> Date: Mon, 20 Nov 2017 00:24:44 -0800 Subject: [PATCH 1/8] DateInput now saves on Enter --- packages/datetime/src/dateInput.tsx | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/datetime/src/dateInput.tsx b/packages/datetime/src/dateInput.tsx index 78b7b83414..9c4114dc6f 100644 --- a/packages/datetime/src/dateInput.tsx +++ b/packages/datetime/src/dateInput.tsx @@ -16,6 +16,7 @@ import { InputGroup, IPopoverProps, IProps, + Keys, Popover, Position, Utils, @@ -248,6 +249,7 @@ export class DateInput extends AbstractComponent<IDateInputProps, IDateInputStat onChange={this.handleInputChange} onClick={this.handleInputClick} onFocus={this.handleInputFocus} + onKeyDown={this.handleInputKeyDown} value={dateString} /> </Popover> @@ -390,7 +392,7 @@ export class DateInput extends AbstractComponent<IDateInputProps, IDateInputStat }; private handleInputBlur = (e: React.FocusEvent<HTMLInputElement>) => { - const valueString = this.state.valueString; + const { valueString } = this.state; const value = this.createMoment(valueString); if ( valueString.length > 0 && @@ -420,6 +422,13 @@ export class DateInput extends AbstractComponent<IDateInputProps, IDateInputStat this.safeInvokeInputProp("onBlur", e); }; + private handleInputKeyDown = (e: React.KeyboardEvent<HTMLInputElement>) => { + if (e.which === Keys.ENTER) { + this.handleDateChange(new Date(this.state.valueString), true); + this.inputRef.blur(); + } + }; + private setInputRef = (el: HTMLElement) => { this.inputRef = el; const { inputProps = {} } = this.props; From f2d07f1d16ef6500af94c859c6fd5b4fdf33058a Mon Sep 17 00:00:00 2001 From: Chris Lewis <clewis@palantir.com> Date: Mon, 20 Nov 2017 00:52:02 -0800 Subject: [PATCH 2/8] DateRangeInput now saves on Enter --- packages/datetime/src/dateRangeInput.tsx | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/packages/datetime/src/dateRangeInput.tsx b/packages/datetime/src/dateRangeInput.tsx index 3e340d1edb..56f09af7e2 100644 --- a/packages/datetime/src/dateRangeInput.tsx +++ b/packages/datetime/src/dateRangeInput.tsx @@ -537,9 +537,12 @@ export class DateRangeInput extends AbstractComponent<IDateRangeInputProps, IDat // - if focused in start field, Tab moves focus to end field // - if focused in end field, Shift+Tab moves focus to start field private handleInputKeyDown = (e: React.KeyboardEvent<HTMLInputElement>) => { - const isTabPressed = e.keyCode === Keys.TAB; + const isTabPressed = e.which === Keys.TAB; + const isEnterPressed = e.which === Keys.ENTER; const isShiftPressed = e.shiftKey; + const { selectedStart, selectedEnd } = this.state; + // order of JS events is our enemy here. when tabbing between fields, // this handler will fire in the middle of a focus exchange when no // field is currently focused. we work around this by referring to the @@ -557,6 +560,21 @@ export class DateRangeInput extends AbstractComponent<IDateRangeInputProps, IDat } else if (wasEndFieldFocused && isTabPressed && isShiftPressed) { isStartInputFocused = true; isEndInputFocused = false; + } else if (wasStartFieldFocused && isEnterPressed) { + const nextStartValue = new Date(this.state.startInputString); + const nextEndValue = isMomentNull(selectedEnd) ? undefined : fromMomentToDate(selectedEnd); + this.handleDateRangePickerChange([nextStartValue, nextEndValue] as DateRange); + isStartInputFocused = false; + isEndInputFocused = true; + } else if (wasEndFieldFocused && isEnterPressed) { + const nextStartValue = isMomentNull(selectedStart) ? undefined : fromMomentToDate(selectedStart); + const nextEndValue = new Date(this.state.endInputString); + this.handleDateRangePickerChange([nextStartValue, nextEndValue] as DateRange); + isStartInputFocused = false; + isEndInputFocused = false; + // need to explicitly blur, because only .focus() is triggered in + // componentShouldUpdate. + this.endInputRef.blur(); } else { // let the default keystroke happen without side effects return; From 4f9b09868514e4a034c49130a6780f884bbdaaa7 Mon Sep 17 00:00:00 2001 From: Chris Lewis <clewis@palantir.com> Date: Mon, 20 Nov 2017 01:26:39 -0800 Subject: [PATCH 3/8] Write DateInput tests, fix locale bug --- packages/datetime/src/dateInput.tsx | 4 +++- packages/datetime/test/dateInputTests.tsx | 23 +++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/packages/datetime/src/dateInput.tsx b/packages/datetime/src/dateInput.tsx index 9c4114dc6f..f1407161e1 100644 --- a/packages/datetime/src/dateInput.tsx +++ b/packages/datetime/src/dateInput.tsx @@ -424,7 +424,9 @@ export class DateInput extends AbstractComponent<IDateInputProps, IDateInputStat private handleInputKeyDown = (e: React.KeyboardEvent<HTMLInputElement>) => { if (e.which === Keys.ENTER) { - this.handleDateChange(new Date(this.state.valueString), true); + const nextValue = this.createMoment(this.state.valueString); + const nextDate = fromMomentToDate(nextValue); + this.handleDateChange(nextDate, true); this.inputRef.blur(); } }; diff --git a/packages/datetime/test/dateInputTests.tsx b/packages/datetime/test/dateInputTests.tsx index c1427e52e1..365ff564c5 100644 --- a/packages/datetime/test/dateInputTests.tsx +++ b/packages/datetime/test/dateInputTests.tsx @@ -145,6 +145,15 @@ describe("<DateInput>", () => { }); describe("when uncontrolled", () => { + it("Pressing Enter saves the inputted date and closes the popover", () => { + const wrapper = mount(<DateInput />).setState({ isOpen: true }); + const input = wrapper.find("input").first(); + input.simulate("change", { target: { value: "2/15/15" } }); + input.simulate("keydown", { which: Keys.ENTER }); + assert.isFalse(wrapper.state("isOpen")); + assert.equal(wrapper.find(InputGroup).prop("value"), "2015-02-15"); + }); + it("Clicking a date puts it in the input box and closes the popover", () => { const wrapper = mount(<DateInput />).setState({ isOpen: true }); assert.equal(wrapper.find(InputGroup).prop("value"), ""); @@ -323,6 +332,20 @@ describe("<DateInput>", () => { const DATE2_STR = "2015-02-01"; const DATE2_DE_STR = "01.02.2015"; + it("Pressing Enter saves the inputted date and closes the popover", () => { + const onChange = sinon.spy(); + const { root } = wrap(<DateInput onChange={onChange} value={DATE} />); + root.setState({ isOpen: true }); + + const input = root.find("input").first(); + input.simulate("change", { target: { value: DATE2_STR } }); + input.simulate("keydown", { which: Keys.ENTER }); + + // onChange is called once on change, once on Enter + assert.isTrue(onChange.calledTwice, "onChange called twice"); + assertDateEquals(onChange.args[1][0], DATE2_STR); + }); + it("Clicking a date invokes onChange callback with that date", () => { const onChange = sinon.spy(); const { getDay, root } = wrap(<DateInput onChange={onChange} value={DATE} />); From b942beee40d296d1668958312cba8ea393e6efb8 Mon Sep 17 00:00:00 2001 From: Chris Lewis <clewis@palantir.com> Date: Mon, 20 Nov 2017 02:00:32 -0800 Subject: [PATCH 4/8] Tests + bug fixes for DateRangeInput --- packages/datetime/src/dateRangeInput.tsx | 14 +++-- .../datetime/test/dateRangeInputTests.tsx | 56 ++++++++++++++++++- 2 files changed, 63 insertions(+), 7 deletions(-) diff --git a/packages/datetime/src/dateRangeInput.tsx b/packages/datetime/src/dateRangeInput.tsx index 56f09af7e2..40cc916d58 100644 --- a/packages/datetime/src/dateRangeInput.tsx +++ b/packages/datetime/src/dateRangeInput.tsx @@ -561,15 +561,17 @@ export class DateRangeInput extends AbstractComponent<IDateRangeInputProps, IDat isStartInputFocused = true; isEndInputFocused = false; } else if (wasStartFieldFocused && isEnterPressed) { - const nextStartValue = new Date(this.state.startInputString); - const nextEndValue = isMomentNull(selectedEnd) ? undefined : fromMomentToDate(selectedEnd); - this.handleDateRangePickerChange([nextStartValue, nextEndValue] as DateRange); + const nextStartValue = this.dateStringToMoment(this.state.startInputString); + const nextStartDate = fromMomentToDate(nextStartValue); + const nextEndDate = isMomentNull(selectedEnd) ? undefined : fromMomentToDate(selectedEnd); + this.handleDateRangePickerChange([nextStartDate, nextEndDate] as DateRange); isStartInputFocused = false; isEndInputFocused = true; } else if (wasEndFieldFocused && isEnterPressed) { - const nextStartValue = isMomentNull(selectedStart) ? undefined : fromMomentToDate(selectedStart); - const nextEndValue = new Date(this.state.endInputString); - this.handleDateRangePickerChange([nextStartValue, nextEndValue] as DateRange); + const nextStartDate = isMomentNull(selectedStart) ? undefined : fromMomentToDate(selectedStart); + const nextEndValue = this.dateStringToMoment(this.state.endInputString); + const nextEndDate = fromMomentToDate(nextEndValue); + this.handleDateRangePickerChange([nextStartDate, nextEndDate] as DateRange); isStartInputFocused = false; isEndInputFocused = false; // need to explicitly blur, because only .focus() is triggered in diff --git a/packages/datetime/test/dateRangeInputTests.tsx b/packages/datetime/test/dateRangeInputTests.tsx index 2d06e440f7..8c5557a19d 100644 --- a/packages/datetime/test/dateRangeInputTests.tsx +++ b/packages/datetime/test/dateRangeInputTests.tsx @@ -9,7 +9,7 @@ import { mount, ReactWrapper } from "enzyme"; import * as React from "react"; import * as sinon from "sinon"; -import { HTMLInputProps, IInputGroupProps, InputGroup, Popover, Position } from "@blueprintjs/core"; +import { HTMLInputProps, IInputGroupProps, InputGroup, Keys, Popover, Position } from "@blueprintjs/core"; import { Months } from "../src/common/months"; import { Classes as DateClasses, DateRange, DateRangeBoundary, DateRangeInput, DateRangePicker } from "../src/index"; import * as DateTestUtils from "./common/dateTestUtils"; @@ -354,6 +354,32 @@ describe("<DateRangeInput>", () => { assertInputTextsEqual(root, START_STR, END_STR); }); + it("Pressing Enter saves the inputted date and closes the popover", () => { + const { root } = wrap(<DateRangeInput />); + root.setState({ isOpen: true }); + + const startInput = getStartInput(root); + startInput.simulate("focus"); + startInput.simulate("change", { target: { value: START_STR } }); + startInput.simulate("keydown", { which: Keys.ENTER }); + expect(isStartInputFocused(root), "start input blurred next").to.be.false; + + expect(root.state("isOpen"), "popover still open").to.be.true; + + const endInput = getEndInput(root); + expect(isEndInputFocused(root), "end input focused next").to.be.true; + endInput.simulate("change", { target: { value: END_STR } }); + endInput.simulate("keydown", { which: Keys.ENTER }); + + expect(startInput.prop("value")).to.equal(START_STR); + expect(endInput.prop("value")).to.equal(END_STR); + + expect(root.state("isOpen"), "popover closed at end").to.be.false; + expect(isStartInputFocused(root), "start input blurred at end").to.be.false; + // TODO (clewis): fix failing statement (works in practice) + // expect(isEndInputFocused(root), "end input blurred at end").to.be.false; + }); + it("Clicking a date invokes onChange with the new date range and updates the input fields", () => { const defaultValue = [START_DATE, null] as DateRange; @@ -2024,6 +2050,34 @@ describe("<DateRangeInput>", () => { assertInputTextsEqual(root, START_STR_2, END_STR_2); }); + it("Pressing Enter saves the inputted date and closes the popover", () => { + const onChange = sinon.spy(); + const { root } = wrap(<DateRangeInput onChange={onChange} value={[undefined, undefined]} />); + root.setState({ isOpen: true }); + + const startInput = getStartInput(root); + startInput.simulate("focus"); + startInput.simulate("change", { target: { value: START_STR } }); + startInput.simulate("keydown", { which: Keys.ENTER }); + expect(isStartInputFocused(root), "start input blurred next").to.be.false; + + expect(root.state("isOpen"), "popover still open").to.be.true; + + const endInput = getEndInput(root); + expect(isEndInputFocused(root), "end input focused next").to.be.true; + endInput.simulate("change", { target: { value: END_STR } }); + endInput.simulate("keydown", { which: Keys.ENTER }); + + expect(isStartInputFocused(root), "start input blurred at end").to.be.false; + // TODO (clewis): fix failing statement (works in practice) + // expect(isEndInputFocused(root), "end input blurred at end").to.be.false; + + // onChange is called once on change, once on Enter + expect(onChange.callCount, "onChange called four times").to.equal(4); + // check one of the invocations + assertDateRangesEqual(onChange.args[1][0], [START_STR, null]); + }); + it("Clicking a date invokes onChange with the new date range and updates the input field text", () => { const onChange = sinon.spy(); const { root, getDayElement } = wrap(<DateRangeInput value={DATE_RANGE} onChange={onChange} />); From daf7b2e28d03bab4ce28bdadeb93fe6f420d0245 Mon Sep 17 00:00:00 2001 From: Chris Lewis <clewis@palantir.com> Date: Mon, 20 Nov 2017 18:30:28 -0800 Subject: [PATCH 5/8] Fix test --- packages/datetime/test/dateInputTests.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/datetime/test/dateInputTests.tsx b/packages/datetime/test/dateInputTests.tsx index 365ff564c5..9fa2925195 100644 --- a/packages/datetime/test/dateInputTests.tsx +++ b/packages/datetime/test/dateInputTests.tsx @@ -148,7 +148,7 @@ describe("<DateInput>", () => { it("Pressing Enter saves the inputted date and closes the popover", () => { const wrapper = mount(<DateInput />).setState({ isOpen: true }); const input = wrapper.find("input").first(); - input.simulate("change", { target: { value: "2/15/15" } }); + input.simulate("change", { target: { value: "2015-02-15" } }); input.simulate("keydown", { which: Keys.ENTER }); assert.isFalse(wrapper.state("isOpen")); assert.equal(wrapper.find(InputGroup).prop("value"), "2015-02-15"); From 62f10ebf815d55c830c86b27df27e3d08b0022cd Mon Sep 17 00:00:00 2001 From: Chris Lewis <clewis@palantir.com> Date: Tue, 28 Nov 2017 10:57:00 -0800 Subject: [PATCH 6/8] Fix blur interactions --- packages/datetime/src/dateInput.tsx | 17 +++-- packages/datetime/src/dateRangeInput.tsx | 66 ++++++++++--------- packages/datetime/test/dateInputTests.tsx | 14 ++-- .../datetime/test/dateRangeInputTests.tsx | 37 ++++++++--- 4 files changed, 87 insertions(+), 47 deletions(-) diff --git a/packages/datetime/src/dateInput.tsx b/packages/datetime/src/dateInput.tsx index e2e24f61ab..d9389c626c 100644 --- a/packages/datetime/src/dateInput.tsx +++ b/packages/datetime/src/dateInput.tsx @@ -308,7 +308,7 @@ export class DateInput extends AbstractComponent<IDateInputProps, IDateInputStat this.setState({ isOpen: false }); }; - private handleDateChange = (date: Date, hasUserManuallySelectedDate: boolean) => { + private handleDateChange = (date: Date, hasUserManuallySelectedDate: boolean, didSubmitWithEnter = false) => { const prevMomentDate = this.state.value; const momentDate = fromDateToMoment(date); @@ -321,10 +321,17 @@ export class DateInput extends AbstractComponent<IDateInputProps, IDateInputStat this.hasTimeChanged(prevMomentDate, momentDate) || !this.props.closeOnSelection; + // if selecting a date via click or Tab, the input will already be + // blurred by now, so sync isInputFocused to false. if selecting via + // Enter, setting isInputFocused to false won't do anything by itself, + // plus we want the field to retain focus anyway. + // (note: spelling out the ternary explicitly reads more clearly.) + const isInputFocused = didSubmitWithEnter ? true : false; + if (this.props.value === undefined) { - this.setState({ isInputFocused: false, isOpen, value: momentDate }); + this.setState({ isInputFocused, isOpen, value: momentDate }); } else { - this.setState({ isInputFocused: false, isOpen }); + this.setState({ isInputFocused, isOpen }); } Utils.safeInvoke(this.props.onChange, date === null ? null : fromMomentToDate(momentDate)); }; @@ -429,9 +436,9 @@ export class DateInput extends AbstractComponent<IDateInputProps, IDateInputStat if (e.which === Keys.ENTER) { const nextValue = this.createMoment(this.state.valueString); const nextDate = fromMomentToDate(nextValue); - this.handleDateChange(nextDate, true); - this.inputRef.blur(); + this.handleDateChange(nextDate, true, true); } + this.safeInvokeInputProp("onKeyDown", e); }; private setInputRef = (el: HTMLElement) => { diff --git a/packages/datetime/src/dateRangeInput.tsx b/packages/datetime/src/dateRangeInput.tsx index e6263a5e5e..f711f2fa0e 100644 --- a/packages/datetime/src/dateRangeInput.tsx +++ b/packages/datetime/src/dateRangeInput.tsx @@ -392,7 +392,7 @@ export class DateRangeInput extends AbstractComponent<IDateRangeInputProps, IDat // Callbacks - DateRangePicker // =========================== - private handleDateRangePickerChange = (selectedRange: DateRange) => { + private handleDateRangePickerChange = (selectedRange: DateRange, didSubmitWithEnter = false) => { // ignore mouse events in the date-range picker if the popover is animating closed. if (!this.state.isOpen) { return; @@ -424,7 +424,10 @@ export class DateRangeInput extends AbstractComponent<IDateRangeInputProps, IDat } else if (this.props.closeOnSelection) { isOpen = false; isStartInputFocused = false; - isEndInputFocused = false; + // if we submit via click or Tab, the focus will have moved already. + // it we submit with Enter, the focus won't have moved, and setting + // the flag to false won't have an effect anyway, so leave it true. + isEndInputFocused = didSubmitWithEnter ? true : false; } else if (this.state.lastFocusedField === DateRangeBoundary.START) { // keep the start field focused isStartInputFocused = true; @@ -555,47 +558,50 @@ export class DateRangeInput extends AbstractComponent<IDateRangeInputProps, IDat const wasStartFieldFocused = this.state.lastFocusedField === DateRangeBoundary.START; const wasEndFieldFocused = this.state.lastFocusedField === DateRangeBoundary.END; - let isEndInputFocused: boolean; - let isStartInputFocused: boolean; - // move focus to the other field - if (wasStartFieldFocused && isTabPressed && !isShiftPressed) { - isStartInputFocused = false; - isEndInputFocused = true; - } else if (wasEndFieldFocused && isTabPressed && isShiftPressed) { - isStartInputFocused = true; - isEndInputFocused = false; + if (isTabPressed) { + let isEndInputFocused: boolean; + let isStartInputFocused: boolean; + let isOpen = true; + + if (wasStartFieldFocused && !isShiftPressed) { + isStartInputFocused = false; + isEndInputFocused = true; + + // prevent the default focus-change behavior to avoid race conditions; + // we'll handle the focus change ourselves in componentDidUpdate. + e.preventDefault(); + } else if (wasEndFieldFocused && isShiftPressed) { + isStartInputFocused = true; + isEndInputFocused = false; + e.preventDefault(); + } else { + // don't prevent default here, otherwise Tab won't do anything. + isStartInputFocused = false; + isEndInputFocused = false; + isOpen = false; + } + + this.setState({ + isEndInputFocused, + isOpen, + isStartInputFocused, + wasLastFocusChangeDueToHover: false, + }); } else if (wasStartFieldFocused && isEnterPressed) { const nextStartValue = this.dateStringToMoment(this.state.startInputString); const nextStartDate = fromMomentToDate(nextStartValue); const nextEndDate = isMomentNull(selectedEnd) ? undefined : fromMomentToDate(selectedEnd); - this.handleDateRangePickerChange([nextStartDate, nextEndDate] as DateRange); - isStartInputFocused = false; - isEndInputFocused = true; + this.handleDateRangePickerChange([nextStartDate, nextEndDate] as DateRange, true); } else if (wasEndFieldFocused && isEnterPressed) { const nextStartDate = isMomentNull(selectedStart) ? undefined : fromMomentToDate(selectedStart); const nextEndValue = this.dateStringToMoment(this.state.endInputString); const nextEndDate = fromMomentToDate(nextEndValue); - this.handleDateRangePickerChange([nextStartDate, nextEndDate] as DateRange); - isStartInputFocused = false; - isEndInputFocused = false; - // need to explicitly blur, because only .focus() is triggered in - // componentShouldUpdate. - this.endInputRef.blur(); + this.handleDateRangePickerChange([nextStartDate, nextEndDate] as DateRange, true); } else { // let the default keystroke happen without side effects return; } - - // prevent the default focus-change behavior to avoid race conditions; - // we'll handle the focus change ourselves in componentDidUpdate. - e.preventDefault(); - - this.setState({ - isEndInputFocused, - isStartInputFocused, - wasLastFocusChangeDueToHover: false, - }); }; private handleInputMouseDown = () => { diff --git a/packages/datetime/test/dateInputTests.tsx b/packages/datetime/test/dateInputTests.tsx index e9bb7bf41a..d9ede47f82 100644 --- a/packages/datetime/test/dateInputTests.tsx +++ b/packages/datetime/test/dateInputTests.tsx @@ -167,12 +167,15 @@ describe("<DateInput>", () => { describe("when uncontrolled", () => { it("Pressing Enter saves the inputted date and closes the popover", () => { - const wrapper = mount(<DateInput />).setState({ isOpen: true }); + const onKeyDown = sinon.spy(); + const wrapper = mount(<DateInput inputProps={{ onKeyDown }} />).setState({ isOpen: true }); const input = wrapper.find("input").first(); input.simulate("change", { target: { value: "2015-02-15" } }); input.simulate("keydown", { which: Keys.ENTER }); - assert.isFalse(wrapper.state("isOpen")); - assert.equal(wrapper.find(InputGroup).prop("value"), "2015-02-15"); + assert.isFalse(wrapper.state("isOpen"), "popover closed"); + assert.isTrue(wrapper.state("isInputFocused"), "input still focused"); + assert.strictEqual(wrapper.find(InputGroup).prop("value"), "2015-02-15"); + assert.isTrue(onKeyDown.calledOnce, "onKeyDown called once"); }); it("Clicking a date puts it in the input box and closes the popover", () => { @@ -354,8 +357,9 @@ describe("<DateInput>", () => { const DATE2_DE_STR = "01.02.2015"; it("Pressing Enter saves the inputted date and closes the popover", () => { + const onKeyDown = sinon.spy(); const onChange = sinon.spy(); - const { root } = wrap(<DateInput onChange={onChange} value={DATE} />); + const { root } = wrap(<DateInput inputProps={{ onKeyDown }} onChange={onChange} value={DATE} />); root.setState({ isOpen: true }); const input = root.find("input").first(); @@ -365,6 +369,8 @@ describe("<DateInput>", () => { // onChange is called once on change, once on Enter assert.isTrue(onChange.calledTwice, "onChange called twice"); assertDateEquals(onChange.args[1][0], DATE2_STR); + assert.isTrue(onKeyDown.calledOnce, "onKeyDown called once"); + assert.isTrue(root.state("isInputFocused"), "input still focused"); }); it("Clicking a date invokes onChange callback with that date", () => { diff --git a/packages/datetime/test/dateRangeInputTests.tsx b/packages/datetime/test/dateRangeInputTests.tsx index 5b93dbac33..3e47bed155 100644 --- a/packages/datetime/test/dateRangeInputTests.tsx +++ b/packages/datetime/test/dateRangeInputTests.tsx @@ -257,6 +257,26 @@ describe("<DateRangeInput>", () => { expect(root.find(DateRangePicker).prop("shortcuts")).to.be.false; }); + it("pressing Shift+Tab in the start field blurs the start field and closes the popover", () => { + const startInputProps = { onKeyDown: sinon.spy() }; + const { root } = wrap(<DateRangeInput {...{ startInputProps }} />); + const startInput = getStartInput(root); + startInput.simulate("keydown", { which: Keys.TAB, shiftKey: true }); + expect(root.state("isStartInputFocused"), "start input blurred").to.be.false; + expect(startInputProps.onKeyDown.calledOnce, "onKeyDown called once").to.be.true; + expect(root.state("isOpen"), "popover closed").to.be.false; + }); + + it("pressing Tab in the end field blurs the end field and closes the popover", () => { + const endInputProps = { onKeyDown: sinon.spy() }; + const { root } = wrap(<DateRangeInput {...{ endInputProps }} />); + const endInput = getEndInput(root); + endInput.simulate("keydown", { which: Keys.TAB }); + expect(root.state("isEndInputFocused"), "end input blurred").to.be.false; + expect(endInputProps.onKeyDown.calledOnce, "onKeyDown called once").to.be.true; + expect(root.state("isOpen"), "popover closed").to.be.false; + }); + describe("selectAllOnFocus", () => { it("if false (the default), does not select any text on focus", () => { const attachTo = document.createElement("div"); @@ -381,29 +401,31 @@ describe("<DateRangeInput>", () => { }); it("Pressing Enter saves the inputted date and closes the popover", () => { - const { root } = wrap(<DateRangeInput />); + const startInputProps = { onKeyDown: sinon.spy() }; + const endInputProps = { onKeyDown: sinon.spy() }; + const { root } = wrap(<DateRangeInput {...{ startInputProps, endInputProps }} />); root.setState({ isOpen: true }); const startInput = getStartInput(root); startInput.simulate("focus"); startInput.simulate("change", { target: { value: START_STR } }); startInput.simulate("keydown", { which: Keys.ENTER }); - expect(isStartInputFocused(root), "start input blurred next").to.be.false; + expect(startInputProps.onKeyDown.calledOnce, "startInputProps.onKeyDown called once"); + expect(isStartInputFocused(root), "start input still focused").to.be.false; expect(root.state("isOpen"), "popover still open").to.be.true; const endInput = getEndInput(root); - expect(isEndInputFocused(root), "end input focused next").to.be.true; + endInput.simulate("focus"); endInput.simulate("change", { target: { value: END_STR } }); endInput.simulate("keydown", { which: Keys.ENTER }); + expect(endInputProps.onKeyDown.calledOnce, "endInputProps.onKeyDown called once"); + expect(isEndInputFocused(root), "end input still focused").to.be.true; expect(startInput.prop("value")).to.equal(START_STR); expect(endInput.prop("value")).to.equal(END_STR); expect(root.state("isOpen"), "popover closed at end").to.be.false; - expect(isStartInputFocused(root), "start input blurred at end").to.be.false; - // TODO (clewis): fix failing statement (works in practice) - // expect(isEndInputFocused(root), "end input blurred at end").to.be.false; }); it("Clicking a date invokes onChange with the new date range and updates the input fields", () => { @@ -2095,8 +2117,7 @@ describe("<DateRangeInput>", () => { endInput.simulate("keydown", { which: Keys.ENTER }); expect(isStartInputFocused(root), "start input blurred at end").to.be.false; - // TODO (clewis): fix failing statement (works in practice) - // expect(isEndInputFocused(root), "end input blurred at end").to.be.false; + expect(isEndInputFocused(root), "end input still focused at end").to.be.true; // onChange is called once on change, once on Enter expect(onChange.callCount, "onChange called four times").to.equal(4); From 87802ee718d7f1c9eb77b6f0514c31ab50b18723 Mon Sep 17 00:00:00 2001 From: Chris Lewis <clewis@palantir.com> Date: Fri, 1 Dec 2017 00:30:36 -0800 Subject: [PATCH 7/8] Close the popover on Shift + TAB --- packages/datetime/src/dateInput.tsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/datetime/src/dateInput.tsx b/packages/datetime/src/dateInput.tsx index d9389c626c..dc7396e263 100644 --- a/packages/datetime/src/dateInput.tsx +++ b/packages/datetime/src/dateInput.tsx @@ -437,6 +437,11 @@ export class DateInput extends AbstractComponent<IDateInputProps, IDateInputStat const nextValue = this.createMoment(this.state.valueString); const nextDate = fromMomentToDate(nextValue); this.handleDateChange(nextDate, true, true); + } else if (e.which === Keys.TAB && e.shiftKey) { + // close the popover if focus will move to the previous element on + // the page. tabbing forward should *not* close the popover, because + // focus will be moving into the popover itself. + this.setState({ isOpen: false }); } this.safeInvokeInputProp("onKeyDown", e); }; From 222f9c8b504747c78e0fc2dd0f03923542bbff7e Mon Sep 17 00:00:00 2001 From: Chris Lewis <clewis@palantir.com> Date: Fri, 8 Dec 2017 08:53:24 -0800 Subject: [PATCH 8/8] Format the date string on Enter in DateInput --- packages/datetime/src/dateInput.tsx | 2 +- packages/datetime/test/dateInputTests.tsx | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/datetime/src/dateInput.tsx b/packages/datetime/src/dateInput.tsx index dc7396e263..80256a0a06 100644 --- a/packages/datetime/src/dateInput.tsx +++ b/packages/datetime/src/dateInput.tsx @@ -329,7 +329,7 @@ export class DateInput extends AbstractComponent<IDateInputProps, IDateInputStat const isInputFocused = didSubmitWithEnter ? true : false; if (this.props.value === undefined) { - this.setState({ isInputFocused, isOpen, value: momentDate }); + this.setState({ isInputFocused, isOpen, value: momentDate, valueString: this.getDateString(momentDate) }); } else { this.setState({ isInputFocused, isOpen }); } diff --git a/packages/datetime/test/dateInputTests.tsx b/packages/datetime/test/dateInputTests.tsx index d9ede47f82..dcf386fc77 100644 --- a/packages/datetime/test/dateInputTests.tsx +++ b/packages/datetime/test/dateInputTests.tsx @@ -167,14 +167,16 @@ describe("<DateInput>", () => { describe("when uncontrolled", () => { it("Pressing Enter saves the inputted date and closes the popover", () => { + const IMPROPERLY_FORMATTED_DATE_STRING = "2015-2-15"; + const PROPERLY_FORMATTED_DATE_STRING = "2015-02-15"; const onKeyDown = sinon.spy(); const wrapper = mount(<DateInput inputProps={{ onKeyDown }} />).setState({ isOpen: true }); const input = wrapper.find("input").first(); - input.simulate("change", { target: { value: "2015-02-15" } }); + input.simulate("change", { target: { value: IMPROPERLY_FORMATTED_DATE_STRING } }); input.simulate("keydown", { which: Keys.ENTER }); assert.isFalse(wrapper.state("isOpen"), "popover closed"); assert.isTrue(wrapper.state("isInputFocused"), "input still focused"); - assert.strictEqual(wrapper.find(InputGroup).prop("value"), "2015-02-15"); + assert.strictEqual(wrapper.find(InputGroup).prop("value"), PROPERLY_FORMATTED_DATE_STRING); assert.isTrue(onKeyDown.calledOnce, "onKeyDown called once"); });