Skip to content

Commit

Permalink
fix(progress-bar): incorrectly handling current path when using hash …
Browse files Browse the repository at this point in the history
…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 angular#12710.
  • Loading branch information
crisbeto committed Aug 17, 2018
1 parent b2247f8 commit 62b7288
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 39 deletions.
80 changes: 46 additions & 34 deletions src/lib/progress-bar/progress-bar.spec.ts
Original file line number Diff line number Diff line change
@@ -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<T>(componentType: Type<T>): ComponentFixture<T> {
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<T>(componentType);
}

describe('basic progress-bar', () => {
let fixture: ComponentFixture<BasicProgressBar>;

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);
Expand All @@ -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);
Expand All @@ -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<BufferProgressBar>;

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');
});
});
Expand Down
37 changes: 32 additions & 5 deletions src/lib/progress-bar/progress-bar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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<MatProgressBarLocation>(
'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;

Expand Down Expand Up @@ -61,13 +85,16 @@ 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 `<base>` 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`, because we can't tell the difference between whether
// the consumer is using the hash location strategy or not.
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. */
Expand Down

0 comments on commit 62b7288

Please sign in to comment.