Add grid "Open To Depth" functionality #4004
Conversation
…OW_LEVEL_OPENER
…OW_LEVEL_OPENER
|
Corresponding TB PR: |
…OW_LEVEL_OPENER # Conflicts: # CHANGELOG.md
data/RecordAction.ts
Outdated
| @@ -81,6 +81,11 @@ export interface ActionFnData { | |||
| [x: string]: any; | |||
| } | |||
|
|
|||
| export interface DisplayFnData extends ActionFnData { | |||
| /** Default display config for the action */ | |||
| defaultConfig?: PlainObject; | |||
There was a problem hiding this comment.
Could we type this now? We have described the object exactly as well as what we pull off of it (the return from getDisplaySpec(). Trying to stamp out these PlainObject types where possible.
Then we could also type the return, from RecordActionSpec:
/** Function called prior to showing this item. */
displayFn?: (data: DisplayFnData) => PlainObject;
There was a problem hiding this comment.
I think this might be cruft. Don't think this defaultConfig is set or used anywhere.
| @@ -33,7 +33,7 @@ export type GridContextMenuToken = | |||
| | 'autosizeColumns' | |||
| | 'copyCell' | |||
| | 'colChooser' | |||
| | 'expandCollapseAll' | |||
| | 'expandCollapse' | |||
There was a problem hiding this comment.
We should call out this change in the changelog - apps might be using this token directly in cases where they have taken over returning what should / should not be in the context menu.
I don't think it needs to rise to the level of a major version breaking change, but it does warrant some mention of it. (We have a few cases where we call out a very minor breaking change with a ⚠️ Note: intro.)
There was a problem hiding this comment.
Actually I don't think I intended to make this change. I had left support for this in the MenuSupport file. Lee and I both thought it would be good to support this for backward compatibility.
| } else { | ||
| agApi.forEachNode(node => { | ||
| node.setExpanded(node.level < this.expandToLevel); | ||
| }); |
There was a problem hiding this comment.
I was wondering how this iteration over every node holds up with very large grids. Tried our Toolbox Admin > Tests > Grid page. Allowed it to load its default (if intense) 200k records, the swapped to tree mode. All OK, but then tried expanding to level three and the tab locked up with 100% CPU for...well, I gave up waiting.
Note that this does not occur with pre-existing expand/collapse all - I can use the toggle in the tree column header and the app is able to execute that operation OK and remain responsive.
Checking first to see how you were looking at / thinking about performance here, happy to discuss what we could do....
There was a problem hiding this comment.
In similar code in a client app we are setting the expanded property on the row nodes directly and then calling agApi.onGroupExpandedOrCollapsed() after updating all the nodes, might be worth trying to see if that performs better
There was a problem hiding this comment.
This appears to improve things by a lot.
…OW_LEVEL_OPENER
# Conflicts: # CHANGELOG.md
|
Hi - I know that things have been even more busy than usual with client work for you @febbraiod , but just catching up and saw your note re. change to bulk-setting expanded flag on nodes improving things "a lot" - sounded promising :) I'm eager to check out the latest and see how things perform, get a sense of what else (if anything) we'd need to address before we could merge and release. Give a shout here please if any known TODOs or blockers, discussion items, etc |
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.