From a3f52061bb7fb5951cfdf2a4c99f81942d304395 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Tue, 21 Aug 2018 18:37:44 +0200 Subject: [PATCH] fix(progress-bar): incorrectly handling current path when using hash location strategy Fixes the progress bar prefixing the references incorrectly when the consumer is using the router's hash location strategy. The issue comes from the fact that the router normalizes the `location.pathname` between the regular strategy and the hash one, whereas we need to use the exact same path as the `window.location`. Fixes #12710. --- src/lib/progress-bar/progress-bar.spec.ts | 80 +++++++++++++---------- src/lib/progress-bar/progress-bar.ts | 38 +++++++++-- 2 files changed, 79 insertions(+), 39 deletions(-) diff --git a/src/lib/progress-bar/progress-bar.spec.ts b/src/lib/progress-bar/progress-bar.spec.ts index 883cbe13dc3e..9b75b4f65900 100644 --- a/src/lib/progress-bar/progress-bar.spec.ts +++ b/src/lib/progress-bar/progress-bar.spec.ts @@ -1,52 +1,49 @@ -import {TestBed, async, ComponentFixture} from '@angular/core/testing'; -import {Component} from '@angular/core'; +import {TestBed, ComponentFixture} from '@angular/core/testing'; +import {Component, Type} from '@angular/core'; import {By} from '@angular/platform-browser'; -import {Location} from '@angular/common'; -import {MatProgressBarModule} from './index'; +import {MatProgressBarModule, MAT_PROGRESS_BAR_LOCATION} from './index'; describe('MatProgressBar', () => { let fakePath = '/fake-path'; - beforeEach(async(() => { + function createComponent(componentType: Type): ComponentFixture { TestBed.configureTestingModule({ imports: [MatProgressBarModule], - declarations: [ - BasicProgressBar, - BufferProgressBar, - ], + declarations: [componentType], providers: [{ - provide: Location, - useValue: {path: () => fakePath} + provide: MAT_PROGRESS_BAR_LOCATION, + useValue: {pathname: fakePath} }] - }); - - TestBed.compileComponents(); - })); + }).compileComponents(); + return TestBed.createComponent(componentType); + } describe('basic progress-bar', () => { - let fixture: ComponentFixture; - - beforeEach(() => { - fixture = TestBed.createComponent(BasicProgressBar); + it('should apply a mode of "determinate" if no mode is provided.', () => { + const fixture = createComponent(BasicProgressBar); fixture.detectChanges(); - }); - it('should apply a mode of "determinate" if no mode is provided.', () => { - let progressElement = fixture.debugElement.query(By.css('mat-progress-bar')); + const progressElement = fixture.debugElement.query(By.css('mat-progress-bar')); expect(progressElement.componentInstance.mode).toBe('determinate'); }); it('should define default values for value and bufferValue attributes', () => { - let progressElement = fixture.debugElement.query(By.css('mat-progress-bar')); + const fixture = createComponent(BasicProgressBar); + fixture.detectChanges(); + + const progressElement = fixture.debugElement.query(By.css('mat-progress-bar')); expect(progressElement.componentInstance.value).toBe(0); expect(progressElement.componentInstance.bufferValue).toBe(0); }); it('should clamp value and bufferValue between 0 and 100', () => { - let progressElement = fixture.debugElement.query(By.css('mat-progress-bar')); - let progressComponent = progressElement.componentInstance; + const fixture = createComponent(BasicProgressBar); + fixture.detectChanges(); + + const progressElement = fixture.debugElement.query(By.css('mat-progress-bar')); + const progressComponent = progressElement.componentInstance; progressComponent.value = 50; expect(progressComponent.value).toBe(50); @@ -68,8 +65,11 @@ describe('MatProgressBar', () => { }); it('should return the transform attribute for bufferValue and mode', () => { - let progressElement = fixture.debugElement.query(By.css('mat-progress-bar')); - let progressComponent = progressElement.componentInstance; + const fixture = createComponent(BasicProgressBar); + fixture.detectChanges(); + + const progressElement = fixture.debugElement.query(By.css('mat-progress-bar')); + const progressComponent = progressElement.componentInstance; expect(progressComponent._primaryTransform()).toEqual({transform: 'scaleX(0)'}); expect(progressComponent._bufferTransform()).toBe(undefined); @@ -95,26 +95,38 @@ describe('MatProgressBar', () => { }); it('should prefix SVG references with the current path', () => { + const fixture = createComponent(BasicProgressBar); + fixture.detectChanges(); + const rect = fixture.debugElement.query(By.css('rect')).nativeElement; expect(rect.getAttribute('fill')).toMatch(/^url\(['"]?\/fake-path#.*['"]?\)$/); }); + it('should account for location hash when prefixing the SVG references', () => { + fakePath = '/fake-path#anchor'; + + const fixture = createComponent(BasicProgressBar); + fixture.detectChanges(); + + const rect = fixture.debugElement.query(By.css('rect')).nativeElement; + expect(rect.getAttribute('fill')).not.toContain('#anchor#'); + }); + it('should not be able to tab into the underlying SVG element', () => { + const fixture = createComponent(BasicProgressBar); + fixture.detectChanges(); + const svg = fixture.debugElement.query(By.css('svg')).nativeElement; expect(svg.getAttribute('focusable')).toBe('false'); }); }); describe('buffer progress-bar', () => { - let fixture: ComponentFixture; - - beforeEach(() => { - fixture = TestBed.createComponent(BufferProgressBar); + it('should not modify the mode if a valid mode is provided.', () => { + const fixture = createComponent(BufferProgressBar); fixture.detectChanges(); - }); - it('should not modify the mode if a valid mode is provided.', () => { - let progressElement = fixture.debugElement.query(By.css('mat-progress-bar')); + const progressElement = fixture.debugElement.query(By.css('mat-progress-bar')); expect(progressElement.componentInstance.mode).toBe('buffer'); }); }); diff --git a/src/lib/progress-bar/progress-bar.ts b/src/lib/progress-bar/progress-bar.ts index 2e316531bbea..b519ac379aeb 100644 --- a/src/lib/progress-bar/progress-bar.ts +++ b/src/lib/progress-bar/progress-bar.ts @@ -12,9 +12,9 @@ import { Inject, Input, Optional, - ViewEncapsulation + ViewEncapsulation, + InjectionToken } from '@angular/core'; -import {Location} from '@angular/common'; import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations'; import {CanColor, mixinColor} from '@angular/material/core'; @@ -29,6 +29,30 @@ export class MatProgressBarBase { export const _MatProgressBarMixinBase = mixinColor(MatProgressBarBase, 'primary'); +/** + * Injection token used to provide the current location to `MatProgressBar`. + * Used to handle server-side rendering and to stub out during unit tests. + * @docs-private + */ +export const MAT_PROGRESS_BAR_LOCATION = new InjectionToken( + 'mat-progress-bar-location', + {providedIn: 'root', factory: MAT_PROGRESS_BAR_LOCATION_FACTORY} +); + +/** + * Stubbed out location for `MatProgressBar`. + * @docs-private + */ +export interface MatProgressBarLocation { + pathname: string; +} + +/** @docs-private */ +export function MAT_PROGRESS_BAR_LOCATION_FACTORY(): MatProgressBarLocation { + return typeof window !== 'undefined' ? window.location : {pathname: ''}; +} + + /** Counter used to generate unique IDs for progress bars. */ let progressbarId = 0; @@ -61,13 +85,17 @@ export class MatProgressBar extends _MatProgressBarMixinBase implements CanColor * @deprecated `location` parameter to be made required. * @breaking-change 8.0.0 */ - @Optional() location?: Location) { + @Optional() @Inject(MAT_PROGRESS_BAR_LOCATION) location?: MatProgressBarLocation) { 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). - this._rectangleFillValue = `url('${location ? location.path() : ''}#${this.progressbarId}')`; + // because named route URLs can contain parentheses (see #12338). Also we don't use + // `Location` from `@angular/common` 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.pathname.split('#')[0] : ''; + this._rectangleFillValue = `url('${path}#${this.progressbarId}')`; } /** Value of the progress bar. Defaults to zero. Mirrored to aria-valuenow. */