Skip to content

Commit

Permalink
fix(form-field): incorrect assumptions about page direction
Browse files Browse the repository at this point in the history
These changes undo the changes from #15415 because they were making incorrect assumptions about the page starting off as LTR, and they were calling `updateOutlineGap` with an outdated value for the direction. This is causing issues like #17390.

I'm not totally sure what was being fixed by these changes, because the test that was added didn't fail even if I reverted all of the changes. From what I can tell the problem #15415 was trying to solve was that the outline gap might be updated too early on a direction change, before the browser has had the chance to recalculate the layout. These changes switch to recalculating inside a `requestAnimationFrame` after direction changes, in order to give the browser enough time to recalculate the layout.

Fixes #17390.
  • Loading branch information
crisbeto committed Oct 14, 2019
1 parent 14c4dba commit f759ca1
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 17 deletions.
22 changes: 9 additions & 13 deletions src/material/form-field/form-field.ts
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 {Direction, Directionality} from '@angular/cdk/bidi';
import {Directionality} from '@angular/cdk/bidi';
import {coerceBooleanProperty} from '@angular/cdk/coercion';
import {
AfterContentChecked,
Expand Down Expand Up @@ -232,15 +232,6 @@ export class MatFormField extends _MatFormFieldMixinBase
/** Whether the Angular animations are enabled. */
_animationsEnabled: boolean;

/* Holds the previous direction emitted by directionality service change emitter.
This is used in updateOutlineGap() method to update the width and position of the gap in the
outline. Only relevant for the outline appearance. The direction is getting updated in the
UI after directionality service change emission. So the outlines gaps are getting
updated in updateOutlineGap() method before connectionContainer child direction change
in UI. We may get wrong calculations. So we are storing the previous direction to get the
correct outline calculations*/
private _previousDirection: Direction = 'ltr';

/**
* @deprecated
* @breaking-change 8.0.0
Expand Down Expand Up @@ -356,8 +347,13 @@ export class MatFormField extends _MatFormFieldMixinBase

if (this._dir) {
this._dir.change.pipe(takeUntil(this._destroyed)).subscribe(() => {
this.updateOutlineGap();
this._previousDirection = this._dir.value;
if (typeof requestAnimationFrame === 'function') {
this._ngZone.runOutsideAngular(() => {
requestAnimationFrame(() => this.updateOutlineGap());
});
} else {
this.updateOutlineGap();
}
});
}
}
Expand Down Expand Up @@ -580,7 +576,7 @@ export class MatFormField extends _MatFormFieldMixinBase

/** Gets the start end of the rect considering the current directionality. */
private _getStartEnd(rect: ClientRect): number {
return this._previousDirection === 'rtl' ? rect.right : rect.left;
return (this._dir && this._dir.value === 'rtl') ? rect.right : rect.left;
}

/** Checks whether the form field is attached to the DOM. */
Expand Down
8 changes: 4 additions & 4 deletions src/material/input/input.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
ViewEncapsulation,
ElementRef,
} from '@angular/core';
import {ComponentFixture, fakeAsync, flush, TestBed} from '@angular/core/testing';
import {ComponentFixture, fakeAsync, flush, TestBed, tick} from '@angular/core/testing';
import {
FormControl,
FormGroup,
Expand Down Expand Up @@ -1444,7 +1444,7 @@ describe('MatInput with appearance', () => {
fakeDirectionality.value = 'rtl';
fakeDirectionality.change.next('rtl');
outlineFixture.detectChanges();
flush();
tick(16.6); // Angular replaces requestAnimationFrame calls with 16.6ms timeouts in tests.
outlineFixture.detectChanges();

expect(outlineFixture.componentInstance.formField.updateOutlineGap).toHaveBeenCalled();
Expand Down Expand Up @@ -1479,7 +1479,7 @@ describe('MatInput with appearance', () => {
fakeDirectionality.value = 'rtl';
fakeDirectionality.change.next('rtl');
outlineFixture.detectChanges();
flush();
tick(16.6); // Angular replaces requestAnimationFrame calls with 16.6ms timeouts in tests.
outlineFixture.detectChanges();

let wrapperElement = outlineFixture.nativeElement;
Expand All @@ -1493,7 +1493,7 @@ describe('MatInput with appearance', () => {
fakeDirectionality.value = 'ltr';
fakeDirectionality.change.next('ltr');
outlineFixture.detectChanges();
flush();
tick(16.6);
outlineFixture.detectChanges();

wrapperElement = outlineFixture.nativeElement;
Expand Down

0 comments on commit f759ca1

Please sign in to comment.