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

Enabling key accelerators on context buttons. #2232

Conversation

ryanbodrug-microsoft
Copy link
Contributor

@ryanbodrug-microsoft ryanbodrug-microsoft commented Apr 19, 2020

Summary of the Pull Request

Allows users to use shortcuts for actions specific to the selected row. eg. CTRL-SHIFT-ENTER = Run as Admin, Ctrl-Shift-E open folder location, etc.

References

Detailed Description of the Pull Request / Additional comments

  1. Add new object ContextMenuResult instead instead of reusing Result for both query results and context menu results.
  2. Binding KeyboardAccelerator keys to contextmenuitemviewmodel
  3. Enabling and disabling contextmenu items when selecting or deselecting each row. Because we are manually maintaining selectionwe can't use ScopeOwners as the textbox is really the only item ever in focus.

@crutkas . #1 is an explicit breaking change if we plan to support Wox plugins directly. However, I would rather be explicit with the data type, and write and adapter if we need to support existing wox plugins in the future. The design of how these actions are executed is quite different, so I doubt wox plugins would work out of the box anyway right now.

AcceleratorKeys

Validation Steps Performed

The following items are being validated by setting breakpoints on each of the modified actions and verifying that the action triggers from the expected accelerator keys

  • Executed Run As Admin on a Win32 Program result.
  • Executed Open File Location on a Win32 Program result
  • Executed Run as Admin on a Shell Program result -- Typed "> ping www.bing.com"
  • Executed Copy Path on a Indexer Plugin result.
  • Executed Open File Location on Indexer Plugin result.
  • Executed Copy Path on a Folder Plugin result.
  • Executed Open File Location on a File plugin result.
  • Verified that Actions for a previously selected row don't fire. IOW - They are disabled.

1. Add new object ContextMenuResult instead instead of reusing Result for both query results and context menu results.
2. Binding KeyboardAccelerator keys to contextmenuitemviewmodel
3. Enabling and disabling contextmenu items when selecting or deselecting each row.  Because we are manually maintaining selectionwe can't use ScopeOwners as the textbox is really the only item ever in focus.
@crutkas
Copy link
Member

crutkas commented Apr 19, 2020

My gut says we can make both work. No matter Rabat we need to work with everyone due to the misspelling in the actual interface too.

@ryanbodrug-microsoft
Copy link
Contributor Author

My gut says we can make both work. No matter Rabat we need to work with everyone due to the misspelling in the actual interface too.

I agree. I just wanted to call out its not going to be an "it just works" scenario.

@ryanbodrug-microsoft ryanbodrug-microsoft marked this pull request as ready for review April 19, 2020 22:17
Copy link
Contributor

@dsrivastavv dsrivastavv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ryanbodrug-microsoft ryanbodrug-microsoft merged commit 9380095 into microsoft:dev/PowerLauncherCustomUI Apr 19, 2020
@ryanbodrug-microsoft ryanbodrug-microsoft deleted the user/ryanbod/keyboard_accelerator_context_buttons_rebase branch April 19, 2020 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants