Skip to content

Commit

Permalink
fix(material/progress-bar): unable to change value through property s…
Browse files Browse the repository at this point in the history
…etter (#19025)

Fixes the progress bar not updating when its value is changed through the setter. Normally we don't really handle cases like this, but I decided to do it in this one, because:

1. We were already paying the payload price for the setter anyway so adding the `markForCheck` call won't be too expensive.
2. The progress bar is a bit of a special case where it might make sense not to go through the view to change a value. E.g. for something like a file upload where everything is being done in memory.

Fixes #18676.

(cherry picked from commit 652f1e3)
  • Loading branch information
crisbeto authored and andrewseguin committed Jan 15, 2022
1 parent 4182717 commit 966b2c5
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 7 deletions.
39 changes: 39 additions & 0 deletions src/material-experimental/mdc-progress-bar/progress-bar.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,45 @@ describe('MDC-based MatProgressBar', () => {
expect(progressElement.componentInstance.mode).toBe('buffer');
expect(progressElement.componentInstance.color).toBe('warn');
});

it('should update the DOM transform when the value has changed', () => {
const fixture = createComponent(BasicProgressBar);
fixture.detectChanges();

const progressElement = fixture.debugElement.query(By.css('mat-progress-bar'))!;
const progressComponent = progressElement.componentInstance;
const primaryBar = progressElement.nativeElement.querySelector(
'.mdc-linear-progress__primary-bar',
);

expect(primaryBar.style.transform).toBe('scaleX(0)');

progressComponent.value = 40;
fixture.detectChanges();

expect(primaryBar.style.transform).toBe('scaleX(0.4)');
});

it('should update the DOM transform when the bufferValue has changed', () => {
const fixture = createComponent(BasicProgressBar);
fixture.detectChanges();

const progressElement = fixture.debugElement.query(By.css('mat-progress-bar'))!;
const progressComponent = progressElement.componentInstance;
const bufferBar = progressElement.nativeElement.querySelector(
'.mdc-linear-progress__buffer-bar',
);

progressComponent.mode = 'buffer';
fixture.detectChanges();

expect(bufferBar.style.flexBasis).toBe('0%');

progressComponent.bufferValue = 40;
fixture.detectChanges();

expect(bufferBar.style.flexBasis).toBe('40%');
});
});

describe('animation trigger on determinate setting', () => {
Expand Down
35 changes: 35 additions & 0 deletions src/material/progress-bar/progress-bar.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,41 @@ describe('MatProgressBar', () => {
expect(progressElement.componentInstance.mode).toBe('buffer');
expect(progressElement.componentInstance.color).toBe('warn');
});

it('should update the DOM transform when the value has changed', () => {
const fixture = createComponent(BasicProgressBar);
fixture.detectChanges();

const progressElement = fixture.debugElement.query(By.css('mat-progress-bar'))!;
const progressComponent = progressElement.componentInstance;
const primaryBar = progressElement.nativeElement.querySelector('.mat-progress-bar-primary');

expect(primaryBar.style.transform).toBe('scale3d(0, 1, 1)');

progressComponent.value = 40;
fixture.detectChanges();

expect(primaryBar.style.transform).toBe('scale3d(0.4, 1, 1)');
});

it('should update the DOM transform when the bufferValue has changed', () => {
const fixture = createComponent(BasicProgressBar);
fixture.detectChanges();

const progressElement = fixture.debugElement.query(By.css('mat-progress-bar'))!;
const progressComponent = progressElement.componentInstance;
const bufferBar = progressElement.nativeElement.querySelector('.mat-progress-bar-buffer');

progressComponent.mode = 'buffer';
fixture.detectChanges();

expect(bufferBar.style.transform).toBeFalsy();

progressComponent.bufferValue = 40;
fixture.detectChanges();

expect(bufferBar.style.transform).toBe('scale3d(0.4, 1, 1)');
});
});

describe('animation trigger on determinate setting', () => {
Expand Down
21 changes: 16 additions & 5 deletions src/material/progress-bar/progress-bar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
Output,
ViewChild,
ViewEncapsulation,
ChangeDetectorRef,
} from '@angular/core';
import {CanColor, mixinColor, ThemePalette} from '@angular/material/core';
import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations';
Expand Down Expand Up @@ -135,16 +136,20 @@ export class MatProgressBar
@Optional()
@Inject(MAT_PROGRESS_BAR_DEFAULT_OPTIONS)
defaults?: MatProgressBarDefaultOptions,
/**
* @deprecated `_changeDetectorRef` parameter to be made required.
* @breaking-change 11.0.0
*/
private _changeDetectorRef?: ChangeDetectorRef,
) {
super(elementRef);

// We need to prefix the SVG reference with the current path, otherwise they won't work
// in Safari if the page has a `<base>` tag. Note that we need quotes inside the `url()`,

// because named route URLs can contain parentheses (see #12338). Also we don't use since
// we can't tell the difference between whether
// the consumer is using the hash location strategy or not, because `Location` normalizes
// both `/#/foo/bar` and `/foo/bar` to the same thing.
// because named route URLs can contain parentheses (see #12338). Also we don't use `Location`
// since we can't tell the difference between whether the consumer is using the hash location
// strategy or not, because `Location` normalizes both `/#/foo/bar` and `/foo/bar` to
// the same thing.
const path = location ? location.getPathname().split('#')[0] : '';
this._rectangleFillValue = `url('${path}#${this.progressbarId}')`;
this._isNoopAnimation = _animationMode === 'NoopAnimations';
Expand All @@ -168,6 +173,9 @@ export class MatProgressBar
}
set value(v: NumberInput) {
this._value = clamp(coerceNumberProperty(v) || 0);

// @breaking-change 11.0.0 Remove null check for _changeDetectorRef.
this._changeDetectorRef?.markForCheck();
}
private _value: number = 0;

Expand All @@ -178,6 +186,9 @@ export class MatProgressBar
}
set bufferValue(v: number) {
this._bufferValue = clamp(v || 0);

// @breaking-change 11.0.0 Remove null check for _changeDetectorRef.
this._changeDetectorRef?.markForCheck();
}
private _bufferValue: number = 0;

Expand Down
6 changes: 4 additions & 2 deletions tools/public_api_guard/material/progress-bar.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import { _AbstractConstructor } from '@angular/material/core';
import { AfterViewInit } from '@angular/core';
import { CanColor } from '@angular/material/core';
import { ChangeDetectorRef } from '@angular/core';
import { _Constructor } from '@angular/material/core';
import { ElementRef } from '@angular/core';
import { EventEmitter } from '@angular/core';
Expand All @@ -31,7 +32,8 @@ export function MAT_PROGRESS_BAR_LOCATION_FACTORY(): MatProgressBarLocation;
// @public
export class MatProgressBar extends _MatProgressBarBase implements CanColor, AfterViewInit, OnDestroy {
constructor(elementRef: ElementRef, _ngZone: NgZone, _animationMode?: string | undefined,
location?: MatProgressBarLocation, defaults?: MatProgressBarDefaultOptions);
location?: MatProgressBarLocation, defaults?: MatProgressBarDefaultOptions,
_changeDetectorRef?: ChangeDetectorRef | undefined);
readonly animationEnd: EventEmitter<ProgressAnimationEnd>;
// (undocumented)
_animationMode?: string | undefined;
Expand All @@ -58,7 +60,7 @@ export class MatProgressBar extends _MatProgressBarBase implements CanColor, Aft
// (undocumented)
static ɵcmp: i0.ɵɵComponentDeclaration<MatProgressBar, "mat-progress-bar", ["matProgressBar"], { "color": "color"; "value": "value"; "bufferValue": "bufferValue"; "mode": "mode"; }, { "animationEnd": "animationEnd"; }, never, never>;
// (undocumented)
static ɵfac: i0.ɵɵFactoryDeclaration<MatProgressBar, [null, null, { optional: true; }, { optional: true; }, { optional: true; }]>;
static ɵfac: i0.ɵɵFactoryDeclaration<MatProgressBar, [null, null, { optional: true; }, { optional: true; }, { optional: true; }, null]>;
}

// @public
Expand Down

0 comments on commit 966b2c5

Please sign in to comment.