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

Add-on store: Handle bulk install #15350

Merged
merged 30 commits into from Sep 28, 2023
Merged

Add-on store: Handle bulk install #15350

merged 30 commits into from Sep 28, 2023

Conversation

seanbudd
Copy link
Member

@seanbudd seanbudd commented Aug 31, 2023

Link to issue number:

None

Summary of the issue:

To be able to stress test the add-on installation system, a method of bulk installing add-ons is useful.
There is also a general request from users to be able to perform bulk actions like installing, updating or removing add-ons.

Description of user facing changes

Users are now able to bulk install add-ons.
This can be done by selecting multiple add-ons in the available add-ons tab, then activating the context menu on the selection and choosing the install action.

Description of development approach

Create a separate context menu for bulk actions. This creates an API for future bulk actions.

Testing strategy:

Manually test STR in #15347

Test selecting multiple add-ons and performing bulk install

Known issues with pull request:

None

Change log entries:

New features

  • The Add-on Store now supports installing add-ons in bulk by selecting multiple add-ons.

Code Review Checklist:

  • Pull Request description:
    • description is up to date
    • change log entries
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • API is compatible with existing add-ons.
  • Documentation:
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • Security precautions taken.

@seanbudd seanbudd requested review from a team as code owners August 31, 2023 04:47
@seanbudd seanbudd marked this pull request as draft August 31, 2023 05:24
@AppVeyorBot
Copy link

See test results for failed build of commit ac2866d2c4

@seanbudd seanbudd marked this pull request as ready for review August 31, 2023 06:05
@seanbudd seanbudd marked this pull request as draft August 31, 2023 07:11
@CyrilleB79
Copy link
Collaborator

CyrilleB79 commented Aug 31, 2023

@seanbudd, it's a very good news that you implement a way to perform actions on multiple add-ons.

Here are feedbacks from my first tests (commit e1a5f88):

  1. When no add-on is selected in the list (control+space), calling context menu does nothing (as well as when the list is empty). Since you have added "No actions available for the selected add-ons" message when many add-ons are selected, it would be coherent to have a ui.message too in this case.
  2. When 3 add-ons are selected, always the first add-on's description (and details) appears in the right panel. It would make sense to indicate "3 add-ons selected" and let the description and other details empty. Or maybe put the list of the add-ons in one of these two fields? (not fully convinced)
  3. When more than one add-on are selected, the menu in the "Actions" button is still the one corresponding to the first selected add-on. It should be the same as the context menu in the list. If there is no action in the context menu, the button should be greyed out.
  4. When you have installed many add-ons that are in pending install state, you still can access the context menu to install them again. You should have the context menu item allowing to install various add-ons only if at least one add-on in the selection can be installed, i.e. would have the "Install" item in its context menu if selected alone.
  5. With commit e1a5f88, if I select and install all the add-ons of the add-on store and control+tab immediately, I still have a lot of errors in the log.
  6. From the description, I can see that you will allow other actions on multipe add-ons. Will you do this in this PR or not? It would make sense to add uninstall and migrate in the current PR (or at least in the same release as this PR). In a second time,enable, disable and enable overriding compatibility would also be very useful (requested in In the add-on manager, allow to enable or disable multiple add-ons in one time #11317).

I can see that this PR is now in Draft again, so feel free to ignore my comment if they do not apply anymore.

@cary-rowen
Copy link
Contributor

cary-rowen commented Aug 31, 2023

Hi @seanbudd

Here is an important question for Chinese users:

As we all know, in a list control that supports multiple selections, users can hold down the Ctrl key and then press the arrow keys to locate the item they wish to select. After finding the item, they can press the space key. This process is valuable for selecting multiple non-adjacent items.

However, there are certain differences in the Chinese version of Windows: the Ctrl+Space shortcut is utilized to globally switch input languages.

Consequently, Chinese users encounter a need for an alternative method when attempting to select multiple non-adjacent items in the list.

The solution provided by Microsoft is to maintain the Ctrl key pressed while searching for the desired item within the list. After locating the item, if the user wishes to select it, they need to momentarily release the Ctrl key and then press the space key to select the current item. If the user intends to continue selecting other items, they must keep the Ctrl key held down and repeat the aforementioned steps.

The existing issue is that the action button activates when I press the space key by itself. While I can select adjacent items by pressing Shift along with the arrow keys, I am unable to select non-adjacent items.

By the way, the interaction of pressing the space key to activate an action button when a list item is focused is somewhat perplexing, and I rarely encounter it.

Cheers
Cary

@CyrilleB79
Copy link
Collaborator

Hi @seanbudd

Here is an important question for Chinese users:

As we all know, in a list control that supports multiple selections, users can hold down the Ctrl key and then press the arrow keys to locate the item they wish to select. After finding the item, they can press the space key. This process is valuable for selecting multiple non-adjacent items.

However, there are certain differences in the Chinese version of Windows: the Ctrl+Space shortcut is utilized to globally switch input languages.

Consequently, Chinese users encounter a need for an alternative method when attempting to select multiple non-adjacent items in the list.

The solution provided by Microsoft is to maintain the Ctrl key pressed while searching for the desired item within the list. After locating the item, if the user wishes to select it, they need to momentarily release the Ctrl key and then press the space key to select the current item. If the user intends to continue selecting other items, they must keep the Ctrl key held down and repeat the aforementioned steps.

The existing issue is that the action button activates when I press the space key by itself. While I can select adjacent items by pressing Shift along with the arrow keys, I am unable to select non-adjacent items.

By the way, the interaction of pressing the space key to activate an action button when a list item is focused is somewhat perplexing, and I rarely encounter it.

Cheers Cary

Note that pressing space on an unselected item in a list (Windows Explorer) also selects it, even on non-Chinese Windows (tested on French). Thus @cary-rowen's suggestion to remove space shortcut key in the add-on list fully makes sense. Enter or Appplication key (or shift+F10) will still allow to open the context menu of an add-on.

@cary-rowen out of curiosity, how do you unselect an item when performing a multi-selection in list on a Chinese system? E.g. when you want to select all items (control+A) and unselect only one in the middle of them?

@cary-rowen
Copy link
Contributor

Hi @CyrilleB79

Thanks for letting me know that this issue is also affecting French users.

@cary-rowen out of curiosity, how do you unselect an item when performing a multi-selection in list on a Chinese system? E.g. when you want to select all items (control+A) and unselect only one in the middle of them?

In this case my steps are as follows:

  1. Press Ctrl+A;
  2. Hold down the CTRL key and press the arrow keys to navigate to an item I don't want selected.
  3. Press Ctrl+Alt+Space NVDA will report "Not selected".
  4. However, in Windows Explorer, an easier way might be to use invert selection LOL but this doesn't seem to have a shortcut key.
    BTW, starting from Windows 10, the default shortcut key for Microsoft Pinyin input method interferes with the shortcut key I mentioned in step (3), because the shortcut key is also used to switch Chinese or English input.

Cheers
Cary

@CyrilleB79
Copy link
Collaborator

Hi @CyrilleB79

Thanks for letting me know that this issue is also affecting French users.

No, this issue does not affect French users. I just wanted to say that the way Chinese people perform selection in list is not a specificity of Chinese systems and is doable on other systems.

@seanbudd seanbudd added this to the 2023.3 milestone Sep 7, 2023
@seanbudd seanbudd marked this pull request as draft September 8, 2023 05:48
@seanbudd seanbudd marked this pull request as ready for review September 8, 2023 06:01
@AppVeyorBot
Copy link

See test results for failed build of commit 603f9a50f8

@seanbudd seanbudd marked this pull request as draft September 8, 2023 07:25
@seanbudd seanbudd marked this pull request as ready for review September 11, 2023 03:20
@AppVeyorBot
Copy link

See test results for failed build of commit a52ebe9971

@seanbudd seanbudd modified the milestones: 2023.3, 2024.1 Sep 12, 2023
@seanbudd seanbudd changed the base branch from beta to master September 12, 2023 03:54
@seanbudd seanbudd marked this pull request as draft September 12, 2023 06:58
@seanbudd seanbudd removed the blocked label Sep 12, 2023
@AppVeyorBot
Copy link

See test results for failed build of commit 68bf9223f2

@seanbudd seanbudd marked this pull request as ready for review September 14, 2023 04:55
Copy link
Member

@Qchristensen Qchristensen left a comment

Choose a reason for hiding this comment

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

Looks good.

@seanbudd seanbudd merged commit a57faa1 into master Sep 28, 2023
1 check was pending
@seanbudd seanbudd deleted the allowBulkInstall branch September 28, 2023 00:24
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.

None yet

6 participants