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

'Peek Definition' missing from context menu #85218

Closed
gjsjohnmurray opened this issue Nov 20, 2019 · 9 comments
Closed

'Peek Definition' missing from context menu #85218

gjsjohnmurray opened this issue Nov 20, 2019 · 9 comments
Assignees
Labels
editor-symbols definitions, declarations, references under-discussion Issue is under discussion for relevance, priority, approach
Milestone

Comments

@gjsjohnmurray
Copy link
Contributor

Issue Type: Bug

VS Code version: Code - Insiders 1.41.0-insider (bd8425f, 2019-11-20T09:17:36.311Z)
OS version: Windows_NT x64 10.0.17763

image

The keyboard equivalent (Alt+F12 on Windows) does still work.

On 1.40.1 it is there:

image

@jrieken jrieken added this to the November 2019 milestone Nov 20, 2019
@jrieken jrieken added editor-symbols definitions, declarations, references under-discussion Issue is under discussion for relevance, priority, approach labels Nov 20, 2019
@jrieken
Copy link
Member

jrieken commented Nov 20, 2019

We have done that on purpose in an effort to cleanup the context menu. All peek-commands are still there, e.g accessible via keybindings or via the command palette but we redesign the context menu like this... Two options that I see

  1. Add another group to the context menu which contains peek commands, like Peek Definition, Peek Declaration, Peek Implementation, Peek References,
  2. Toggle commands when holding alt, similar to editor title actions, e.g hold alt while the menu shows goto-commands would turn them into their alternative peek-commands

Personally, I'd prefer option 2 because it will keep the context menu small and gives all the power, however at the cost of being harder to discover.

[edit] screen capture of how this works on the Mac Nov-20-2019 17-06-11

@jrieken
Copy link
Member

jrieken commented Nov 20, 2019

For completeness - this has been mentioned before here: #83752 (comment)

@gjsjohnmurray
Copy link
Contributor Author

How about grouping them all on a "Peek" submenu? That'd make them discoverable to folk who rely on right-click to find out what they can do on things (as opposed to reading the doc).

@gjsjohnmurray
Copy link
Contributor Author

Maybe this case can get some activity underway on #9827 🙏

@jrieken
Copy link
Member

jrieken commented Nov 21, 2019

Yeah, it would require to implement sub menus. I'll do some exploring

@jrieken
Copy link
Member

jrieken commented Nov 21, 2019

I have pushed this to master

Screenshot 2019-11-21 at 12 27 24

jrieken added a commit that referenced this issue Nov 21, 2019
@gjsjohnmurray
Copy link
Contributor Author

I like this. Just wondering why the sequence of "Peek" options differs from the sequence of "Go to" options.

jrieken added a commit that referenced this issue Nov 21, 2019
@jrieken
Copy link
Member

jrieken commented Nov 21, 2019

The order should now be the same... Anyways, we aren't sold on the sub menu approach yet. There are concerns about this being too noisy and therefore some questions why you have missed "Peek Definition".

  • Is that something you simply noticed?
  • Is that something you use (regular/rare) but only through the context menu and not via keybinding or F1?
  • Is this the only peek command that you are missing?

@jrieken
Copy link
Member

jrieken commented Nov 22, 2019

We will stick with the sub context menu for now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
editor-symbols definitions, declarations, references under-discussion Issue is under discussion for relevance, priority, approach
Projects
None yet
Development

No branches or pull requests

2 participants