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

[PT Run] [Settings] Additional Plugin Options: Allow description for checkbox #15853

Closed
htcfreek opened this issue Jan 30, 2022 · 17 comments
Closed
Assignees
Labels
Help Wanted We encourage anyone to jump in on these and submit a PR. Idea-Enhancement New feature or request on an existing product Priority-2 Bug that is medium priority Product-PowerToys Run Improved app launch PT Run (Win+R) Window Product-Settings The standalone PowerToys Settings application Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release. Run-Plugin Manager Issues with the PowerToys Run plugin manager Run-Plugin Things that relate with PowerToys Run's plugin interface

Comments

@htcfreek
Copy link
Collaborator

htcfreek commented Jan 30, 2022

Description of the new feature / enhancement

For some additional plugin options it would be great to add a description subtitle. Then we can provide additional information to the user.

Scenario when this would be used?

An optional plugin setting that needs additional information that we need to provide to the user.

Supporting information

I think the class property can be null if now subtitle should be shown.

@htcfreek htcfreek added Idea-Enhancement New feature or request on an existing product Product-PowerToys Run Improved app launch PT Run (Win+R) Window Product-Settings The standalone PowerToys Settings application Run-Plugin Things that relate with PowerToys Run's plugin interface Run-Plugin Manager Issues with the PowerToys Run plugin manager labels Jan 30, 2022
@ghost ghost added the Needs-Triage For issues raised to be triaged and prioritized by internal Microsoft teams label Jan 30, 2022
@htcfreek
Copy link
Collaborator Author

@crutkas
Can we give this a bit more priority. Some of the new WindowWalker settings name are a bit long. Having a subtitle would help here. (But I can't implement it, as it's a bit to complicated to me and touches winui code.)

cc: @niels9001

@htcfreek
Copy link
Collaborator Author

@crutkas
Can we give this a bit more priority. Some of the new WindowWalker settings name are a bit long. Having a subtitle would help here. (But I can't implement it, as it's a bit to complicated to me and touches winui code.)

cc: @niels9001

See screenshots in #16325.

@niels9001
Copy link
Contributor

@htcfreek There's a CheckBoxWithDescription control in PowerToys.Settings.UI. That allows you to set the header and description. Does that help to solve this issue?

@htcfreek
Copy link
Collaborator Author

htcfreek commented Feb 28, 2022

@niels9001
Technically no. The list of additional plugin options created dynamically. So we have to check for each element in the object if we need description or not. Additional this might touch SettingsUi.Library.
But if you asking for the user experience then definitely yes.

I can update wox code. But I don't find the settings code that has to be changed. And I don't know what has to be changed in xaml item template to select the checkbox control dynamically.

@htcfreek
Copy link
Collaborator Author

@niels901
Can you implement the feature in settings ui code? This would be great to have for v0.57.0 release as I worry that some of the new setting names will be to long when translated. 🤨

@crutkas
Copy link
Member

crutkas commented Feb 28, 2022

Help me understand impact here. What does it look like currently if this wasn’t prioritized.

@htcfreek
Copy link
Collaborator Author

Help me understand impact here. What does it look like currently if this wasn’t prioritized.

img

I worry that some of the translated strings become to long for the ui. Or does the checkbox control has word wrap enable?

@crutkas
Copy link
Member

crutkas commented Mar 1, 2022

It has word wrap. I don't think this would be too hard to implement as global results already has the secondary description

image

@crutkas
Copy link
Member

crutkas commented Mar 1, 2022

@htcfreek why won't CheckBoxWithDescription control work?

@crutkas crutkas added Help Wanted We encourage anyone to jump in on these and submit a PR. Priority-2 Bug that is medium priority and removed Needs-Triage For issues raised to be triaged and prioritized by internal Microsoft teams labels Mar 1, 2022
@htcfreek
Copy link
Collaborator Author

htcfreek commented Mar 1, 2022

@htcfreek why won't CheckBoxWithDescription control work?

The box work. But I can't implement it. I don't know how to change code that it works.

This file is easy to change:
https://github.com/microsoft/PowerToys/blob/main/src/settings-ui/Settings.UI.Library/PluginAdditionalOption.cs

But this file is my problem that I don't know how to change atm (@niels9001):
https://github.com/microsoft/PowerToys/blob/main/src/settings-ui/Settings.UI/Views/PowerLauncherPage.xaml#L252-L262

@niels9001
Copy link
Contributor

@htcfreek You'd need to replace the CheckBox with the CheckBoxWithDescription control, and then bind it's Content to DisplayLabel and Description to another string (which isn't defined yet in PluginAdditionalOption.cs. Maybe we should move to public string Title and public string Description there?

@htcfreek
Copy link
Collaborator Author

htcfreek commented Mar 1, 2022

@htcfreek You'd need to replace the CheckBox with the CheckBoxWithDescription control, and then bind it's Content to DisplayLabel and Description to another string (which isn't defined yet in PluginAdditionalOption.cs. Maybe we should move to public string Title and public string Description there?

What happened if description property of CheckBoxWithDescription is null? Not every setting needs a description.

@htcfreek
Copy link
Collaborator Author

htcfreek commented Mar 1, 2022

Maybe we should move to public string Title and public string Description there?

Might be more work as all existing setting might be broken then. We shouldn't do that. But for description we can omit the word Display.

@htcfreek
Copy link
Collaborator Author

htcfreek commented Mar 1, 2022

Let's wait with this issue until #16325 is merged.

@htcfreek
Copy link
Collaborator Author

htcfreek commented Mar 17, 2022

@niels9001
I looked at the control you mentioned: https://github.com/microsoft/PowerToys/blob/main/src/settings-ui/Settings.UI/Controls/CheckBoxWithDescriptionControl.cs#L42

  • Shouldn't we updated description in update method too? 🤔
  • Is the whole control recreated on opening the page? (Then I would simply add an if else condition to add/not add the subtitle text box.)

@htcfreek htcfreek self-assigned this Mar 17, 2022
@htcfreek htcfreek added the Status-In progress This issue or work-item is under development label Mar 17, 2022
@htcfreek
Copy link
Collaborator Author

@jaimecbernardo
Cam you please update issue state. thank you.

@jaimecbernardo jaimecbernardo added Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed Status-In progress This issue or work-item is under development labels Mar 21, 2022
@htcfreek
Copy link
Collaborator Author

htcfreek commented Apr 2, 2022

This is fixed with 0.57.0. Please head over to https://aka.ms/installpowertoys

@htcfreek htcfreek closed this as completed Apr 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Help Wanted We encourage anyone to jump in on these and submit a PR. Idea-Enhancement New feature or request on an existing product Priority-2 Bug that is medium priority Product-PowerToys Run Improved app launch PT Run (Win+R) Window Product-Settings The standalone PowerToys Settings application Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release. Run-Plugin Manager Issues with the PowerToys Run plugin manager Run-Plugin Things that relate with PowerToys Run's plugin interface
Projects
Status: No status
Development

No branches or pull requests

4 participants