Skip to content
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

Allow viewlets to migrate between panel and sidebar #84642

Closed
wants to merge 5 commits into from

Conversation

sbatten
Copy link
Member

@sbatten sbatten commented Nov 13, 2019

The goal of the PR is to allow entire viewlets to move between the sidebar and panel.

As a first step, I have created what is currently called a Pane (name can be changed) to wrap the logic surrounding Panels, ViewContainers, Viewlets, and Views. Initial limitation for v0.0 is that we assume the view container has one view as is the case with Search. I have changed Search's implementation to use the PaneRegistry to start which now is working.

We can see that the interface for the PaneRegistry is similar to other Registries in the code but it does a poor job at abstracting away the concept of Views, Viewlets, and Panels. This is because it was easier to get Search working without dropping half of its implementation.

Next steps involve making a simpler view work with an API that abstracts away the older concepts. I.e. the new view would define a single view and handle its layout accordingly instead of defining a panel and view and viewlet.

Following that, I will look into adding multiple views per viewlet.

Lastly, polishing the basic experience.

  • Get a basic registry working
  • Get search working in the new model
  • Clean up the registry interface to abstract older concepts out
  • Get a simple single view working with the new model
  • Adapt Search to the simplified approach
  • Allow multiple views
  • Allow other views (built-in, extension contributions)
  • Polish the experience

@sbatten sbatten requested review from sandy081 and isidorn November 13, 2019 01:06
@sbatten sbatten self-assigned this Nov 13, 2019
@sbatten sbatten added this to the November 2019 milestone Nov 13, 2019
@sbatten sbatten added the layout General VS Code workbench layout issues label Nov 13, 2019
@sandy081
Copy link
Member

sandy081 commented Nov 15, 2019

My feedback on the design:

I have a basic question on the design, why are you not using ViewDescriptor and ViewRegistry which is already an abstraction for all views (in Sidebar & Panel). There is also ViewContainer which can be used for the location of ViewDescriptor. Currently only sidebar views use view descriptors. So my suggestion would be to start moving Panels using view descriptors by creating a new View Container for each panel (similar to viewlets). This can allow moving views not only across side bar and panel but also across each viewlet. It will also give infrastructure for providing custom views in panels.

Current ViewRegistry already supports moving views from one view container to another.

moveViews(views: IViewDescriptor[], viewContainer: ViewContainer): void;

Views are updated in the Viewlets when views are added/removed/moved.

As said, currently viewlets use view descriptors and the first step would be to let panels also us it.

@sbatten
Copy link
Member Author

sbatten commented Nov 15, 2019

@sandy081 greet feedback, when trying to support a more generic model for custom views yesterday I learned a little more about the "purpose" of the view containers. I will try to map some of what I learned into using the existing registries and apply your suggestions.

@kieferrm kieferrm mentioned this pull request Nov 18, 2019
68 tasks
@sbatten sbatten closed this Nov 20, 2019
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
layout General VS Code workbench layout issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants