[DateInput/DateRangeInput] Commit typed value on 'Enter' key press #1825

cmslewis · 2017-11-20T10:06:02Z

Fixes #1798

Checklist

Include tests

Changes proposed in this pull request:

DateInput and DateRangeInput now save the typed value(s) on Enter.
- They also move/transfer the focus state appropriately.

Reviewers should focus on:

The tests don't corroborate my understanding of what should be happening. Maybe I just need to sleep on it. I've commented or omitted some checks in the meantime.
Idea for followup: Revert to the old value on Esc? Thoughts?
Does controlled mode look good?

Screenshot

DateInput

DateRangeInput

blueprint-bot · 2017-11-21T02:40:01Z

Fix test

Preview: documentation

llorca · 2017-11-21T20:57:08Z

does this PR change any interaction?

+1 on reverting to previous value on Esc, if current value hasn't been submitted

cmslewis · 2017-11-21T21:30:56Z

@llorca no, it should be strictly additive.

blueprint-bot · 2017-11-23T01:31:27Z

Merge branch 'master' into cl/1798-dri-enter-key

Preview: documentation

cmslewis · 2017-11-27T17:15:24Z

packages/datetime/src/dateInput.tsx

+            const nextDate = fromMomentToDate(nextValue);
+            this.handleDateChange(nextDate, true);
+            this.inputRef.blur();
+        }


Whoops, need to do this.safeInvokeInputProp("onKeyDown", e);.

@cmslewis is this comment still relevant?

giladgray

looks good but there are some unresolved comments from you @cmslewis

giladgray

actually, not a fan of the blur behavior.

giladgray · 2017-11-28T02:14:16Z

packages/datetime/src/dateInput.tsx

+            const nextValue = this.createMoment(this.state.valueString);
+            const nextDate = fromMomentToDate(nextValue);
+            this.handleDateChange(nextDate, true);
+            this.inputRef.blur();


🤔 this .blur() feels intuitively wrong, as it kills the focus flow: I can't hit enter and then tab to move to the next form input.

can we do everything else but simply not blur? same goes for DRI.

I buy that.

blueprint-bot · 2017-11-28T19:06:32Z

Fix blur interactions

Preview: documentation

Addressed blur issues

cmslewis · 2017-11-29T18:54:48Z

@giladgray is this good to merge?

giladgray · 2017-11-30T01:14:40Z

@cmslewis pressing enter in DateInput does not expand to full date. works great for DRI though.

adidahiya · 2017-12-04T20:48:21Z

pressing enter in DateInput does not expand to full date

seems fine to address in a follow up. current behavior seems fine to me. @erinmarchand @LindsayWard @dalessi can you verify in the "documentation" link above?

giladgray · 2017-12-05T19:07:51Z

wait @adidahiya how is this fine to merge? it literally doesn't work properly in one of the title use cases.

adidahiya · 2017-12-05T19:54:19Z

It seems to address the original intent of the filed issue though. Once you type something in DateInput, you get it committed when you blur out of the input (the popover gets closed, which suggests to the user that something worked -- this was not the case before this PR). Seems reasonable in most cases, and we can make further adjustments if we get more feedback. But I'm also cool with waiting to hear from the people tagged in my last comment ^

llorca · 2017-12-05T22:15:37Z

Check out this GIF:

What's good: in both DI and DRI, the date picker activates with the date typed in the field
What's not good: DRI updates the input value on enter, DI doesn't. Instead, DI updates the input value on blur.

Not a fan of that inconsistency, as @giladgray pointed out.

llorca

DI should update input value on enter, just like DRI. It currently happens on blur

cmslewis · 2017-12-08T16:54:07Z

@llorca @adidahiya @giladgray Sorry for the delay. Should be good to go now.

blueprint-bot · 2017-12-08T17:03:08Z

Format the date string on Enter in DateInput

Preview: documentation | landing | table

adidahiya · 2017-12-08T17:18:23Z

nice, lgtm 👍

Addressed comments

…1825) * DateInput now saves on Enter * DateRangeInput now saves on Enter * Write DateInput tests, fix locale bug * Tests + bug fixes for DateRangeInput * Fix test * Fix blur interactions * Close the popover on Shift + TAB * Format the date string on Enter in DateInput

