Skip to content

fix(grid-list): incorrectly laying out tiles for nested list #13086

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
Oct 2, 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
25 changes: 25 additions & 0 deletions src/lib/grid-list/grid-list-base.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import {InjectionToken} from '@angular/core';

/**
* Injection token used to provide a grid list to a tile and to avoid circular imports.
* @docs-private
*/
export const MAT_GRID_LIST = new InjectionToken<MatGridListBase>('MAT_GRID_LIST');

/**
* Base interface for a `MatGridList`.
* @docs-private
*/
export interface MatGridListBase {
cols: number;
gutterSize: string;
rowHeight: number | string;
}
28 changes: 28 additions & 0 deletions src/lib/grid-list/grid-list.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,18 @@ describe('MatGridList', () => {
expect(getStyle(tiles[2], 'top')).toBe('101px');
});

it('should lay out the tiles correctly for a nested grid list', () => {
const fixture = createComponent(NestedGridList);
fixture.detectChanges();

const innerTiles = fixture.debugElement.queryAll(
By.css('mat-grid-tile mat-grid-list mat-grid-tile'));

expect(getStyle(innerTiles[0], 'top')).toBe('0px');
expect(getStyle(innerTiles[1], 'top')).toBe('101px');
expect(getStyle(innerTiles[2], 'top')).toBe('202px');
});

it('should set the gutter size if passed', () => {
const fixture = createComponent(GridListWithGutterSize);
fixture.detectChanges();
Expand Down Expand Up @@ -635,3 +647,19 @@ class GridListWithRtl { }
`
})
class GridListWithIndirectTileDescendants {}


@Component({template: `
<div style="width:200px">
<mat-grid-list cols="2" rowHeight="100px">
<mat-grid-tile></mat-grid-tile>
<mat-grid-tile>
<mat-grid-list cols="1" rowHeight="100px">
<mat-grid-tile></mat-grid-tile>
<mat-grid-tile></mat-grid-tile>
<mat-grid-tile></mat-grid-tile>
</mat-grid-list>
</mat-grid-tile>
</mat-grid-list>
</div>`})
class NestedGridList { }
15 changes: 11 additions & 4 deletions src/lib/grid-list/grid-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {TileCoordinator} from './tile-coordinator';
import {TileStyler, FitTileStyler, RatioTileStyler, FixedTileStyler} from './tile-styler';
import {Directionality} from '@angular/cdk/bidi';
import {coerceNumberProperty} from '@angular/cdk/coercion';
import {MAT_GRID_LIST, MatGridListBase} from './grid-list-base';


// TODO(kara): Conditional (responsive) column count / row size.
Expand All @@ -40,10 +41,14 @@ const MAT_FIT_MODE = 'fit';
host: {
'class': 'mat-grid-list',
},
providers: [{
provide: MAT_GRID_LIST,
useExisting: MatGridList
Copy link
Member

Choose a reason for hiding this comment

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

I don't quite follow why this is necessary- wouldn't injecting MatGridList do the same thing?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's there to avoid a circular import between the grid list and tile. The grid list has an import of the tile already.

}],
changeDetection: ChangeDetectionStrategy.OnPush,
encapsulation: ViewEncapsulation.None,
})
export class MatGridList implements OnInit, AfterContentChecked {
export class MatGridList implements MatGridListBase, OnInit, AfterContentChecked {
/** Number of columns being rendered. */
private _cols: number;

Expand Down Expand Up @@ -139,16 +144,18 @@ export class MatGridList implements OnInit, AfterContentChecked {
/** Computes and applies the size and position for all children grid tiles. */
private _layoutTiles(): void {
if (!this._tileCoordinator) {
this._tileCoordinator = new TileCoordinator(this._tiles);
this._tileCoordinator = new TileCoordinator();
}


const tracker = this._tileCoordinator;
const tiles = this._tiles.filter(tile => !tile._gridList || tile._gridList === this);
const direction = this._dir ? this._dir.value : 'ltr';

this._tileCoordinator.update(this.cols);
this._tileCoordinator.update(this.cols, tiles);
this._tileStyler.init(this.gutterSize, tracker, this.cols, direction);

this._tiles.forEach((tile, index) => {
tiles.forEach((tile, index) => {
const pos = tracker.positions[index];
this._tileStyler.setStyle(tile, pos.row, pos.col);
});
Expand Down
7 changes: 6 additions & 1 deletion src/lib/grid-list/grid-tile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,17 @@ import {
ViewEncapsulation,
ElementRef,
Input,
Optional,
ContentChildren,
QueryList,
AfterContentInit,
Directive,
ChangeDetectionStrategy,
Inject,
} from '@angular/core';
import {MatLine, MatLineSetter} from '@angular/material/core';
import {coerceNumberProperty} from '@angular/cdk/coercion';
import {MAT_GRID_LIST, MatGridListBase} from './grid-list-base';

@Component({
moduleId: module.id,
Expand All @@ -36,7 +39,9 @@ export class MatGridTile {
_rowspan: number = 1;
_colspan: number = 1;

constructor(private _element: ElementRef<HTMLElement>) {}
constructor(
private _element: ElementRef<HTMLElement>,
@Optional() @Inject(MAT_GRID_LIST) public _gridList?: MatGridListBase) {}

/** Amount of rows that the grid tile takes up. */
@Input()
Expand Down
7 changes: 2 additions & 5 deletions src/lib/grid-list/tile-coordinator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
* found in the LICENSE file at https://angular.io/license
*/

import {QueryList} from '@angular/core';
import {MatGridTile} from './grid-tile';

/**
Expand Down Expand Up @@ -53,19 +52,17 @@ export class TileCoordinator {
/** The computed (row, col) position of each tile (the output). */
positions: TilePosition[];

constructor(private _tiles: QueryList<MatGridTile>) {}

/**
* Updates the tile positions.
* @param numColumns Amount of columns in the grid.
*/
update(numColumns: number) {
update(numColumns: number, tiles: MatGridTile[]) {
this.columnIndex = 0;
this.rowIndex = 0;

this.tracker = new Array(numColumns);
this.tracker.fill(0, 0, this.tracker.length);
this.positions = this._tiles.map(tile => this._trackTile(tile));
this.positions = tiles.map(tile => this._trackTile(tile));
}

/** Calculates the row and col position of a tile. */
Expand Down