Add cache to viewdescriptorservice #88656
Conversation
|
@sbatten Not sure why this looks bit complicated. I am guessing the implementation as follows:
Let me know if I am missing anything else here? |
|
I think we can do a bit of clean up without your changes and after that your changes can become more simpler. for eg: try removing the events |
|
|
||
| this.onDidChangeActiveViews({ added: viewDescriptorCollection.activeViewDescriptors, removed: [] }); | ||
| viewDescriptorCollection.onDidChangeActiveViews(changed => this.onDidChangeActiveViews(changed), this, disposables); | ||
|
|
||
| this.viewDescriptorCollections.set(viewContainer, { viewDescriptorCollection, disposable: disposables }); | ||
|
|
||
| // Check if we have any views that were awaiting container registration due to a cached position |
There was a problem hiding this comment.
Looks this comment is outdated
| const disposables = new DisposableStore(); | ||
| const viewDescriptorCollection = disposables.add(new ViewDescriptorCollection(this.contextKeyService)); | ||
| viewDescriptorCollection.addViews(this.viewsRegistry.getViews(viewContainer)); | ||
| private getViewContainerById(id: string): ViewContainer | undefined { |
There was a problem hiding this comment.
Is this method does not exist on view container registry? If does not, lets please create one there and use it.
| this.deregisterGroupedViews(regroupedViews); | ||
| } | ||
|
|
||
| private regroupViews(containerId: string, views: IViewDescriptor[], seedMap?: Map<string, IViewDescriptor[]>): Map<string, IViewDescriptor[]> { |
There was a problem hiding this comment.
I do not see seedMap is passed any where. So this arg is not needed.
|
LGTM. Provided some feedback. |
|
will update and merge thanks |
refs #85164