…press (#1825)" This reverts commit 826fbc7.

cmslewis added 4 commits November 20, 2017 00:24

DateInput now saves on Enter

33f01bd

DateRangeInput now saves on Enter

f2d07f1

Write DateInput tests, fix locale bug

4f9b098

Tests + bug fixes for DateRangeInput

b942bee

cmslewis added the Package: datetime label Nov 20, 2017

cmslewis requested review from llorca, giladgray and gscshoyru November 20, 2017 10:06

Fix test

daf7b2e

Merge branch 'master' into cl/1798-dri-enter-key

66cdf5e

cmslewis commented Nov 27, 2017

View reviewed changes

giladgray reviewed Nov 28, 2017

View reviewed changes

giladgray previously requested changes Nov 28, 2017

View reviewed changes

Fix blur interactions

62f10eb

Close the popover on Shift + TAB

87802ee

adidahiya approved these changes Dec 4, 2017

View reviewed changes

llorca previously requested changes Dec 5, 2017

View reviewed changes

Format the date string on Enter in DateInput

222f9c8

cmslewis merged commit 826fbc7 into master Dec 8, 2017

cmslewis deleted the cl/1798-dri-enter-key branch December 8, 2017 17:56

cmslewis mentioned this pull request Dec 13, 2017

Update gh-pages docs for v1.35.0 #1907

Merged

cmslewis mentioned this pull request Dec 13, 2017

Prepare release datetime@1.25.1 #1910

Closed

cmslewis added a commit that referenced this pull request Dec 13, 2017

Revert "[DateInput/DateRangeInput] Commit typed value on 'Enter' key …

1ad1816

…press (#1825)" This reverts commit 826fbc7.

alshain mentioned this pull request Jan 11, 2018

[DateTime] Decouple "Enter to close" from "closeOnSelection" for DateInput/DateRangeInput #1988

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DateInput/DateRangeInput] Commit typed value on 'Enter' key press #1825

[DateInput/DateRangeInput] Commit typed value on 'Enter' key press #1825

cmslewis commented Nov 20, 2017

blueprint-bot commented Nov 21, 2017

llorca commented Nov 21, 2017

cmslewis commented Nov 21, 2017

blueprint-bot commented Nov 23, 2017

cmslewis Nov 27, 2017

giladgray Nov 30, 2017

giladgray left a comment

giladgray left a comment

giladgray Nov 28, 2017

cmslewis Nov 28, 2017

blueprint-bot commented Nov 28, 2017

cmslewis commented Nov 29, 2017

giladgray commented Nov 30, 2017

adidahiya commented Dec 4, 2017

giladgray commented Dec 5, 2017

adidahiya commented Dec 5, 2017 •

edited

Loading

llorca commented Dec 5, 2017

llorca left a comment

cmslewis commented Dec 8, 2017

blueprint-bot commented Dec 8, 2017

adidahiya commented Dec 8, 2017

[DateInput/DateRangeInput] Commit typed value on 'Enter' key press #1825

[DateInput/DateRangeInput] Commit typed value on 'Enter' key press #1825

Conversation

cmslewis commented Nov 20, 2017

Fixes #1798

Checklist

Changes proposed in this pull request:

Reviewers should focus on:

Screenshot

blueprint-bot commented Nov 21, 2017

Fix test

llorca commented Nov 21, 2017

cmslewis commented Nov 21, 2017

blueprint-bot commented Nov 23, 2017

Merge branch 'master' into cl/1798-dri-enter-key

cmslewis Nov 27, 2017

Choose a reason for hiding this comment

giladgray Nov 30, 2017

Choose a reason for hiding this comment

giladgray left a comment

Choose a reason for hiding this comment

giladgray left a comment

Choose a reason for hiding this comment

giladgray Nov 28, 2017

Choose a reason for hiding this comment

cmslewis Nov 28, 2017

Choose a reason for hiding this comment

blueprint-bot commented Nov 28, 2017

Fix blur interactions

cmslewis commented Nov 29, 2017

giladgray commented Nov 30, 2017

adidahiya commented Dec 4, 2017

giladgray commented Dec 5, 2017

adidahiya commented Dec 5, 2017 • edited Loading

llorca commented Dec 5, 2017

llorca left a comment

Choose a reason for hiding this comment

cmslewis commented Dec 8, 2017

blueprint-bot commented Dec 8, 2017

Format the date string on Enter in DateInput

adidahiya commented Dec 8, 2017

adidahiya commented Dec 5, 2017 •

edited

Loading