Skip to content

feat(progress-spinner): add injection token for configuring the diameter and stroke globally #11493

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 25, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 46 additions & 1 deletion src/lib/progress-spinner/progress-spinner.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import {TestBed, async} from '@angular/core/testing';
import {Component} from '@angular/core';
import {By} from '@angular/platform-browser';
import {MatProgressSpinnerModule, MatProgressSpinner} from './index';
import {
MatProgressSpinnerModule,
MatProgressSpinner,
MAT_PROGRESS_SPINNER_DEFAULT_OPTIONS,
} from './index';


describe('MatProgressSpinner', () => {
Expand Down Expand Up @@ -232,6 +236,47 @@ describe('MatProgressSpinner', () => {
expect(spinner.nativeElement.style.width).toBe('32px');
expect(spinner.nativeElement.style.height).toBe('32px');
});

it('should be able to set a default diameter', () => {
TestBed
.resetTestingModule()
.configureTestingModule({
imports: [MatProgressSpinnerModule],
declarations: [BasicProgressSpinner],
providers: [{
provide: MAT_PROGRESS_SPINNER_DEFAULT_OPTIONS,
useValue: {diameter: 23}
}]
})
.compileComponents();

const fixture = TestBed.createComponent(BasicProgressSpinner);
fixture.detectChanges();

const progressElement = fixture.debugElement.query(By.css('mat-progress-spinner'));
expect(progressElement.componentInstance.diameter).toBe(23);
});

it('should be able to set a default stroke width', () => {
TestBed
.resetTestingModule()
.configureTestingModule({
imports: [MatProgressSpinnerModule],
declarations: [BasicProgressSpinner],
providers: [{
provide: MAT_PROGRESS_SPINNER_DEFAULT_OPTIONS,
useValue: {strokeWidth: 7}
}]
})
.compileComponents();

const fixture = TestBed.createComponent(BasicProgressSpinner);
fixture.detectChanges();

const progressElement = fixture.debugElement.query(By.css('mat-progress-spinner'));
expect(progressElement.componentInstance.strokeWidth).toBe(7);
});

});


Expand Down
39 changes: 33 additions & 6 deletions src/lib/progress-spinner/progress-spinner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
ElementRef,
ViewEncapsulation,
Optional,
InjectionToken,
} from '@angular/core';
import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations';
import {CanColor, mixinColor} from '@angular/material/core';
Expand Down Expand Up @@ -43,6 +44,26 @@ export class MatProgressSpinnerBase {
}
export const _MatProgressSpinnerMixinBase = mixinColor(MatProgressSpinnerBase, 'primary');

/** Default `mat-progress-spinner` options that can be overridden. */
export interface MatProgressSpinnerDefaultOptions {
/** Diameter of the spinner. */
diameter?: number;
/** Width of the spinner's stroke. */
strokeWidth?: number;
}

/** Injection token to be used to override the default options for `mat-progress-spinner`. */
export const MAT_PROGRESS_SPINNER_DEFAULT_OPTIONS =
new InjectionToken<MatProgressSpinnerDefaultOptions>('mat-progress-spinner-default-options', {
providedIn: 'root',
factory: MAT_PROGRESS_SPINNER_DEFAULT_OPTIONS_FACTORY,
});

/** @docs-private */
export function MAT_PROGRESS_SPINNER_DEFAULT_OPTIONS_FACTORY(): MatProgressSpinnerDefaultOptions {
return {diameter: BASE_SIZE};
}

// .0001 percentage difference is necessary in order to avoid unwanted animation frames
// for example because the animation duration is 4 seconds, .1% accounts to 4ms
// which are enough to see the flicker described in
Expand Down Expand Up @@ -98,7 +119,7 @@ const INDETERMINATE_ANIMATION_TEMPLATE = `
export class MatProgressSpinner extends _MatProgressSpinnerMixinBase implements CanColor {

private _value = 0;
private _strokeWidth: number;
private _strokeWidth = this._defaults ? this._defaults.strokeWidth : undefined;
private _fallbackAnimation = false;

/** Tracks diameters of existing instances to de-dupe generated styles (default d = 100) */
Expand All @@ -120,7 +141,8 @@ export class MatProgressSpinner extends _MatProgressSpinnerMixinBase implements
this._attachStyleNode();
}
}
private _diameter = BASE_SIZE;
private _diameter = this._defaults && this._defaults.diameter ?
this._defaults.diameter : BASE_SIZE;

/** Stroke width of the progress spinner. */
@Input()
Expand All @@ -131,7 +153,6 @@ export class MatProgressSpinner extends _MatProgressSpinnerMixinBase implements
this._strokeWidth = coerceNumberProperty(value);
}


/** Mode of the progress circle */
@Input() mode: ProgressSpinnerMode = 'determinate';

Expand All @@ -147,7 +168,10 @@ export class MatProgressSpinner extends _MatProgressSpinnerMixinBase implements
constructor(public _elementRef: ElementRef,
platform: Platform,
@Optional() @Inject(DOCUMENT) private _document: any,
@Optional() @Inject(ANIMATION_MODULE_TYPE) public _animationMode?: string) {
// @deletion-target 7.0.0 _animationMode and _defaults parameters to be made required.
@Optional() @Inject(ANIMATION_MODULE_TYPE) public _animationMode?: string,
@Inject(MAT_PROGRESS_SPINNER_DEFAULT_OPTIONS)
private _defaults?: MatProgressSpinnerDefaultOptions) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we have our own defaults value in a instance of MatProgressSpinnerDefaultOptions?

Then we can just Object.assign it, rather than holding our defaults inline in location.

In the file above the class:

export interface MatProgressSpinnerDefaultOptions {
  /** Diameter of the spinner. */
  diameter?: number;
  /** Width of the spinner's stroke. */
  strokeWidth?: number;
}

const BASE_OPTIONS  = {
  diameter: 100,
  strokeWidth: 10
};

In the class:

export class MatProgressSpinner 
  ...
  private defaults: MatProgressSpinnerDefaultOptions
  ...
  constructor(
  ...
  @Inject(MAT_PROGRESS_SPINNER_DEFAULT_OPTIONS) defaults: MatProgressSpinnerDefaultOptions) {
    this.defaults = Object.assign(BASE_OPTIONS, defaults);
  ...
  }
...

Copy link
Member Author

@crisbeto crisbeto May 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our defaults are provided as the MAT_PROGRESS_SPINNER_DEFAULT_OPTIONS token. I kept the inline defaults until we can make the defaults a required parameter.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My concern here is that since the interface for MatProgressSpinnerDefaultOptions has all optional properties, that someone who provides a MatProgressSpinnerDefaultOptions, would not provide everything thats needed.


super(_elementRef);
this._fallbackAnimation = platform.EDGE || platform.TRIDENT;
Expand Down Expand Up @@ -249,8 +273,11 @@ export class MatProgressSpinner extends _MatProgressSpinnerMixinBase implements
export class MatSpinner extends MatProgressSpinner {
constructor(elementRef: ElementRef, platform: Platform,
@Optional() @Inject(DOCUMENT) document: any,
@Optional() @Inject(ANIMATION_MODULE_TYPE) _animationMode?: string) {
super(elementRef, platform, document, _animationMode);
// @deletion-targets 7.0.0 animationMode and defaults parameters to be made required.
@Optional() @Inject(ANIMATION_MODULE_TYPE) animationMode?: string,
@Inject(MAT_PROGRESS_SPINNER_DEFAULT_OPTIONS)
defaults?: MatProgressSpinnerDefaultOptions) {
super(elementRef, platform, document, animationMode, defaults);
this.mode = 'indeterminate';
}
}