Skip to content

Prev/Next Panel/Sidebar view. Part of #44625 #59270

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

Merged
merged 3 commits into from
Oct 9, 2018
Merged

Prev/Next Panel/Sidebar view. Part of #44625 #59270

merged 3 commits into from
Oct 9, 2018

Conversation

octref
Copy link
Contributor

@octref octref commented Sep 24, 2018

Supersedes #44796.

@sandy081 @isidorn I think some methods in the Panel/Sidebar parts are confusing, for example:

  • From Panel perspective, Search is visible when search.location is set to panel. However, you can unpin the search panel through context menu like so:

    image

    In this case, even though Search panel is hidden (through unpinning), it is still visible.

  • In viewlet.ts, getViewlets and getAllViewlets should be better named to reflect what they do

@isidorn
Copy link
Collaborator

isidorn commented Sep 25, 2018

Assigning to October to review then

@isidorn isidorn added this to the October 2018 milestone Sep 25, 2018
@isidorn
Copy link
Collaborator

isidorn commented Oct 2, 2018

@octref great work! I like the changes and I like how this behaves.
I will merge if @sandy081 agrees

I was not sure about naming, but after second thought the current naming works for me.
fyi @bpasero
"Next Sidebar View" / "Previous Sidebar View"
"Next Panel View" / "Previous Panel View"

@octref as for your feedback regarding viewlet.ts feel free to open more prs with proposed changes so we discuss them and potentially merge. Thanks a lot!

@isidorn
Copy link
Collaborator

isidorn commented Oct 9, 2018

Since @sandy081 is not replying I will just merge this in.
I like the work.

@isidorn isidorn merged commit 2afc872 into master Oct 9, 2018
@isidorn isidorn deleted the octref/44625 branch October 9, 2018 15:37
@sandy081
Copy link
Member

Sorry I was on vacation.

LGTM

@octref octref mentioned this pull request Oct 29, 2018
2 tasks
@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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants