Skip to content

fix(popover-edit): incorrectly caching values for more than one control #16057

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 24, 2019
Merged

fix(popover-edit): incorrectly caching values for more than one control #16057

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/cdk-experimental/popover-edit/edit-ref.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {Injectable, OnDestroy, Self} from '@angular/core';
import {Injectable, OnDestroy, Self, NgZone} from '@angular/core';
import {ControlContainer} from '@angular/forms';
import {Observable, Subject} from 'rxjs';
import {take} from 'rxjs/operators';
Expand Down Expand Up @@ -41,7 +41,8 @@ export class EditRef<FormValue> implements OnDestroy {

constructor(
@Self() private readonly _form: ControlContainer,
private readonly _editEventDispatcher: EditEventDispatcher) {
private readonly _editEventDispatcher: EditEventDispatcher,
private readonly _ngZone: NgZone) {
this._editEventDispatcher.setActiveEditRef(this);
}

Expand All @@ -51,11 +52,10 @@ export class EditRef<FormValue> implements OnDestroy {
* applicable.
*/
init(previousFormValue: FormValue|undefined): void {
// Wait for either the first value to be set, then override it with
// the previously entered value, if any.
this._form.valueChanges!.pipe(take(1)).subscribe(() => {
// Wait for the zone to stabilize before caching the initial value.
// This ensures that all form controls have been initialized.
this._ngZone.onStable.pipe(take(1)).subscribe(() => {
this.updateRevertValue();

if (previousFormValue) {
this.reset(previousFormValue);
}
Expand Down
79 changes: 49 additions & 30 deletions src/cdk-experimental/popover-edit/popover-edit.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const NAME_EDIT_TEMPLATE = `
[cdkEditControlIgnoreSubmitUnlessValid]="ignoreSubmitUnlessValid"
[cdkEditControlClickOutBehavior]="clickOutBehavior">
<input [ngModel]="element.name" name="name" required>
<input [ngModel]="element.weight" name="weight">
<br>
<button class="submit" type="submit">Confirm</button>
<button class="revert" cdkEditRevert>Revert</button>
Expand Down Expand Up @@ -131,12 +132,16 @@ abstract class BaseTestComponent {
return document.querySelector('.cdk-overlay-connected-position-bounding-box');
}

getInput() {
return document.querySelector('input') as HTMLInputElement|null;
getNameInput() {
return document.querySelector('input[name="name"]') as HTMLInputElement|null;
}

getWeightInput() {
return document.querySelector('input[name="weight"]') as HTMLInputElement|null;
}

lensIsOpen() {
return !!this.getInput();
return !!this.getNameInput();
}

getSubmitButton() {
Expand Down Expand Up @@ -602,7 +607,7 @@ cdkPopoverEditTabOut`, fakeAsync(() => {
it('shows a lens with the value from the table', fakeAsync(() => {
component.openLens();

expect(component.getInput()!.value).toBe('Hydrogen');
expect(component.getNameInput()!.value).toBe('Hydrogen');
clearLeftoverTimers();
}));

Expand Down Expand Up @@ -654,8 +659,8 @@ cdkPopoverEditTabOut`, fakeAsync(() => {
it('updates the form and submits, closing the lens', fakeAsync(() => {
component.openLens();

component.getInput()!.value = 'Hydragon';
component.getInput()!.dispatchEvent(new Event('input'));
component.getNameInput()!.value = 'Hydragon';
component.getNameInput()!.dispatchEvent(new Event('input'));

component.clickSubmitButton();
fixture.detectChanges();
Expand All @@ -667,8 +672,8 @@ cdkPopoverEditTabOut`, fakeAsync(() => {
it('does not close the lens on submit when form is invalid', fakeAsync(() => {
component.openLens();

component.getInput()!.value = '';
component.getInput()!.dispatchEvent(new Event('input'));
component.getNameInput()!.value = '';
component.getNameInput()!.dispatchEvent(new Event('input'));

component.clickSubmitButton();

Expand All @@ -681,8 +686,8 @@ cdkPopoverEditTabOut`, fakeAsync(() => {
component.ignoreSubmitUnlessValid = false;
component.openLens();

component.getInput()!.value = '';
component.getInput()!.dispatchEvent(new Event('input'));
component.getNameInput()!.value = '';
component.getNameInput()!.dispatchEvent(new Event('input'));

component.clickSubmitButton();

Expand All @@ -702,8 +707,8 @@ cdkPopoverEditTabOut`, fakeAsync(() => {
it('closes and reopens a lens with modified value persisted', fakeAsync(() => {
component.openLens();

component.getInput()!.value = 'Hydragon';
component.getInput()!.dispatchEvent(new Event('input'));
component.getNameInput()!.value = 'Hydragon';
component.getNameInput()!.dispatchEvent(new Event('input'));

component.clickCloseButton();
fixture.detectChanges();
Expand All @@ -713,46 +718,60 @@ cdkPopoverEditTabOut`, fakeAsync(() => {

component.openLens();

expect(component.getInput()!.value).toBe('Hydragon');
expect(component.getNameInput()!.value).toBe('Hydragon');
clearLeftoverTimers();
}));

it('resets the lens to original value', fakeAsync(() => {
component.openLens();

component.getInput()!.value = 'Hydragon';
component.getInput()!.dispatchEvent(new Event('input'));
component.getNameInput()!.value = 'Hydragon';
component.getNameInput()!.dispatchEvent(new Event('input'));

component.clickRevertButton();

expect(component.getInput()!.value).toBe('Hydrogen');
expect(component.getNameInput()!.value).toBe('Hydrogen');
clearLeftoverTimers();
}));

it('should not reset the values when clicking revert without making changes',
fakeAsync(() => {
component.openLens();

expect(component.getNameInput()!.value).toBe('Hydrogen');
expect(component.getWeightInput()!.value).toBe('1.007');

component.clickRevertButton();

expect(component.getNameInput()!.value).toBe('Hydrogen');
expect(component.getWeightInput()!.value).toBe('1.007');
clearLeftoverTimers();
}));

it('resets the lens to previously submitted value', fakeAsync(() => {
component.openLens();

component.getInput()!.value = 'Hydragon';
component.getInput()!.dispatchEvent(new Event('input'));
component.getNameInput()!.value = 'Hydragon';
component.getNameInput()!.dispatchEvent(new Event('input'));

component.clickSubmitButton();
fixture.detectChanges();

component.openLens();

component.getInput()!.value = 'Hydragon X';
component.getInput()!.dispatchEvent(new Event('input'));
component.getNameInput()!.value = 'Hydragon X';
component.getNameInput()!.dispatchEvent(new Event('input'));

component.clickRevertButton();

expect(component.getInput()!.value).toBe('Hydragon');
expect(component.getNameInput()!.value).toBe('Hydragon');
clearLeftoverTimers();
}));

it('closes the lens on escape', fakeAsync(() => {
component.openLens();

component.getInput()!.dispatchEvent(
component.getNameInput()!.dispatchEvent(
new KeyboardEvent('keyup', {bubbles: true, key: 'Escape'}));

expect(component.lensIsOpen()).toBe(false);
Expand All @@ -762,7 +781,7 @@ cdkPopoverEditTabOut`, fakeAsync(() => {
it('does not close the lens on click within lens', fakeAsync(() => {
component.openLens();

component.getInput()!.dispatchEvent(new Event('click', {bubbles: true}));
component.getNameInput()!.dispatchEvent(new Event('click', {bubbles: true}));

expect(component.lensIsOpen()).toBe(true);
clearLeftoverTimers();
Expand All @@ -771,8 +790,8 @@ cdkPopoverEditTabOut`, fakeAsync(() => {
it('closes the lens on outside click', fakeAsync(() => {
component.openLens();

component.getInput()!.value = 'Hydragon';
component.getInput()!.dispatchEvent(new Event('input'));
component.getNameInput()!.value = 'Hydragon';
component.getNameInput()!.dispatchEvent(new Event('input'));
document.body.dispatchEvent(new Event('click', {bubbles: true}));
fixture.detectChanges();

Expand All @@ -786,8 +805,8 @@ cdkPopoverEditTabOut`, fakeAsync(() => {
component.clickOutBehavior = 'submit';
component.openLens();

component.getInput()!.value = 'Hydragon';
component.getInput()!.dispatchEvent(new Event('input'));
component.getNameInput()!.value = 'Hydragon';
component.getNameInput()!.dispatchEvent(new Event('input'));
document.body.dispatchEvent(new Event('click', {bubbles: true}));
fixture.detectChanges();

Expand All @@ -801,8 +820,8 @@ cdkPopoverEditTabOut`, fakeAsync(() => {
component.clickOutBehavior = 'noop';
component.openLens();

component.getInput()!.value = 'Hydragon';
component.getInput()!.dispatchEvent(new Event('input'));
component.getNameInput()!.value = 'Hydragon';
component.getNameInput()!.dispatchEvent(new Event('input'));
document.body.dispatchEvent(new Event('click', {bubbles: true}));
fixture.detectChanges();

Expand All @@ -814,7 +833,7 @@ cdkPopoverEditTabOut`, fakeAsync(() => {
it('sets focus on the first input in the lens', fakeAsync(() => {
component.openLens();

expect(document.activeElement).toBe(component.getInput());
expect(document.activeElement).toBe(component.getNameInput());
clearLeftoverTimers();
}));

Expand Down