From 7b3fca04c644eda97bc88170324e47d1e28fa867 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Tue, 16 Oct 2018 14:33:49 +0200 Subject: [PATCH] fix(progress-bar): not taking current path after first initialization Currently we use a provider in the progress bar in order to get the current path and to account for server-side rendering. Since the provider is an object with a plain `pathname` property, Angular will create it once and cache it, which means that we'll always get the first path that it landed on. The issue can be seen by going to the card demos and then clicking through to the progress bar demo. --- src/lib/progress-bar/progress-bar.spec.ts | 23 +++++++++++++++++++++-- src/lib/progress-bar/progress-bar.ts | 12 ++++++++---- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/lib/progress-bar/progress-bar.spec.ts b/src/lib/progress-bar/progress-bar.spec.ts index c277dbc4eed3..ffedf948a5b7 100644 --- a/src/lib/progress-bar/progress-bar.spec.ts +++ b/src/lib/progress-bar/progress-bar.spec.ts @@ -8,16 +8,18 @@ import {MatProgressBar} from './progress-bar'; describe('MatProgressBar', () => { - let fakePath = '/fake-path'; + let fakePath: string; function createComponent(componentType: Type, imports?: Array>): ComponentFixture { + fakePath = '/fake-path'; + TestBed.configureTestingModule({ imports: imports || [MatProgressBarModule], declarations: [componentType], providers: [{ provide: MAT_PROGRESS_BAR_LOCATION, - useValue: {pathname: fakePath} + useValue: {getPathname: () => fakePath} }] }).compileComponents(); @@ -122,6 +124,23 @@ describe('MatProgressBar', () => { const svg = fixture.debugElement.query(By.css('svg')).nativeElement; expect(svg.getAttribute('focusable')).toBe('false'); }); + + it('should use latest path when prefixing the SVG references', () => { + let fixture = createComponent(BasicProgressBar); + fixture.detectChanges(); + + let rect = fixture.debugElement.query(By.css('rect')).nativeElement; + expect(rect.getAttribute('fill')).toMatch(/^url\(['"]?\/fake-path#.*['"]?\)$/); + + fixture.destroy(); + fakePath = '/another-fake-path'; + + fixture = TestBed.createComponent(BasicProgressBar); + fixture.detectChanges(); + rect = fixture.debugElement.query(By.css('rect')).nativeElement; + + expect(rect.getAttribute('fill')).toMatch(/^url\(['"]?\/another-fake-path#.*['"]?\)$/); + }); }); describe('animation trigger on determinate setting', () => { diff --git a/src/lib/progress-bar/progress-bar.ts b/src/lib/progress-bar/progress-bar.ts index 41755cdbeda8..b4bea8726be0 100644 --- a/src/lib/progress-bar/progress-bar.ts +++ b/src/lib/progress-bar/progress-bar.ts @@ -60,14 +60,18 @@ export const MAT_PROGRESS_BAR_LOCATION = new InjectionToken string; } /** @docs-private */ export function MAT_PROGRESS_BAR_LOCATION_FACTORY(): MatProgressBarLocation { const _document = inject(DOCUMENT); - const pathname = (_document && _document.location && _document.location.pathname) || ''; - return {pathname}; + + return { + // Note that this needs to be a function, because Angular will only instantiate + // this provider once, but we want the current location on each call. + getPathname: () => (_document && _document.location && _document.location.pathname) || '' + }; } @@ -114,7 +118,7 @@ export class MatProgressBar extends _MatProgressBarMixinBase implements CanColor // 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.pathname ? location.pathname.split('#')[0] : ''; + const path = location ? location.getPathname().split('#')[0] : ''; this._rectangleFillValue = `url('${path}#${this.progressbarId}')`; this._isNoopAnimation = _animationMode === 'NoopAnimations'; }