Add experimental enableFullWidthScroll Grid feature
#3517
Conversation
cmp/grid/Grid.ts
Outdated
|
|
||
| private viewportResizeObserver: ResizeObserver; | ||
|
|
||
| @computed |
There was a problem hiding this comment.
why is this computed? don't think we expect this to change, be observable, or be expensive to check
cmp/grid/Grid.ts
Outdated
| override afterLinked() { | ||
| if (!this.isFullWidthScrollEnabled) return; | ||
| this.addReaction({ | ||
| track: () => this.model.isReady, |
cmp/grid/Grid.ts
Outdated
| ...getLayoutProps(props), | ||
| ...impl.agOptions | ||
| }), | ||
| div({ |
There was a problem hiding this comment.
I would make this its own HoistComponent to avoid extra re-rendering whenever the scrollbar calcs need to change. ALSO, weird that the (experimental) scrollbar dominates the render method the way it does now.
Ideally put all the state for this in a dedicated GridScrollBarModel/GridScrollBar pair in their own files.
There was a problem hiding this comment.
Especially nice to have these split out while this is experimental, but also will be useful even if this becomes built-in
|
Greg and I reviewed this and he has just a few tweaks to make. Hoping to merge this tomorrow! 👍🏻 |
cmp/grid/Grid.ts
Outdated
| return fragment( | ||
| const container = model.experimental['enableFullWidthScroll'] ? vframe : frame; | ||
|
|
||
| return container( |
There was a problem hiding this comment.
Can we avoid the extra level of DOM nesting that this appears to introduce? Previously we were returning a fragment at the top level, with a frame and supporting colChooser/filter dialogs inside.
Could those dialogs be moved to nest within the frame/vframe items (alongside the agGrid) so we still have a single outer (v)frame?
There was a problem hiding this comment.
Sorry about that- actually didn't mean to change the outer fragment- this was in a bad state mid-refactor. Just updated the PR
Hoist P/R Checklist
Pull request authors: Review and check off the below. Items that do not apply can also be
checked off to indicate they have been considered. If unclear if a step is relevant, please leave
unchecked and note in comments.
developbranch as of last change.breaking-changelabel + CHANGELOG if so.If your change is still a WIP, please use the "Create draft pull request" option in the split
button below to indicate it is not ready yet for a final review.