[PowerToys Run] Plugin manager

[mykhailopylyp]

Summary of the Pull Request

What is this about:

Specs: https://github.com/microsoft/PowerToys/wiki/PowerToys-Run-Plugin-spec

What is include in the PR:

• Added Name and Description to IPlugin in order to take localized strings.
• Enable one action keyword for several plugins
• Move `disable drive detection warning' under the Windows Search plugin
• Plugins section UI

How does someone test / validate:
If you were testing PRs in feature/plugin-manager branch then delete PowerToys Run\settings.json file.

Test consistency of settings with the previous version:

• Start PT Run for v0.31.2 and try to search for something
• Modify few settings in PowerToys Run tab in settings
• Start local build
• Check if settings persisted, PT Run works and there is plugins section on PowerToys Run tab
• Start PowerToys v0.31.2 and verify that settings persisted and PT Run works

Feature testing:

• Check if settings are propagated to PT Run
• Try disabling a plugin, modifying Include in global result and Direct activation phrase for all plugins
• Switch the theme and check that the plugins section still looks okay
• Test tab navigation with the narrator. Take into account existing issue: 'Navigation to next or previous item does not work if the focus is on controls from AdditionalInfoPanel'
• Assign the same Direct activation phrase for several plugins and test it in PT Run

Quality Checklist

• Linked issue: [Run][Plugin Manager] Plugin management section (enable / disable, ...)  #5273
• Communication: I've discussed this with core contributors in the issue.
• Tests: Added/updated and all pass
• Installer: Added/updated and all pass
• Localization: All end user facing strings can be localized
• Docs: Added/ updated
• Binaries: Any new files are added to WXS / YML
  • No new binaries
  • YML for signing for new binaries
  • WXS for installer for new binaries

Contributor License Agreement (CLA)

A CLA must be signed. If not, go over here and sign the CLA.

[htcfreek]

Do we then merge the remaining tasks against master?

[ivan100sic]

Test consistency of settings with the previous version

Disable drive detection warning gets disabled after switching, but I think it's not a big deal. Everything else is consistent.

[mykhailopylyp]

@htcfreek

Do we then merge the remaining tasks against master?

We decided to postpone telemetry, direct shortcut, and some UI fixes. They will not be done in this release. Anyway, if you see something that should be done in this release feel free to comment.

[ivan100sic]

Apart from the comment on Disable drive detection warning, all functionality works perfectly. All relevant unit tests pass. Tested running PowerToys as normal user and admin. Tested enabling/disabling various plugins and checked that the change is propagated to PT Run. Since I've been following the work on this feature branch I only skipped through the code this time. To me, the UI looks great.

[enricogior]

Let's change this description to something actually useful! :)

Navigate folders starting from a drive letter 'c:\' or from the user home '~'

[enricogior]

Change the description to:
Allows you to do mathematical calculations (e.g. 5*3-2)

[enricogior]

We either use the term Returns or Search, consistency helps the UX.
Also remove in PowerToys Run.

[enricogior]

Let's add some example of system commands:

Allows to execute system commands (e.g. 'shutdown', 'lock', 'sleep' etc.)

Allows to execute commands (e.g 'ping', 'cmd', etc.)

[enricogior]

from #9653 (comment)

Opens URLs in your default browser.

[htcfreek]

from #9653 (comment)

Opens URLs in your default browser.

Plugin name should be updated to "URL Handler" too.

[enricogior]

Not a fan of the current description but we can live with it for now.

[enricogior]

Allows to execute system commands (e.g. 'shutdown', 'lock', 'sleep' etc.)

[htcfreek]

Not a fan of the current description but we can live with it for now.

What about "Switch between open windows"?

[mykhailopylyp]

@enricogior @htcfreek
Url handler is not the correct name. Actually, it is URI plugin. Should we adjust the name and description?
Name: URI handler
Description: Opens URIs in your default browser.

On the other hand, URL is more common.

[htcfreek]

@enricogior @htcfreek
Url handler is not the correct name. Actually, it is URI plugin. Should we adjust the name and description?
Name: URI handler
Description: Opens URIs in your default browser.

On the other hand, URL is more common.

Because MS Edge uses URL we should use URL.

[enricogior]

@mykhailopylyp @htcfreek
URL is a type of URI, but since the plugin only supports URL, we should use the term URL.

[enricogior]

What about adding a hint to let the users know that clicking on the description will open the plugin settings?
Right now the UI doesn't provide any hint and the discoverability is not ideal.

[enricogior]

We can remove the attribution for Window Walker from here, since it's now in the plugin's details.

[htcfreek]

What about adding a hint to let the users know that clicking on the description will open the plugin settings?
Right now the UI doesn't provide any hint and the discoverability is not ideal.

I suggest an arrow on the right side.
Or changing cursor on hover.

Btw: While testing today I saw the Plugin items look a bit movable when selecting and moving the mouse.

[mykhailopylyp]

Btw: While testing today I saw the Plugin items look a bit movable when selecting and moving the mouse.

It seems as default behavior of ListView.

[htcfreek]

Btw: While testing today I saw the Plugin items look a bit movable when selecting and moving the mouse.

It seems as default behavior of ListView.

I agree. Checked this today but forgot it.

[htcfreek]

We can remove the attribution for Window Walker from here, since it's now in the plugin's details.

Yes we have it in the plugin section. But only the developer and not the application's name and github url.

[htcfreek]

@htcfreek

We have two BUGS regarding the drive detection warning:

1. The warning isn't shown anymore when not disabled (and when disabled).

It is a regression. We should fix it. Am I missing something?

UPD: Can you confirm that it shows a warning for 0.31.2 and does not for this branch?

Yes I can confirm. I have checked this by enable warning on 0.32.1.

[mykhailopylyp]

Checked that driver detection warning settings propagate to the Indexer plugin so it is very unlikely that the regression is caused by this PR.

[mykhailopylyp]

With the setting uncheked

With the setting checked

It seems as I can not reproduce it. The warning is not the first item but it is a known issue.

[htcfreek]

Checked that driver detection warning settings propagate to the Indexer plugin so it is very unlikely that the regression is caused by this PR.

I will give it a second try later. But at the moment I reinstall my VM.
Let's not block the merge for this. I will open a issue later which then can be investigated and fixed.

[htcfreek]

I'll open the issues tomorrow.

Great. We will also merge the PR tomorrow.

@enricogior
I have identified all the remaining work items. Should we sync later or should I simply create the issues?

[enricogior]

@htcfreek
if you can create individual issues, that would be great! Thanks.

[htcfreek]

@enricogior
All the new issues for each of the remaining work items in this issue are created. (Except of #9872 (comment).)