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

Viewlet: header gets updated for each view within #45033

Closed
bpasero opened this issue Mar 5, 2018 · 6 comments
Closed

Viewlet: header gets updated for each view within #45033

bpasero opened this issue Mar 5, 2018 · 6 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug layout General VS Code workbench layout issues verification-found Issue verification failed verified Verification succeeded
Milestone

Comments

@bpasero
Copy link
Member

bpasero commented Mar 5, 2018

Steps to Reproduce:

  1. add a breakpoint here
  2. open the debug view

=> the selectbox is created 4 times for each view within

I find that weird, I would only expect the header of a viewlet to change when there is a transition from 1 view to many views or vice versa. And I would possibly not expect this on first creation time of the view where its already known how many views are being added.

@bpasero bpasero added bug Issue identified by VS Code Team member as probable bug layout General VS Code workbench layout issues labels Mar 5, 2018
@isidorn isidorn added this to the March 2018 milestone Mar 5, 2018
@isidorn
Copy link
Contributor

isidorn commented Mar 5, 2018

The issue here is panelViewet updates the title area whenever a split view is added.
However this is only needed for the explorer (as some actions can move to the title area).
So I have pushed a fix for this to be only done for the explorer viewlet
@sandy081 please reveiw

@isidorn isidorn closed this as completed in 43ead81 Mar 5, 2018
@sandy081
Copy link
Member

sandy081 commented Mar 5, 2018

@isidorn View headers and title area are updated depending on the number of views. They are a pair and to be invoked together. I think moving updateTitleArea away from it makes it disconnected and broken.

updateHeaders does it smarty by checking if there is a single view or not. updateTitleArea should also do the same.

I would suggest to combine them into single method and do the updates?

isidorn added a commit that referenced this issue Mar 5, 2018
@isidorn
Copy link
Contributor

isidorn commented Mar 5, 2018

@sandy081 thanks for feedback.
I think updateHeaders and updateTitleArea can not be moved to a single method since updateTitleArea needs to be called when transitioning in and out of single view mode and that is not the case with updateHeaders. Still I think I covered some of your concerns with the attached commit.

Feel free to change it if you do not like my approach.

@sandy081
Copy link
Member

sandy081 commented Mar 6, 2018

@isidorn your argument makes sense. New change looks good to me.

@bpasero bpasero reopened this Mar 27, 2018
@bpasero
Copy link
Member Author

bpasero commented Mar 27, 2018

@isidorn the SelectBox is created each time I open the debug view but never disposed it seems. There maybe more things leaking when switching views in the title area, I have not checked.

@bpasero bpasero added the verification-found Issue verification failed label Mar 27, 2018
@isidorn
Copy link
Contributor

isidorn commented Mar 28, 2018

@bpasero yes that is because I am not caching my debug actions which I plan to tackle as part of this debt item #46867. However this is not related to the main issue thus closing this one

@isidorn isidorn closed this as completed Mar 28, 2018
@bpasero bpasero added the verified Verification succeeded label Mar 28, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators May 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug layout General VS Code workbench layout issues verification-found Issue verification failed verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

4 participants
@bpasero @isidorn @sandy081 and others