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

Standardize windowing behavior from Remote Explorer view #8397

Closed
joyceerhl opened this issue Apr 20, 2023 · 15 comments
Closed

Standardize windowing behavior from Remote Explorer view #8397

joyceerhl opened this issue Apr 20, 2023 · 15 comments
Assignees
Labels
feature-request Request for new features or functionality on-testplan polish
Milestone

Comments

@joyceerhl
Copy link

joyceerhl commented Apr 20, 2023

Part of microsoft/vscode#178854

All remotes which contribute views to the remote explorer currently offer different ways to open a remote in a current or new window. Ideally we would offer the same options across all our remotes to reduce hesitation when choosing to open a remote.

Remote view Open in current window Open in new window
image
image ✅ (uses right arrow instead of window codicon)
image ✅ (available with alt modifier in inline actions)
image ✅ (uses folder codicon instead of window codicon)
image ✅ (not available in inline actions)

Proposal:

  • Always give the user the option to choose between using the current window and the new window
    • Suggestion 1: Offer one inline action and use alt modifiers, this is done by setting a command ID in the alt property for a command contributed to view/item/context (only Remote Repos does this today):
      image

    • Suggestion 2: Offer two inline actions (one for opening in current and another for opening new window)

    • Suggestion 3: Only offer one option (open in current or open in new window)

  • Use common codicons so that user doesn't need to hover to know what the action will do
    • $(window) for current window, $(empty-window) for new window
  • State location in the command name so that it's unambiguous what location will be used
    • E.g. Remote Repos and Containers should have 'Open in Current Window' and 'Open in New Window'
@joyceerhl joyceerhl added feature-request Request for new features or functionality polish labels Apr 20, 2023
@joyceerhl joyceerhl added this to the April 2023 milestone Apr 20, 2023
@joyceerhl joyceerhl self-assigned this Apr 20, 2023
@joyceerhl
Copy link
Author

joyceerhl commented Apr 20, 2023

@bamurtaugh
Copy link
Member

Thanks so much for putting this together @joyceerhl!

Suggestion 2: Offer two inline actions (one for opening in current and another for opening new window)

I like this option to help avoid limiting users to just one path.

@joyceerhl
Copy link
Author

I just noticed that in the Ctrl+R recent list, the ctrl modifier means new window while the alt modifier means to use the same window:
image

Remote Repositories currently uses the alt modifier to denote a new window, and I wouldn't want to change the default action for Remote Repositories just to align with the recent list...so +1 for standardizing on two inline actions.

@roblourens
Copy link
Member

roblourens commented Apr 21, 2023

We also have the setting window.openFoldersInNewWindow which sets a default for whether the window is reused when opening a folder. I can't tell you exactly why we went the totally explicit route instead of trying to match folder opening behavior... I think that we were thinking of opening remotes as just being a sort of different thing than opening a folder? But I don't see a reason to change anything to use that setting at this point.

Also I don't think it's an issue if alt in the open recent picker works differently than an alt-action on a menu button, to me they are different things.

I'm fine with 1 or with 2. If it's 1, then both options should show up in the context menu too

@aeschli
Copy link
Contributor

aeschli commented Apr 21, 2023

Same with WSL. The Connect to WSL command is following the setting window.openFoldersInNewWindow. Should I remove that behavior in favor of having two commands?

@joyceerhl
Copy link
Author

joyceerhl commented Apr 21, 2023

@aeschli when I have "window.openFoldersInNewWindow": "off", the action from the WSL remote explorer view still opens the distro in a new window:
image

@joyceerhl
Copy link
Author

I think for SSH it makes sense that you do not currently observe "window.openFoldersInNewWindow" because in my experience when you first connect to the SSH remote there isn't an initial folder open, instead it's an empty remote window. WSL appears to be the same.

@joyceerhl joyceerhl modified the milestones: April 2023, May 2023 Apr 24, 2023
@joyceerhl
Copy link
Author

Moving this out to May so I can get input from all remote owners on this issue.

@bamurtaugh
Copy link
Member

Here's a related issue on this topic: #8356

@aeschli
Copy link
Contributor

aeschli commented Apr 26, 2023

when I have "window.openFoldersInNewWindow": "off", the action from the WSL remote explorer view still opens the distro in a new window

Ok you are right, I didn't realize that. On the distro item, it's currently a force new window. On recent folders, it's the setting.

I'm not a big fan of the setting (window.openFoldersInNewWindow) as few users know about it. But on the other hand, it's (consistently) followed by the workspace commands like open folder, open recent and I'm a huge fan of consistency :-)

@joyceerhl
Copy link
Author

I'd suggest that we offer both options (open in current window, open in new window) for the remote explorer (inline as well as context menu), as I wouldn't want to take away functionality for the remotes which already offer the ability to do both. What do you think @aeschli?

@chrmarti
Copy link
Contributor

That's what Remote-SSH currently does which I like for being very clear. It does take some space, but I think it's still a good trade-off as a user setting can't cover that you don't always want the same behavior and the modifier keys tend to be too subtle for new users. (Consistency with modifier keys where we support them would still make sense though.)

@aeschli
Copy link
Contributor

aeschli commented May 11, 2023

Sounds good. So to summarise, for the remote explorer context menu we add two commands that are explicit, and we do not use the window.openFoldersInNewWindow setting .

@eleanorjboyd
Copy link
Member

agreed that I think we should maintain both options (open in current window, open in new window) so this sounds good to me. Thanks!

@joyceerhl
Copy link
Author

Closing this issue as it looks like Containers, WSL and Remote Repos have updated windowing behavior in the remote explorer and SSH/Tunnels has no changes required. Thank you everyone!

@github-actions github-actions bot locked and limited conversation to collaborators Jul 8, 2023
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 on-testplan polish
Projects
None yet
Development

No branches or pull requests

6 participants