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

Panels should toggle focus between editor and panel just like other views #7540

Closed
4 tasks
sandy081 opened this issue Jun 10, 2016 · 23 comments
Closed
4 tasks
Assignees
Labels
feature-request Request for new features or functionality ux User experience issues
Milestone

Comments

@sandy081
Copy link
Member

sandy081 commented Jun 10, 2016

Current behavior of panels is to toggle visibility when

  • Selecting them from the View menu
  • Using key board short cuts
    Eg: Ctrl + Shift + U toggles visibility of the output panel.

Instead they should be having the same behavior as other views like Explorer, Search, Git, Debug.
They toggle the focus between active editor and themselves.

  • Easy to navigate between editor and panels
  • Makes consistent across
  • Removes duplicate behavior: there is a specific command (Ctrl + J) to toggle panel's visibility
  • Output
  • Problems
  • Terminal
  • Debug
@sandy081
Copy link
Member Author

I have already implemented this for Problems view.

@sandy081 sandy081 added the feature-request Request for new features or functionality label Jun 10, 2016
@sandy081
Copy link
Member Author

I implemented a base action that toggles the focus just like ToggleViewletAction

https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/browser/panel.ts

@Tyriar
Copy link
Member

Tyriar commented Jun 10, 2016

The terminal is planning toggle and focus commands, users are going to want a combined toggle visibility and focus which is what toggle currently is.

@bpasero
Copy link
Member

bpasero commented Jun 11, 2016

-1 for doing this for the output because there is really no use case for putting focus into the output panel. I often use Cmd+Shift+U to quickly bring in the output to see for compile errors and then want to dismiss it again.

@yisibl
Copy link
Contributor

yisibl commented Jun 12, 2016

In Sublime, Ctrl + J used to make multiple lines of text into one line.

@sandy081
Copy link
Member Author

@Tyriar You are right.. all panels are having that behavior now (toggle display and focus). I think not toggling the display would be better user experience. Switching focus between active editor and panels like Problems, Terminal and Debug would be really helpful. Instead of inventing a new command for this, it would be good to slightly change the existing command to do this. This will also be consistent with views behavior.

@bpasero Yah, I do not think of any use case either. But I think this would cause some inconsistency.. To dismiss any panel, user can use Ctrl/Cmd + J.
Do you think is there any loss of functionality if we make Output panel consistent with other panels?

@gregvanl
Copy link

I would be very cautious about changing the current keyboard short cut toggling behavior for the panels. Developers get muscle memory (Ctrl+Shift+Y, Ctrl+Shift+U, Ctrl+`) for quickly bringing up and dismissing a panel and they likely won't be happy if it changes. I don't think there is harm in having duplication with Ctrl+J to toggle any active panel.

@sandy081
Copy link
Member Author

That is a valid point. But since we are simplifying views by getting them under single section, it is also important to have consistent behavior.

All views:

  • specific command to show and toggle focus between active editor and themselves
  • cmd + B to toggle side bar
  • cmd + J to toggle bottom bar

I think users might like this simplification and consistency and accept the change.

@sandy081
Copy link
Member Author

@gregvanl Another usecase where user wants to toggle focus between editor and Problems view (or other views in bottom)

  • While fixing problems or debugging its most common to navigate between editor and views. Currently, you would need to toggle the view twice to get this.

@isidorn
Copy link
Contributor

isidorn commented Jun 23, 2016

-1 for this proposal - I personally do not think it is adding value for the output and debug panel.
The users are already used to the current behavior and changing that might break their muscle memory.

As Ben pointed out giving the focus to the output does not make much sense also.
If you disagree then we should bring this item up on the next ux meeting.

@sandy081
Copy link
Member Author

Since this is still under discussion, I reverted Problems views to be consistent with other bottom views.

@FrenchBen
Copy link

Can we add the Explorer panel to this list?
CMD + Shift + E - Should toggle the Explorer panel's visibility.

@isidorn
Copy link
Contributor

isidorn commented Jul 5, 2016

I suggest we bring this up in tomorrow's ux sync.

@isidorn isidorn added the ux User experience issues label Jul 5, 2016
@isidorn isidorn added this to the July 2016 milestone Jul 5, 2016
@bpasero
Copy link
Member

bpasero commented Jul 6, 2016

To summarise, as of today:

  • views toggle focus
  • panels toggle visibility

I think we did this originally for the panel because we wanted it to dismiss very easily. For viewlets the behaviour is afaik very very old, and maybe we did not have a UX discussion on this after all.

I think changing the behaviour for views is a questionable idea because all users might be very used to be able to quickly jump between view and editor using the keybindings.

For the panels I think we have some room to change the behaviour because they are relatively new. Though I am not sure if our current behaviour is good or bad, so we might still cause issues changing it for some users.

At least, I give us +1 to be consistent (all views behave the same, all panels behave the same).

@andrew-w-ross
Copy link

My 2c but is it possible just to get a focus panel action? Currently the only solution to refocus the panel is to toggle close it and open it again.

@bpasero
Copy link
Member

bpasero commented Jul 8, 2016

That would make sense, we have it for the sidebar already:

image

@isidorn can we add it as global command? I noticed that this command will actually open the sidebar if it is closed, so it should do the same for panels.

@isidorn
Copy link
Contributor

isidorn commented Jul 8, 2016

@bpasero sure I can add this. While adding this let me try to wrap up on the whole issue.
Did we land on something here @sandy081 ?

isidorn added a commit that referenced this issue Jul 8, 2016
@isidorn
Copy link
Contributor

isidorn commented Jul 8, 2016

Added the global workbench action to focus into panel so we are consistent with the sidebar

@andrew-w-ross
Copy link

@isidorn That was incredibly fast, thanks.

@Tyriar
Copy link
Member

Tyriar commented Jul 8, 2016

I want to remove workbench.action.terminal.focus in favor of workbench.action.focusPanel now, but I see you removed the command deprecation code in 375bd62 @bpasero? I would think that this code should stay in for good, with the command mappings being present for at least several versions (I just got a bug of several people experiencing issues upgrading 1.0 to 1.3 for example).

@sandy081
Copy link
Member Author

Introducing a separate action for focussing the panels brings much closer to our views behavior and its a good step forward.

Regarding toggling panels, I do not know how users might react with the change. I think we can close this and probably wait for feedback for the change.

@pkazmier
Copy link

pkazmier commented Jul 14, 2016

Personally, I've bound cmd+0 to the new focus panel action. Although I'm a keyboard junkie, I don't need keyboard focus in the explorer as that is a mouse-only part of the screen for me. On the other hand, it was frustrating that I could not get focus in an open terminal window without double ctrl+`. I like the cmd+0 binding as it mirrors the functionality with cmd+1, cmd+2, etc ... In any case, thank you for the new action and an amazing editor!

@thematthopkins
Copy link

I most commonly use the errors panel to jump to the next error. The toggle behavior makes this a little awkward, because I'd:

  1. Shift + Ctrl + M to navigate to the first error
  2. Fix the error
  3. Use Shift + Ctrl + M close the errors panel
  4. Use Shift + Ctrl + M open the errors panel + focus

I could use different shortcuts depending on which panel is active, but that seems awkward.

Focus as the default action for panels seems to make sense to me even for output, as I'd usually want to use pgup/pgdn to navigate once it's up.

In most cases when I close the panel, focus is already in the panel, so I can hide via the escape key. A keyboard shortcut dedicated to this doesn't seem necessary, though others may disagree.

@sandy081 has already voiced these opinions, so this is mostly agreeing with the original proposal.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality ux User experience issues
Projects
None yet
Development

No branches or pull requests

10 participants