Skip to content

Commit

Permalink
fix(Input): Datepicker handles manual input correctly (#1672)
Browse files Browse the repository at this point in the history
* parent fb7c386
author dean <dean.jones@terminus.com> 1567090130 -0400
committer dean <dean.jones@terminus.com> 1567169046 -0400

parent fb7c386
author dean <dean.jones@terminus.com> 1567090130 -0400
committer dean <dean.jones@terminus.com> 1567169006 -0400

fix(Input): handle textual changes to date picker

The datepicker functionality has an issue when a user changes the text
value of the input after a date has been selected. Example of this:

Input value is set to:
08-22-2019

User decides they want change the date to:
08-23-2019

So, they put the cursor after the 22 and change the last 2 to a 3.
During this editing, if we update the value that the datepicker looks
at, it gets confused and the value ends up looking like:

8-232-019

This value cannot be parsed by the datepicker and we end up with a null
Date() value. However, the value inside the input looks correct. The
emitted value from the input, however, ends up being null.

We tried a few workarounds here.

1. Tried setting this.value = new Date(textualValue)
   This was a problem because the moment you removed the 2 in the
   example above, the value ended up looking like: 08-22-019. The
   2 from 2019 moved into the day section of the mask and that value
   is actually a valid date! The date turns out to parse into
   08-22-2019, which is what we already had.  This, in turn, would
   fire off an event on the datepicker which would re-update its
   internal stuff and kick the cursor to the end of the line.
   Making it literally impossible to change the date manually
   unless you deleted every character and reentered the whole
   date by hand.
2. Tried creating a hidden input element that the datepicker was tied to
   and then manually update the shown input based on changes.  This
   caused a lot of issues where we had to manually update the mask on
   every event and make sure we updated both values on the hidden and
   shown input.  This became messy quickly.
3. The solution that's being committed here.  We don't fire off a
   selected event on input changes. We instead just update the masked
   value and store the masked value in a separate store.  Then, on blur
   events, the change event will look to see if the date is null (due to
   the description of the original issue above) and if the textual value
   is not null. If so, the textual value wins and we create a new date
   object based on that info.  Otherwise, the datepicker value wins.
   This prevents the input text changing while the user is typing and
   throws out an event on blur with the updated value.

* fix(Input): handle textual changes to date picker

The datepicker functionality has an issue when a user changes the text
value of the input after a date has been selected. Example of this:

Input value is set to:
08-22-2019

User decides they want change the date to:
08-23-2019

So, they put the cursor after the 22 and change the last 2 to a 3.
During this editing, if we update the value that the datepicker looks
at, it gets confused and the value ends up looking like:

8-232-019

This value cannot be parsed by the datepicker and we end up with a null
Date() value. However, the value inside the input looks correct. The
emitted value from the input, however, ends up being null.

We tried a few workarounds here.

Tried setting this.value = new Date(textualValue)
This was a problem because the moment you removed the 2 in the
example above, the value ended up looking like: 08-22-019. The
2 from 2019 moved into the day section of the mask and that value
is actually a valid date! The date turns out to parse into
08-22-2019, which is what we already had. This, in turn, would
fire off an event on the datepicker which would re-update its
internal stuff and kick the cursor to the end of the line.
Making it literally impossible to change the date manually
unless you deleted every character and reentered the whole
date by hand.
Tried creating a hidden input element that the datepicker was tied to
and then manually update the shown input based on changes. This
caused a lot of issues where we had to manually update the mask on
every event and make sure we updated both values on the hidden and
shown input. This became messy quickly.
The solution that's being committed here. We don't fire off a
selected event on input changes. We instead just update the masked
value and store the masked value in a separate store. Then, on blur
events, the change event will look to see if the date is null (due to
the description of the original issue above) and if the textual value
is not null. If so, the textual value wins and we create a new date
object based on that info. Otherwise, the datepicker value wins.
This prevents the input text changing while the user is typing and
throws out an event on blur with the updated value.
  • Loading branch information
deanterm authored and benjamincharity committed Aug 30, 2019
1 parent d2828e2 commit d61ae61
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 5 deletions.
5 changes: 5 additions & 0 deletions terminus-ui/date-range/src/date-range.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,9 @@ describe(`TsDateRangeComponent`, function() {
expect(endInputInstance.formControl.value).toBeNull();

typeInElement('3-4-2019', startInputInstance.inputElement.nativeElement);
startInputInstance.inputElement.nativeElement.blur();
typeInElement('3-8-2019', endInputInstance.inputElement.nativeElement);
endInputInstance.inputElement.nativeElement.blur();
fixture.detectChanges();

expect(fixture.componentInstance.dateRangeComponent['internalStartControl'].value).toEqual(new Date('3-4-2019'));
Expand All @@ -164,6 +166,7 @@ describe(`TsDateRangeComponent`, function() {
const [startInputInstance, endInputInstance] = getRangeInputInstances(fixture);

typeInElement('3-4-2019', startInputInstance.inputElement.nativeElement);
startInputInstance.inputElement.nativeElement.blur();
fixture.detectChanges();
expect(fixture.componentInstance.startSelected).toHaveBeenCalledWith(new Date('3-4-2019'));
expect(fixture.componentInstance.dateRangeChange).toHaveBeenCalledWith({
Expand All @@ -172,6 +175,7 @@ describe(`TsDateRangeComponent`, function() {
});

typeInElement('3-8-2019', endInputInstance.inputElement.nativeElement);
endInputInstance.inputElement.nativeElement.blur();
fixture.detectChanges();
expect(fixture.componentInstance.endSelected).toHaveBeenCalledWith(new Date('3-8-2019'));
expect(fixture.componentInstance.dateRangeChange).toHaveBeenCalledWith({
Expand Down Expand Up @@ -229,6 +233,7 @@ describe(`TsDateRangeComponent`, function() {
fixture.detectChanges();
const startInputInstance = getRangeInputInstances(fixture)[0];
typeInElement('3-4-2019', startInputInstance.inputElement.nativeElement);
startInputInstance.inputElement.nativeElement.blur();
fixture.detectChanges();

expect(fixture.componentInstance.startSelected).toHaveBeenCalled();
Expand Down
2 changes: 1 addition & 1 deletion terminus-ui/input/src/input.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@
[matDatepicker]="picker"
[min]="minDate"
[max]="maxDate"
(dateChange)="selected.emit($event.value)"
(dateChange)="onDateChanged($event.value)"
#inputElement
>
</ng-container>
Expand Down
24 changes: 23 additions & 1 deletion terminus-ui/input/src/input.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -839,6 +839,29 @@ describe(`TsInputComponent`, function() {
});


describe(`onDateChanged`, () => {
test(`should trigger selected.emit with the date passed in`, () => {
const date = new Date();
const fixture = createComponent(TestComponents.DateFilter);
const component = fixture.componentInstance.inputComponent;
(component as any).textualDateValue = '01-02-2019';
component.selected.emit = jest.fn();
component.onDateChanged(date);
expect(component.selected.emit).toHaveBeenCalledWith(date);
});

test(`should trigger selected.emit with the text value date`, () => {
const date = new Date('01-02-2019');
const fixture = createComponent(TestComponents.DateFilter);
const component = fixture.componentInstance.inputComponent;
(component as any).textualDateValue = '01-02-2019';
component.selected.emit = jest.fn();
component.onDateChanged(undefined as any);
expect(component.selected.emit).toHaveBeenCalledWith(date);
});
});


describe(`reset()`, () => {

test(`should reset the input`, () => {
Expand Down Expand Up @@ -896,7 +919,6 @@ describe(`TsInputComponent`, function() {
typeInElement('01-01-2018', inputElement);

expect(component._valueChange.emit).toHaveBeenCalledWith(new Date('01-01-2018'));
expect(component.selected.emit).toHaveBeenCalledWith(new Date('01-01-2018'));
});

test('should return if target is not set', () => {
Expand Down
31 changes: 28 additions & 3 deletions terminus-ui/input/src/input.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,11 @@ export class TsInputComponent implements
*/
private textMaskInputElement!: TextMaskInputElement;

/*
* The textual value of the date entered into the input.
*/
private textualDateValue = '';

/**
* Define the default component ID
*/
Expand Down Expand Up @@ -394,7 +399,7 @@ export class TsInputComponent implements
return true;
}

return !!(input && !input.value && !this.isBadInput() && !this.autofilled);
return !!input && !input.value && !this.isBadInput() && !this.autofilled;
}

/**
Expand Down Expand Up @@ -844,7 +849,6 @@ export class TsInputComponent implements
@Output()
public readonly selected: EventEmitter<Date> = new EventEmitter();


public constructor(
private elementRef: ElementRef,
private renderer: Renderer2,
Expand Down Expand Up @@ -1111,6 +1115,7 @@ export class TsInputComponent implements
} else {
// Trigger the onTouchedCallback for blur events
this.onTouchedCallback();
this.onDateChanged(this.value);
this.inputBlur.emit(this.value);
}
}
Expand Down Expand Up @@ -1164,6 +1169,7 @@ export class TsInputComponent implements
}

let value = target.value;

// We need to trim the last character due to a bug in the text-mask library
const trimmedValue = this.trimLastCharacter(value);
this.inputElement.nativeElement.value = trimmedValue;
Expand All @@ -1188,13 +1194,32 @@ export class TsInputComponent implements

// istanbul ignore else
if (this.datepicker) {
this.selected.emit(this.value);
// set the new date string the user input
this.textualDateValue = value;
this._valueChange.emit(new Date(value));
}
}
// tslint:enable: no-unused-variable


/**
* Notify consumer of date changed from the picker being used.
*
* @param date - The date that has been set.
*/
public onDateChanged(date: Date): void {
// if the user input changed since the last selection, we want to use that date.
// we also need to reset the textual date value once we use it because we don't
// want to keep it fresh in case another date is selected but no user input was given.
if (!date && this.textualDateValue) {
date = new Date(this.textualDateValue);
this.textualDateValue = '';
}

this.selected.emit(date);
}


/**
* Remove the mask if needed
*
Expand Down

0 comments on commit d61ae61

Please sign in to comment.