Show the status of add-ons in the system config when copying user config to system config#19808
Conversation
|
Can you consider removing the author column from the dialog? Some Add ons have very long authors, which requires listening to a lot of useless information to obtain the checkbox status |
|
I have given a try to this for early feedback. Checkbox listMixing items with and without checkbox in the same list is not the best UX, more because:
Instead of the checkable items, could we add an action column containing "Copy" / "Do not copy" value for add-ons not already in the system-wide config and "Delete" / "Update" for add-ons already in the system-wide config. This could be the second column. And pressing space could allow to toggle the value in the column. At last, for add-ons only in system config but not in user one, you'd only have "Delete" value with no possibility to toggle it. About add-on buttonRegarding the "About add-on..." button: Does it open the information of the user version or the system-wide version? Dialog introduction textI was already of the opinion that the intro message of the "Copy add-ons" dialog was poorly worded and had to be reworded. This is more true with this new experience / feature. Extra noteBy the way, you should not use: Mixing items with and without checkbox in the same list is not the best UX, more because:
Instead of the checkable items, could we add an action column containing "Copy" / "Do not copy" value for add-ons not already in the system-wide config and "Delete" / "Update" for add-ons already in the system-wide config. This could be the second column. And pressing space could allow to toggle the value in the column. At last, for add-ons only in system config but not in user one, you'd only have "Delete" value with no possibility to toggle it. Regarding the "About add-on..." button: Does it open the information of the user version or the system-wide version? |
Done. I was already thinking of doing this, so I'm glad I wasn't the only one. The author information is still available through the "About add-on..." button. |
This goes for all checkboxes in NVDA.
Note that I have removed the author column, so there is less verbosity before you hear (or rather don't hear) that there is no checkbox. I've also added a prefix to add-ons without checkboxes.
Possibly in a future enhancement of this dialog. For now, what we have uses features already in wx or the underlying win32 controls. Your proposal would be more difficult (though by no means impossible) to implement.
User version, falling back to system version. In a future enhancement perhaps we could use a tabbed dialog to present both.
TIL! It happened to work here, but looking at the output it was indeed wrong. Thanks for the tip :) |
|
Perhaps disabling the checkbox is better than removing it? At least this ensures consistency in behavior, that is, all items have checkboxes. |
That is what I would prefer too, but to the best of my knowledge it isn't possible with a win32 list-view control. |
Yes, I agree with removing this column too; I was about to suggest it myself too.
Nice addition. This strongly mitigates the difficulty that user could encounter when discovering this list with unusual mixing of checkbox items with no-checkbox items.
I do not understand why it would be more difficult from a GUI point of view; it's just about removing the use of checkbox items and add a column containing the corresponding information in text. In the case of add-ons not present in user config, I've just thought another design.
It's just an improvement idea, but the current design that you have is already acceptable. You may want to ship this PR as is so that all beta PRs be quickly fixed. |
There was a problem hiding this comment.
Pull request overview
This PR enhances the “Copy settings to system-wide configuration” flow by showing which add-ons already exist in the system-wide config (and their versions), and by surfacing add-ons that will be removed when replacing the system config with the user config.
Changes:
- Updates the copy add-ons dialog UI/UX: adds “System-wide version”, renames “Version” to “User version”, removes “Author”, and lists system-only add-ons as “[remove]” without checkboxes.
- Adds lower-level Win32 list-view plumbing (new CommCtrl bindings + a list control that can suppress per-item checkboxes) to support the UI behavior.
- Introduces a side-effect-free helper to enumerate add-on manifests from a directory for system-config inspection.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| user_docs/en/userGuide.md | Updates user guide text to describe new dialog behavior and columns. |
| user_docs/en/changes.md | Updates changelog entry regarding deprecated WinUser structures. |
| source/winUser.py | Deprecates NMHdrStruct in favor of winBindings.user32.NMHDR. |
| source/winBindings/user32.py | Adds Win32 APIs/types needed for wndproc subclassing + defines NMHDR. |
| source/winBindings/CommCtrl.py | Adds CommCtrl list-view constants/structs for checkbox state manipulation. |
| source/gui/nvdaControls.py | Adds _CheckListCtrl that can remove item checkboxes by intercepting notifications. |
| source/gui/addonStoreGui/controls/messageDialogs.py | Updates _CopyAddonsDialog to show user/system versions and system-only removal rows. |
| source/addonHandler/init.py | Adds _getAddonManifestsFromPath and _loadManifest for manifest-only directory scans. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Another small issue: |
…g or its addons dir doesn't exist
…better memory management
Fixed. Good catch, thank you. |
Link to issue number:
Closes #19550
Summary of the issue:
Since #19446, users can select which add-ons should be copied to the system-wide configuration when replacing it with their current user configuration. However, they cannot see whether an add-on is already present in the system configuration, and if so which version is present.
Description of user facing changes:
Description of developer facing changes:
None in public API
Description of development approach:
_CopyAddonsDialogto take two dictionaries - one mapping add-on names toAddonobjects from the user config; and one mapping add-on names toAddonManifestobjects from the system config (gathered with the new new_getAddonManifestsFromPathfunction).AddonManifest, systemAddonManifest) pairs, and used that to populate the add-ons list. This allows us to show details of the user and system add-ons.wx.ListCtrlwhich allows removing the checkbox from specific list items.ListViewwhich sets the item's state image to0(0is no checkbox,1is unchecked and2is checked).IsItemCheckedimplementation, since the win32 macro that wx uses internally returns evaluates to true if the state image is set to0.Testing strategy:
Tested interacting with the dialog with various combinations of add-ons in the user and system configurations, and that in all cases the expected list of add-on IDs was returned. Testing was done by calling
from gui.addonStoreGui.controls.messageDialogs import _getAddonsToCopy;_getAddonsToCopy(None)in the python console when running from source._getAddonsToCopywas slightly modified to force it to use%ProgramFiles%\nvda\systemConfig\addons.Known issues with pull request:
None known
Code Review Checklist: