From 56b3320e7d9bf446cef5fafcc659f11dc33d01b7 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Mon, 3 May 2021 18:10:04 +0200 Subject: [PATCH] fix(material/progress-bar): unable to change value through property setter 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. --- .../mdc-progress-bar/progress-bar.spec.ts | 37 +++++++++++++++++++ .../progress-bar/progress-bar.spec.ts | 35 ++++++++++++++++++ src/material/progress-bar/progress-bar.ts | 28 ++++++++++---- .../material/progress-bar.d.ts | 5 ++- 4 files changed, 96 insertions(+), 9 deletions(-) diff --git a/src/material-experimental/mdc-progress-bar/progress-bar.spec.ts b/src/material-experimental/mdc-progress-bar/progress-bar.spec.ts index 5a49039fde0c..a3f71e8b3b0b 100644 --- a/src/material-experimental/mdc-progress-bar/progress-bar.spec.ts +++ b/src/material-experimental/mdc-progress-bar/progress-bar.spec.ts @@ -152,6 +152,43 @@ describe('MDC-based MatProgressBar', () => { .toBe(false, 'Expect aria-valuenow to be cleared in query mode.'); }); + 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', () => { diff --git a/src/material/progress-bar/progress-bar.spec.ts b/src/material/progress-bar/progress-bar.spec.ts index c792a678607e..90f9465f6b8e 100644 --- a/src/material/progress-bar/progress-bar.spec.ts +++ b/src/material/progress-bar/progress-bar.spec.ts @@ -196,6 +196,41 @@ describe('MatProgressBar', () => { .toBe(false, 'Expect aria-valuenow to be cleared in query mode.'); }); + 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', () => { diff --git a/src/material/progress-bar/progress-bar.ts b/src/material/progress-bar/progress-bar.ts index f7d59f6ae7fa..cf803a42216c 100644 --- a/src/material/progress-bar/progress-bar.ts +++ b/src/material/progress-bar/progress-bar.ts @@ -23,6 +23,7 @@ import { Output, ViewChild, ViewEncapsulation, + ChangeDetectorRef, } from '@angular/core'; import {CanColor, CanColorCtor, mixinColor} from '@angular/material/core'; import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations'; @@ -114,16 +115,21 @@ export class MatProgressBar extends _MatProgressBarMixinBase implements CanColor * @deprecated `location` parameter to be made required. * @breaking-change 8.0.0 */ - @Optional() @Inject(MAT_PROGRESS_BAR_LOCATION) location?: MatProgressBarLocation) { + @Optional() @Inject(MAT_PROGRESS_BAR_LOCATION) location?: MatProgressBarLocation, + + /** + * @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 `` 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'; @@ -137,13 +143,21 @@ export class MatProgressBar extends _MatProgressBarMixinBase implements CanColor get value(): number { return this._value; } set value(v: number) { this._value = clamp(coerceNumberProperty(v) || 0); + + // @breaking-change 11.0.0 Remove null check for _changeDetectorRef. + this._changeDetectorRef?.markForCheck(); } private _value: number = 0; /** Buffer value of the progress bar. Defaults to zero. */ @Input() get bufferValue(): number { return this._bufferValue; } - set bufferValue(v: number) { this._bufferValue = clamp(v || 0); } + 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; @ViewChild('primaryValueBar') _primaryValueBar: ElementRef; diff --git a/tools/public_api_guard/material/progress-bar.d.ts b/tools/public_api_guard/material/progress-bar.d.ts index 34999c4837ad..4e9d6c1fe4af 100644 --- a/tools/public_api_guard/material/progress-bar.d.ts +++ b/tools/public_api_guard/material/progress-bar.d.ts @@ -16,7 +16,8 @@ export declare class MatProgressBar extends _MatProgressBarMixinBase implements get value(): number; set value(v: number); constructor(_elementRef: ElementRef, _ngZone: NgZone, _animationMode?: string | undefined, - location?: MatProgressBarLocation); + location?: MatProgressBarLocation, + _changeDetectorRef?: ChangeDetectorRef | undefined); _bufferTransform(): { transform: string; } | null; @@ -27,7 +28,7 @@ export declare class MatProgressBar extends _MatProgressBarMixinBase implements ngOnDestroy(): void; static ngAcceptInputType_value: NumberInput; static ɵcmp: i0.ɵɵComponentDeclaration; - static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵfac: i0.ɵɵFactoryDeclaration; } export interface MatProgressBarLocation {