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 moving debug viewlet items to the panel #90468

Closed
sbatten opened this issue Feb 11, 2020 · 5 comments
Closed

Allow moving debug viewlet items to the panel #90468

sbatten opened this issue Feb 11, 2020 · 5 comments
Assignees
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality on-testplan workbench-views Workbench view issues
Milestone

Comments

@sbatten
Copy link
Member

sbatten commented Feb 11, 2020

Tracking issue for adopting movable property in debug views

@sbatten sbatten added feature-request Request for new features or functionality debug Debug viewlet, configurations, breakpoints, adapter issues workbench-views Workbench view issues labels Feb 11, 2020
@sbatten sbatten self-assigned this Feb 11, 2020
@vscodebot vscodebot bot added this to the Backlog Candidates milestone Feb 11, 2020
@microsoft microsoft deleted a comment from vscodebot bot Feb 11, 2020
sbatten added a commit that referenced this issue Feb 11, 2020
@sbatten
Copy link
Member Author

sbatten commented Feb 11, 2020

with my commit, views can be moved, but there are a few rough edges.

  1. When all views are moved, there is an empty viewlet with the launch config selector toolbar.
  2. I didn't allow moving the simple start view because the above. if they moved together, maybe it would make more sense.
  3. Most of the views have some header actions on hover. When they are the only view in the container (pre-existing issue in sidebar), these actions are not visible. e.g. If watch is the only view in the view container, you have to know to right click the pane and add expression

@isidorn
Copy link
Contributor

isidorn commented Feb 12, 2020

To tackle 1 and 2 I suggest the following limitation for now: if a view is the last view in the container it is not moveable.
3 is fine for now.

I saw your changes to the debug views and they make sense overall. My only concern is that this feels like a lot of duplication, and I am not sure if the view should be the one that cares about having proper background based on where it is located. Does it make sense to extract this out of the views, so we avoid duplication and make adoption more easier?

@isidorn
Copy link
Contributor

isidorn commented Feb 12, 2020

Also let me know if you would like me to adopt the Explroer or if there is some more work needed in the debug views.

@sandy081
Copy link
Member

If user decides to move all views and leave it empty then it is up to the user. I think having special logic to not to move view when single might make things confused. So I would allow move them all and show the empty viewlet. How about unpinning the viewlet when empty?

@sbatten
Copy link
Member Author

sbatten commented Feb 12, 2020

After discussion, we will leave the current state as-is and allow the user to configure.

@sbatten sbatten closed this as completed Feb 12, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Mar 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality on-testplan workbench-views Workbench view issues
Projects
None yet
Development

No branches or pull requests

3 participants