configuration profiles dialog. Added context menu and shortcuts for profiles#20435
configuration profiles dialog. Added context menu and shortcuts for profiles#20435amirmahdifard wants to merge 4 commits into
Conversation
…iles list of configuration profiles dialog
SaschaCowley
added
the
conceptApproved
|
@seanbudd Thanks! I've applyed all your addressed reviews. Typ hints for evt, I've also seperated Translater labels (sorry about that I didn't know that will confuse translaters!). And I've updated the user gide. |
| labelDeactivate = _("Manual deactivate") | ||
| # Translators: Context menu item label to manually activate a profile. | ||
| labelActivate = _("Manual activate") |
There was a problem hiding this comment.
| labelDeactivate = _("Manual deactivate") | |
| # Translators: Context menu item label to manually activate a profile. | |
| labelActivate = _("Manual activate") | |
| labelDeactivate = _("Deactivate") | |
| # Translators: Context menu item label to manually activate a profile. | |
| labelActivate = _("Activate") |
or
| labelDeactivate = _("Manual deactivate") | |
| # Translators: Context menu item label to manually activate a profile. | |
| labelActivate = _("Manual activate") | |
| labelDeactivate = _("Deactivate now") | |
| # Translators: Context menu item label to manually activate a profile. | |
| labelActivate = _("Activate now") |
There was a problem hiding this comment.
The reason I called this label as manual activate and manual deactivate, was because the button's names were like that. But if you want the items names here to be diffrent, then i'm ok. (#20435 (comment))
Please reply to review comments directly, replying in the main comments makes the conversation really hard to track.
We should keep the names consistent between buttons. Please revert my suggested changes.
There was a problem hiding this comment.
ok i'm sorry about incorrect comment place.
you're right. But what should I do, use the button's names on context menu items? or should I use your new suggestion on buttons names as well?
I think your new suggestion which is "activate" and "deactivate" is much better raather than buttons names which is manual activate and manual deactivate because In that case we need to hear the word "manual" before hearing the state.
so I think it's best to use your suggestion on the buttons as well. "activate" and "deactivate that you suggested is the best of both worlds.
There was a problem hiding this comment.
Sean's suggestion was to turn back to the original name.
I support it since "Manual activate" is clearer. Indeed, you can have an application profile, e.g. for Outlook, which is triggered when in Outlook but not necessarily manually activated. If you remove "Manual", "Activate" may be confusing: why a "triggered" (and active) profile could still be activated?
Let's keep the original names for this PR and, if necessary, discuss button names in a subsequent issue/PR.
There was a problem hiding this comment.
In that case, We can also call them "activate manually" and "deactivate manually" To hear the state first.
Also, When a profile is manually activated, In the profiles list, it says
test (editing, manual)
So I think this also would be good idea too. Lets see
|
@seanbudd hi |
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
|
@seanbudd done. I've applyed your suggestion regarding the label and the user gide. |
4 participants
Link to issue number:
fixes #18169
Summary of the issue:
The configuration dialog was not supporting a context menu and shortcuts. When I Opened the issue, feedback suggestions came from users regarding the shortcuts (f2 to rename), (delete to delete) useful shortcuts
Description of user facing changes:
A context menu has been added to the profiles list of configuration profiles dialog and also shortcuts, focusing a profile, press f2 to rename the profile, and delete to delete the profile
Description of developer facing changes:
none
Description of development approach:
Added a on context menu method contaning actions and statements regarding taking care of profile actions are not available for normal configuration profile, and also added a charhook method, and bound them to the profiles list.
Testing strategy:
manual testing. Tested The configuration profiles dialog with the folowing. Tested that the context menu works, the actions inside it are all correctly working and bound to the existing methods. Same for onCharhook. Made sure that shortcuts are correctly working and bound to the existing rename and delete profiles method.
Known issues with pull request:
none
Code Review Checklist: