-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
test(grid-list): add unit tests for grid-list #555
Conversation
|
||
it('should throw error if cols is not defined', () => { | ||
var template = `<md-grid-list></md-grid-list>`; | ||
return builder.overrideTemplate(TestGridList, template) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than doing return
on each build, you can simply make each test async
and they should auto finish because zones.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
fixture.detectChanges(); | ||
let tile = fixture.debugElement.query(By.directive(MdGridTile)); | ||
|
||
// 149.5 * 2 = 299px + 1px gutter = 300px |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great comments
{text: 'One', cols: 3, rows: 1, color: 'lightblue'}, | ||
{text: 'Two', cols: 1, rows: 2, color: 'lightgreen'}, | ||
{text: 'Three', cols: 1, rows: 1, color: 'lightpink'}, | ||
{text: 'Four', cols: 2, rows: 1, color: '#DDBDF1'}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are there colors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also these colors are boring. You should try some better ones like sienna
, mediumaquamarine
, or lemonchiffon
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ha, it's a relic from the demo page. I'll remove!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lemonchiffon?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM |
0748ef8
to
258513c
Compare
All tests are now passing (added a tick() to the 4 failures to fix Mobile Safari). Merging it in! |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
r: @jelbourn