feat(table): support sticky headers, footers, and columns #11483
Conversation
src/cdk/table/cell.ts
Outdated
@@ -59,6 +66,26 @@ export class CdkColumnDef { | |||
} | |||
_name: string; | |||
|
|||
/** Whether this column should be sticky positioned on the left of the row */ | |||
@Input('stickyLeft') |
We typically use either start/end or before/after so that the term applies for RTL as well
src/cdk/table/has-sticky-state.ts
Outdated
* sticky value. | ||
* @docs-private | ||
*/ | ||
export interface HasStickyState { |
What do you think of CanStick
?
SGTM
src/cdk/table/has-sticky-state.ts
Outdated
|
||
checkStickyChanged(): boolean; | ||
|
||
resetStickyChanged(): void; |
Needs descriptions for these other members
src/cdk/table/has-sticky-state.ts
Outdated
* @docs-private | ||
*/ | ||
export interface HasStickyState { | ||
/** State of whether the sticky input has changed since it was last checked. */ |
Omit "State of"
if (!this._columnsDiffer) { | ||
const columns = (changes['columns'] && changes['columns'].currentValue) || []; |
When would columns
not be defined?
If the sticky input changes, then changes: SimpleChanges
will not have the columns property (since it didn't change). Prior to this change, the only time ngOnChange
was called was when the columns changed; now this is not the only case.
* the data source provides a new set of data or when a column definition changes its sticky | ||
* input. May be called manually for cases where the cell content changes outside of these events. | ||
*/ | ||
updateStickyColumnStyles() { |
You have another method with the same name that's internal; they should have different names if they're doing different things
Thanks, didn't catch that. Kept this as update (it removes/adds) and the internal one as add (it only adds)
src/cdk/table/table.ts
Outdated
return acc || d.checkStickyChanged(); | ||
}; | ||
|
||
if (this._headerRowDefs.reduce(stickyCheckReducer, false)) { |
Is this not just a some
?
const hasStickyStateChanged = (d: HasStickyState) => d.checkStickyChanged();
if (this._headerRowDefs.some(hasStickyStateChanged)) {
...
}
No, and I'll add a comment explaining why. The check needs to occur on every definition since it also resets its dirty state
src/cdk/table/has-sticky-state.ts
Outdated
_hasStickyChanged: boolean = false; | ||
|
||
/** Whether the sticky value has changed since this was last called. */ | ||
checkStickyChanged(): boolean { |
hasStickyStateChanged
?
src/cdk/table/table.spec.ts
Outdated
}); | ||
} | ||
|
||
describe('on flex layout', () => { |
on "display: flex" table style"
src/lib/table/cell.ts
Outdated
@@ -71,6 +71,12 @@ export class MatFooterCellDef extends CdkFooterCellDef { | |||
export class MatColumnDef extends CdkColumnDef { | |||
/** Unique name for this column. */ | |||
@Input('matColumnDef') name: string; | |||
|
|||
/** Whether this column should be sticky positioned on the left of the right */ | |||
@Input() stickyLeft: boolean; |
The vast, vast majority of cases are doing to want to stick to the start(left) side. Optimizing for the common case, I think we should just make the start/left position sticky
and the other one stickyEnd
.
We should talk separately about whether this should be something like cdkColumnSticky
SGTM, I agree the vast majority wants to just use sticky
and mean it to be the start.
Based on angular#11483 (comment). Adds a custom tslint rule to enforce that we put getters before setters.
Heads up - looks like RTL support is lacking. Left and right are absolute positions and the logic should reverse the column sticky position directions |
Based on #11483 (comment). Adds a custom tslint rule to enforce that we put getters before setters.
Hi @andrewseguin! This PR has merge conflicts due to recent upstream merges. |
This PR should capture RTL as well, unless you want to retarget it to a branch
src/cdk/table/cell.ts
Outdated
@@ -59,6 +66,26 @@ export class CdkColumnDef { | |||
} | |||
_name: string; | |||
|
|||
/** Whether this column should be sticky positioned on the start of the row */ | |||
@Input('sticky') |
The argument to @Input()
is unnecessary when it matches the property name (here and below)
cc @crisbeto might be another fun lint check :)
(also, at some point we should probably think about upstreaming these to codelyzer)
/** | ||
* Mixin to provide a directive with a function that checks if the sticky input has been | ||
* changed since the last time the function was called. Essentially adds a dirty-check to the | ||
* sticky value. |
Include something in the description here that explains why the actual sticky
property isn't included in the mixin
src/cdk/table/sticky-styler.ts
Outdated
* @param stickCellCSS The CSS class that will be applied to every row/cell that has | ||
* sticky positioning applied. | ||
*/ | ||
constructor(private isNativeHtmlTable: boolean, private stickCellCSS: string) { } |
stickCellCSS
-> stickCellCss
Added two commits:
|
So, I started thinking about this in terms of how tree-shakable the code is. Let's chat on Thursday.
src/cdk/table/sticky-styler.ts
Outdated
const cellWidths: number[] = this._getCellWidths(rows[0]); | ||
const startPositions = this._getStickyStartColumnPositions(cellWidths, stickyStartStates); | ||
const endPositions = this._getStickyEndColumnPositions(cellWidths, stickyEndStates); | ||
const isLtr = this.direction === 'ltr'; |
You should invert this; it's better to explicitly check for RTL instead of LTR. We've had cases where the check was like this and dir
ended up being empty- the component then assumed it was in RTL.
Looks good overall.
src/cdk/table/sticky-styler.ts
Outdated
const endPositions = this._getStickyEndColumnPositions(cellWidths, stickyEndStates); | ||
const isRtl = this.direction === 'rtl'; | ||
|
||
for (let row of rows) { |
Why let
vs `const?
src/cdk/table/sticky-styler.ts
Outdated
return; | ||
} | ||
|
||
const numCells = rows[0].children.length; |
I'd make a pointer to rows[0]
, its used here and below.
One footnote of this is its not compatible with virtual scrolling, we would need to rethink some of this when we apply that strategy. |
LGTM
Amazing!!! EDIT : if i'm working straight off of master what sort of syntax should I employ to sticky a column? |
For examples on how to use the sticky stuff, check out the examples that start with "table-sticky-*" https://github.com/angular/material2/tree/master/src/material-examples |
Next minor release is probably going to be in a couple weeks |
@andrewseguin thanks! I haven't figured out how to have an angular cli boilerplater with master version of material2 though. (tried ...sorry I'm a beginner |
Hi, is there any specific date when the Mat-table release with sticky/ pinned columns for IE be publicly available |
Hi, Yes, that is really cool and i really need the sticky columns for my project. So if possible i need to know more a less when this is going to be released. Thanks :) |
@sadplace three comments higher : #11483 (comment) |
Sticky is not working in IE. In chrome also, Sticky column alignments are breaking while scrolling. |
Hi, Sticky/Fixed header is not working in IE. Is there any plan to fix this issue in next release? |
@andrewseguin are You guys planning to add examples showing sticky header and columns to main material site (https://material.angular.io/)? |
IE11 explicitly won't be supported; we decided that the amount of code it would take to work around the absence of Andrew was just able to finish up the feature before going out-of-office for a while; I'd be happy to accept PRs that add examples for the docs site. |
Hi! I have updated to the latest version, but if i add the sticky keyword to multiple columns, the columns go behind each other other then staying at the position where they are. So all three of my colums end up sticky on the left side of the screen: `
` Please advice! |
I edited, because in the meanwhile found more info and solutions: I have a similar problem. It seems that it is browser dependant:
My solution No.1: Attach My solution No. 2: Define updated yesterday, July 5th (v.6.3):
|
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. |
Closes #5885, #5890
The text was updated successfully, but these errors were encountered: