add grid-list.spec.ts test for switching between ratio and fixed height #4048
Conversation
exposes bug where paddingTop/height is not set back to zero when switching tile layout modes.
googlebot
added
the
cla: yes
|
I came-up with a fix (pushed to branch) that passes the test, but someone more familiar with TileStylers code should probably look at this bug. |
unit-test height of MdGridList after switching ratio to fixed.
| this._tileStyler.clearRowStyles(tile); | ||
| }); | ||
| // clear styles set on grid by _layoutTiles() | ||
| this._setListStyle(['paddingBottom', '0px']); |
There was a problem hiding this comment.
This code isn't as clean because it does not defer to the TileStyler implementation to clear the MdGridList styles.
But I wanted to just add a quick fix before adding another method to TileStyler.
I was thinking of changing clearRowStyles to:
clearTileStyles(tiles: MdGridTile[]): [string, string]
which would return the style to clear the grid styling using _setListStyle().
| <md-grid-tile></md-grid-tile> | ||
| </md-grid-list> | ||
| </div>`}) | ||
| class GirdListWithRowHeightRatio { | ||
| heightRatio: string; | ||
| rowHeight: string; |
There was a problem hiding this comment.
Changed from heightRatio to rowHeight because it more clearly indicates direct binding to rowHeight.
I'm reusing the GirdListWithRowHeightRatio test component. Is there a rule that there should only be one test Component per test; should I create a new component for the test I added?
|
Closing this since it's become quite stale. Feel free to open a new PR if this is still relevant. |
|
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. |
This creates a unit tests that demonstrates bug #4047
EDIT: and also tries to fix the bug. However, need code review before merging.
In theory, all permutations of switching between ratio, fixed, and fit should be tested, but that's probably overkill.