Add movable property to views and add toggle view commands #88823
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
see update to description |
13 tasks
sandy081
requested changes
Jan 22, 2020
| if (viewDescriptor.canMoveView) { | ||
| result.push(<IAction>{ | ||
| id: `${viewDescriptor.id}.removeView`, | ||
| label: this.isViewMergedWithContainer() ? nls.localize('toggleSpecificViewLocation', "Toggle {0} View Location", viewDescriptor.name) : nls.localize('toggleViewLocation', "Toggle View Location"), |
There was a problem hiding this comment.
Why not show the view name always?
src/vs/workbench/services/views/browser/viewDescriptorService.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/services/views/browser/viewDescriptorService.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/services/views/browser/viewDescriptorService.ts
Outdated
Show resolved
Hide resolved
|
@sandy081 updated with what we discussed. one issue that arose with the panel context menu entry is that we can only get this entry information once the panel (meaning the specific composite) has been instantiated. with that limitation, the move to sidebar command will only show when you right click the active panel header. this is still much better than how it was working this morning. If you have any desired changes, feel free to make them and merge the PR. |
- Use this menu id to get context menu action for a view
|
@sbatten Did following changes
|
|
Merging!!! |
sandy081
approved these changes
Jan 23, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
refs #85164
this is not compiling right now because i paused in the middle of a change.@sandy081 this is just a PR so you can see where my head is at. There are lot of things hacked together to see this working and play with different ideas while I sort it out. The most hacky stuff comes around where we actually surface the menu entries. I notice that panel headers do not have the same context menu luxuries that the viewlet has due to the composite bar and view headers being a combined concept for panel.
Notes from implementation:
Update: Moved logic into the service and moved service to its own filed due to cyclic dependency issue.
when a view is moved to the panel and then the window is reloaded, the view descriptor service needs to be able to create the cached view container at the point of the views registration because the container was created on the fly previously.
the cache may need to be enhanced to store that a cached view container was auto-generated and not registered the traditional way as well as the location info about that view container so it can be recreated.
Update: the cache now stores the id of the container along with its source view id and location if it is auto generated.
Update: by using the source view id to regenerate the id, we assume that the source view will be registered at some point. if it is not, we move the pending view back to its default location. this is the same logic as containers contributed by extensions.
/Cc @isidorn