-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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
Show the context menu verb only when shift key is down #9358
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, this is exactly how I'd imagine this would be implemented. I'm gonna block this PR for a bit to give the team a chance to discuss in our next team sync - probably next Monday.
IMO, I definitely want there to be a setting to configure this, since people are totally split on whether they want this to appear on Click or Shift+Click. However, we don't have a really good way of configuring settings for the context menu quite yet. Setting up the context menu to be able to read the settings isn't something I'd ask of a community member, but we haven't really committed writing that code to a particular milestone quite yet. So I'm afraid that this PR might sit open for a while while we figure that out.
The code's totally fine, I just wanted to give you a heads up. Thanks!
Thanks @zadjii-msft . I agree the configurable context menu part can be a bit tricky. From my limited understanding of the code base atm, ShellExtension doesn't seem to like the headers in TerminalSettingsModel right now otherwise it should be possible to allow ShellExtension to read global settings? On DLL load ( Or, a Just some random thoughts. :D |
Hey so sorry to leave this one on read for so long. Apparently, I never came back and replied to this thread last year when we had this discussion 😕 IIRC, we were cool with the idea, but concurred that it needed to be something that needed a setting to toggle this behavior. As it stands, there's probably no line-of-sight on actually providing settings for the context menu entry at this time 😢 We've got a lot of cool ideas, just not any time to implement them. At this time last year, I think we were still hoping to get to doing the settings for the context menu a little faster, but alas, changing resourcing and priorities has tossed those plans to the wind. I also don't want to ask you to try and hook these things up, either. Theoretically, you could just add a project reference to My only big worry here is what the perf impact of loading the settings when the context menu is invoked. We'd have to also make sure to obey Again, apologies for leaving this on read so long. That's on me. |
This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment. |
So, until this feature is approved, are there any workarounds for users? The known method #7008 (comment) removes the menu completely, not just when Shift isn't pressed. |
Summary of the Pull Request
References
Related: #8105
PR Checklist
added/passed (Invoke-OpenConsoleTests
Total=329, Passed=329)Detailed Description of the Pull Request / Additional comments
Validation Steps Performed
Haven't found a proper way to test ShellExtension so I went creative: (much appreciated if anybody can point me to the proper way to test this :p)
C:\Program Files\WindowsApps\
C:\Program Files\WindowsApps\
for verb iconHKCR\PackagedCom\Package\Microsoft.WindowsTerminal_1.6.10571.0_x64__8wekyb3d8bbwe\Class\{9F156763-7844-4DC4-B2B1-901F640F5155}\
, changedDllPath
to..\WindowsTerminalShellExt.dll
In a folder, shift up;
In a folder, shift down:
On a folder, shift up:
On a folder, shift down: