-
-
Notifications
You must be signed in to change notification settings - Fork 646
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 #13985
Add-on store #13985
Conversation
Initial design for addon store. - A new dialog ("add-on store") can be launched from the 'tools' menu. - Data is pulled from the live Add-on API and shown in the Dialog. - The dialog presents a filterable sortable list with basic add-on data. - An add-on can be selected and its description / further details are presented. - When filtering, care is taken to preserve the selection. Known issues: - The current version of NVDA (alpha 2022.2) is not known to the add-on store yet. - Fetching add-on listings for it produces a 400 error. This could be resolved with Allow 'dev' version of add-on addon-datastore#40 - The first time the dialog is opened, there is a noticeable delay, the dialog creation is blocked by downloading the add-on data. This is then cached in memory, there is no way to refresh the data, NVDA must be restarted. Will be resolved in a future commit. - The list currently shows all add-ons. There is no way to select between stable / beta / all. Planned for resolution. - Due to inheriting from SettingsDialog (for other functionality, such as single instance etc) there are ok / cancel buttons. A single button "done" may be more appropriate.
- In order to be able to test the NVDA add-on store implementation locally, developers must be able to provide test data for the store. - The add-on store is fetching data from the server every time it is opened, this may overload the server. - There was a delay when opening the add-on store Description of change: Introduce a (json) file cache, which includes: - the string (data) returned by the API - the datetime when the data was fetched (stored as an ISO date time string) With this cache file: - Testing can be achieved by overriding the cached data. - Data is cached every 6 hours. - The cache can be used (almost) instantly. Testing strategy: - Ran NVDA without an internet connection. Led to ConnectionError. - Ran NVDA with a version that returns no data. Cache is used instead. - Ensured that when the cache has a time stamp less than 6 hour old there is no delay to use it. - Ensured that the cache can be modified locally, the change is reflected in the GUI. - When the cache is older than 6 hours, there is a short delay before data is populated into the GUI. Known issues: - The cache does not include the NVDA version that was used when fetching. This may mean that an invalid cache could be used for up to 6 hours after updating NVDA. - When the cache is older than 6 hours, there is a short delay before data is populated into the GUI.
- The add-on store needs a way to allow installation. - The actions should happen in the background, steps like downloading may take significant time. - Progress should be discoverable. Description of change: - Introduces a way to associate actions with the GUI. - To cater to power users a context menu has been added to list items (available add-ons) - To ensure discoverability, buttons for actions are added to the details. Note There is currently an "update" action, which only logs. It is included just as a demonstration / proof of concept. There are many rough edges in this implementation, but since this is already a sizable change, and many further changes are more likely to self contained it is a good checkpoint. Testing strategy: - Used the links in the add-on store data to download several *.nvda-addon files - The manifests were modified (in various ways) to create different installation outcomes. - The python "simpleHttpServer" module was used to srt. - The links to the URL for add-ons in the cached addon-store file were pointed to localhost. - Used 'Clumsy' tool to simulate slow connections, adding latency to the download process. Known issues: These will be worked on separately: - The status of installed add-ons isn't reflected in the list, installed add-ons with a matching version should be omitte". There is a problem here, an add-on manifest does not include a numerical version, only a version name string. Ordering based on version number with the add-on store will not be possible for side-loaded add-ons. - Pressing enter on an addon should open the context menu. - Pressing enter on a link (in the details) should open the URL - When closing the dialog (after inste blocked by add-ons being installed (consider NVDA exit / os shutdown), and progress shown. - Task management required for download / install steps. Install steps for multiple add-ons should be serialized. Download can happen in parallel. - Ok / Cancel should be eplaced wih a 'Done' button - Add-ons optionally have an install_task which runs post installation, prior to restart. This can be disruptive when installing multiple add-ons from the store. Consider making this occur post restart.
- When an addon was already installed (matching ID and version) it should not have been listed as available (to download) - When the addon was installed, but there is a newer version available, it should not have been listed as "update available". - When the addon isn't compatible, the status column should say so. This shouldn't occur if the add-on submission process works correctly, however, to catch bugs with that system it is worth showing. - Adds typing and deeper docs to the addonHandler and addonVersioning. This needs to be understood for the version / name / id comparisons with addonStore data. - Set status of add-ons to "installed" / "enabled" / "disabled" / "update available" / "incompatible" - Don't show add-ons that are "enabled" (I.E. Installed and running). - Don't show add-ons that are "disabled" - Check for compatibility, show incompatible as status. - Used the links in the add-on store data to download several *.nvda-addon files - The manifests were modified (in various ways) to create different installation outcomes. - The python "simpleHttpServer" module was used to serve the add-ons. - The links to the URL for add-ons in the cached addon-store file were pointed to localhost. - Used 'Clumsy' tool to simulate slow connections, adding latency to the download process. - Modified the cached addon-store file to test different add-on versions being available (to test updates available) - Installed an add-on to ensure that they aren't listed when "running". - Disabled add-on to ensure that they aren't listed when "disabled". - Modified the addon-store file to test showing addon compatibility in status. - Does not properly handle add-on updates (from the store). The numerical version information from the add-on needs to be stored somewhere. Information for side loaded add-ons will be shown as "update available" if the version doesn't match, even if the add-on store version is older.
Installing add-ons must happen on the main thread. They must be installed one by one. Downloads may happen concurrently, moving off the main thread allows NVDA to stay responsive. Description of change: Use a ThreadPoolExecutor to enable downloading in the background. This will allow scaling up the number of threads as required. Provided an abstraction to make cancellation/tracking of complete simpler. Addons are installed (if download has completed) when the user tries to exit the dialog.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
…14955) Part of #13985 Originally raised by @XLTechie in #14912 (reply in thread) Summary of the issue: The installer for add-on store versions of NVDA, is creating the addonStore folder in %appdata%\nvda, even before the license agreement is accepted! E.g. If starting the installer, and selecting "exit" instead of agreeing to the license agreement, the addonStore directory is created and populated with some files and directories under %appdata%\nvda. It won't create %appdata%\nvda, but if that already exists, it will create the store directory. This is absolutely not expected behavior. NVDA should never be doing any directory and file creation on the local system before "install" is selected. I have not looked at the code, so do not know what this would do to an existing installed store copy, but really no user would expect that the installed NVDA's configuration could be altered by an installer that was not told to "install". Description of user facing changes Ensure the caching behaviour of add-ons matches that of NVDA's config. Files should only be written to disk when not running as launcher nor in a secure context. Description of development approach Adds relevant checks to prevent saving the add-on handler state and add-on store caches to disk.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just two small changes to the user guide. But otherwise I'm good.
user_docs/en/userGuide.t2t
Outdated
This button opens the [NVDA Add-ons page https://addons.nvda-project.org/]. | ||
If NVDA is installed and running on your system, you can open the add-on directly from the browser to begin the installation process as described below. | ||
Otherwise, save the add-on package and follow the instructions below. | ||
You can sort add-ons by their various fields by pressing ``ctrl+1,2,3...``, where the number is the column index you wish to sort by. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was originally very confused with this feature. As I would have expected that ctrl+1 sorted by the first column (I.e. the add-on's name) however I think ctrl+1 matches version? And ctrl+0 sorts by name? I also agree that this feature could be better if the sorting was exposed somehow. My feeling is that these shortcuts should be removed from the user guide before this is merged. And consider perhaps before 2023.2 adding another combo box for sorting.
See test results for failed build of commit 3098ecf63a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. In the user guide section about incompatible add-ons, it mentions that "If the incompatible add-on causes issues, you can disable or remove it." Is it worth mentioning here or linking to the ways you can start NVDA with add-ons disabled? (NVDA+q, down arrow to "restart with add-ons disabled", or the --disable-addons command line option)?
Quentin wrote:
I think this is a good idea for completeness. |
Co-authored-by: Noelia Ruiz Martínez <nrm1977@gmail.com>
Now that #13985 has been merged, please open new issues for add-on store issues, as you would for any other issue on alpha. |
Part of #13985 Summary of the issue: After reading the proposed add-on store user guide sections, I thought they could be more user friendly. The current form appeared to assume more knowledge than the user may have. Description of user facing changes The following summarizes the changes I made to each section. Add-on Store: Rewrote introductory section to better explain what add-ons are. Explained that it being a "store" does not mean users must pay. Browsing add-ons: Better explained the use case for alt+l. Explained what "selecting an add-on" means. Elaborated a bit more on actions. Add-on list views: Changed the sentence order to be more logical. Noted the possibility of view changing via arrow keys. Include incompatible add-ons: This heading was duplicated. Changed the first one to "Filtering for enabled or disabled add-ons". Filtering for enabled or disabled add-ons: Wrote a substantive introductory section, explaining enabled/disabled states. Include incompatible add-ons: Minor grammar. Filter add-ons by channel: Slightly more explanation for beta add-ons. Moved explanations before usage instructions. Search add-ons: Changed title to "Searching for add-ons". Added instructions on how to reach the search field. Removed the line "Add-ons can be filtered by display name, publisher and description.", because there are no instructions on how to do that. Expect to add back in later, after that part can be more easily explained. Add-on actions: Made the instructions more complete. Installing add-ons: Added an NV Access disclaimer at the top. I Used the word "vetted", because that has been used to describe NV Access accepting add-ons into the store, by some members in the community, and I wanted to make clear that they are not "vetted" in any way by NV Access. Slight clarifications. Removed a rogue space at the end of a line. Removing Add-ons: Minor word change to avoid proximate word repetition. Disabling and Enabling Add-ons: Changed "a status" to "the status", since the cases are specific. Incompatible Add-ons: Suggested grammar changes for better flow. Added potential reasons why a user may want to disable all add-ons.
…le (#107) Another issue found on the .pot generation Issue Some strings of the add-on store GUI are not provided to translators as reported in this message. Cause When generating the .pot file, svn2nvda looks for .py files only in NVDA's source folder and maximum two levels below. However, after merging of nvaccess/nvda#13985, the GUI files of the add-on store are three levels below, thus they are not taken as source when generating the .pot file. Solution Added one more level for the sources used by xgettext to generate the .pot file.
#13985 introduced new restrictions for temporary copies of NVDA, preventing the add-on store from opening and preventing writing to disk. These should be explained in the user guide Description of user facing changes N/A Description of development approach Simplified some of the structure in the user guide, including explaining what a temporary copy is.
This is intended to act as a base for "stacked pull requests" to build up this feature until it is considered ready to merge into NVDA.
Link to issue number:
None
Summary of the issue:
Historically, NVDA users have had to browse websites or use add-ons, not directly run/developed by NV Access, in order to learn about, download, and install add-ons. For a long time, easier manual updates for add-ons, and automatic updates for add-ons has been desired by NVDA users. Additionally, with the more common and structured approach to add-on API compatibility breaks, lowering the friction for delivery of updated (and compatible) add-ons is more important than ever.
User stories enabled by this work:
Additionally, this work unblocks the following user stories which may yet be incorporated into this PR:
Description of changes for the user:
A new dialog is introduced (NVDA menu, tools, addon store) which shows the latest versions of add-ons (submitted to the add-on store).
The list of functionality for the add-on store is expansive.
This should be covered in the user guide and manual test plan
Images of the add-on store dialog:
Testing strategy:
The manual test plan exists to cover common user paths and a general smoke test of the add-on store interface:
https://github.com/nvaccess/nvda/blob/addonStore-base/tests/manual/addonStore.md
Unit tests include testing a new data-structure: CaseInsensitiveSet, intended to manage tracking add-on IDs, which should be done in a case insensitive manner.
Known issues with pull request:
There are many known issues with this PR.
They should be documented as issues against NVDA, or unresolved comments in #14912
Change log entries:
New features
Deprecations:
Code Review Checklist: