Skip to content
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

feat(material): Upgrade @angular/material to v 2.0.0-beta.12 #482

Merged
merged 2 commits into from
Nov 8, 2017

Conversation

izifortune
Copy link
Contributor

Angular material removed the global MaterialModule all the
components needs to be imported individually.

Fixes #448

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.06%) to 91.413% when pulling b0d6297 on izifortune:feature/upgrade-angular-material into 100a8ef on ngrx:master.

<mat-input-container>
<input matInput placeholder="Search for a book" [value]="query" (keyup)="search.emit($event.target.value)">
</mat-input-container>
<mat-progress-spinner mode="indeterminate" [class.show]="searching"></mat-progress-spinner>
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of using mat-progress-spinner you can use the mat-spinner. The <mat-spinner> component is an alias for <mat-progress-spinner mode="indeterminate">. You should also add the diameter and strokeWidth inputs. This will allow us to not have to use !important when setting the width and height. It should look something like:

<mat-spinner [class.show]="searching" [diameter]="30" [strokeWidth]="3"></mat-spinner>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

great i was struggling to set the correct values for this :)

height: 30px;
.mat-progress-spinner {
width: 30px !important;
height: 30px !important;
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the width and height

@@ -10,3 +10,8 @@ html {
-ms-overflow-style: none;
overflow: auto;
}

.mat-progress-spinner svg {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is no longer needed after fixing the book-search component.

package.json Outdated
@@ -63,7 +63,7 @@
"@angular/core": "^4.2.0",
"@angular/forms": "^4.2.0",
"@angular/http": "^4.2.0",
"@angular/material": "^2.0.0-beta.7",
"@angular/material": "^2.0.0-beta.12",
Copy link
Contributor

Choose a reason for hiding this comment

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

We should lock this to version 2.0.0-beta.12 so it won't break if a new release is published with breaking changes.

package.json Outdated
@@ -123,7 +123,7 @@
"zone.js": "^0.8.12"
},
"dependencies": {
"@angular/cdk": "^2.0.0-beta.8",
"@angular/cdk": "^2.0.0-beta.12",
Copy link
Contributor

Choose a reason for hiding this comment

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

We should lock this to version 2.0.0-beta.12 so it won't break if a new release is published with breaking changes. This can probably also be moved to the devDependencies.

setup-jest.ts Outdated
@@ -1,6 +1,6 @@
import 'jest-preset-angular';
import { MdCommonModule } from '@angular/material';

// import { MdCommonModule } from '@angular/material';
Copy link
Contributor

Choose a reason for hiding this comment

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

These should be deleted instead of commented out.

@izifortune izifortune force-pushed the feature/upgrade-angular-material branch from b0d6297 to b21c54b Compare October 22, 2017 17:23
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.1%) to 91.413% when pulling b21c54b on izifortune:feature/upgrade-angular-material into 998b9d2 on ngrx:master.

@izifortune izifortune force-pushed the feature/upgrade-angular-material branch from b21c54b to d8b7a81 Compare October 22, 2017 17:41
Angular material removed the global MaterialModule all the
components needs to be imported individually.

Fixes ngrx#448
@izifortune
Copy link
Contributor Author

All comments addressed

@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.54% when pulling d8b7a81 on izifortune:feature/upgrade-angular-material into 998b9d2 on ngrx:master.

@@ -10,3 +10,8 @@ html {
-ms-overflow-style: none;
overflow: auto;
}

.mat-progress-spinner svg {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remove this block? It's no longer needed.

package.json Outdated
@@ -123,7 +123,7 @@
"zone.js": "^0.8.12"
},
"dependencies": {
"@angular/cdk": "^2.0.0-beta.8",
"@angular/cdk": "^2.0.0-beta.12",
Copy link
Contributor

@krjordan krjordan Oct 22, 2017

Choose a reason for hiding this comment

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

We should lock this to version 2.0.0-beta.12 so it won't break if a new release is published with breaking changes. This can probably also be moved to the devDependencies.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.494% when pulling f5971c0 on izifortune:feature/upgrade-angular-material into b5485fe on ngrx:master.

@krjordan krjordan merged commit aedf20e into ngrx:master Nov 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants