nvda
nvda copied to clipboard
nvaccess
Add the possibility to open its documentation from an add-on's installation dialog
Link to issue number:
Fixes #13454
Summary of the issue:
Feature request: before installing it, the user want to be able to check its documentation.
Description of user facing changes
- A new button allowing to open the documentation is added in the dialog box asking to confirm the installation of an add-on.
- Also the "About add-on" button label has been shortened to "About"
Description of development approach
- Extract the documentation from the add-on to a file located in %tmp%\nvdaDocs; the name of the file is a randomly name randomly generated by python's tempfile so that two different files can be opened simultaneously.
- %tmp%\nvdaDocs is cleaned only when NVDA is closed; this way, when the add-on doc file is opened in the browser, pressing F5 allows to reload the page (unless NVDA has been closed/restarted in the meantime).
- The opened documentation is the one in NVDA's language or in English by default.
- If no doc is found (neither in the current language nor in English), the button is greyed out.
Testing strategy:
Various manual tests:
- With an add-on containing a doc in the current language: pressing the doc button opens documentation containing no-ASCII characters
- With an add-on in English only (no translation): pressing the button opens the English doc
- With an add-on with no documentation: check that the button is greyed out
Known issues with pull request:
Open questions for reviewers:
- Should I also add the doc button in the error dialogs appearing when the add-on is too old for current NVDA version or when NVDA version is too old for this add-on? For now I have not done it, but this could be added easily.
- Is there a better strategy regarding temporary files persistence/cleaning? Just putting the files in %tmp% without cleaning seemd a bit exaggerated to me.
Change log entries:
New features
When installing an add-on, you can now read its documentation before installing it. (#13454)
Code Review Checklist:
- [x] Pull Request description:
- description is up to date
- change log entries
- [x] Testing:
- Unit tests
- System (end to end) tests
- Manual testing
- [x] API is compatible with existing add-ons.
- [x] Documentation:
- User Documentation
- Developer / Technical Documentation
- Context sensitive help for GUI changes
- [x] UX of all users considered:
- Speech
- Braille
- Low Vision
- Different web browsers
- Localization in other languages / culture than English
- [x] Security precautions taken.
Hi Cyrille:
< 1. Should I also add the doc button in the error dialogs appearing when the add-on is too old for current NVDA version or when NVDA version is too old for this add-on? For now I have not done it,
I think it should be added, in the same way that the About button is added even when add-ons are incompatible, for consistency, so that documentation can be reviewed and people is encouraged to update NVDA or contact add-on authors to provide a new version of the add-on if needed.
- Is there a better strategy regarding temporary files persistence/cleaning? Just putting the files in %tmp% without cleaning seemd a bit exaggerated to me.
I think it's a good idea to clean folders when NVDA is closed, but imo maybe better to add a different folder for each add-on, so that, if some day NVDA includes the possibility of opening several installation dialogs or in other cases, different documentation files can be opened at the same time for comparison. For instance, maybe different add-ons with similar purposes and users may want to compare the documentation of them to decide which they want to install.
I would agree with both of Noelia's comments. Always better to design for the future, for possibilities that we may not now consider likely, if doing so does not add a tun of extra work.
Thanks @nvdaes and @XLTechie for your comments.
- Should I also add the doc button in the error dialogs appearing when the add-on is too old for current NVDA version or when NVDA version is too old for this add-on? For now I have not done it,
I think it should be added, in the same way that the About button is added even when add-ons are incompatible, for consistency, so that documentation can be reviewed and people is encouraged to update NVDA or contact add-on authors to provide a new version of the add-on if needed.
This makes sense. I will do it.
- Is there a better strategy regarding temporary files persistence/cleaning? Just putting the files in %tmp% without cleaning seemd a bit exaggerated to me.
I think it's a good idea to clean folders when NVDA is closed, but imo maybe better to add a different folder for each add-on, so that, if some day NVDA includes the possibility of opening several installation dialogs or in other cases, different documentation files can be opened at the same time for comparison. For instance, maybe different add-ons with similar purposes and users may want to compare the documentation of them to decide which they want to install.
With the current version of the code, it's already possible to open two documentation of two different add-ons. You just need to press documentation in the first install dialog before opening the second one. There is no collision since the name of the temp file is randomly generated, e.g. the URL in the address bar of the browser is file:///C:/Users/Cyrille/AppData/Local/Temp/nvdaDoc/tmpn6dh7h03.html.
I will update the initial description to clarify this point.
You just need to press documentation in the first install dialog before opening the second one. There is no collision since the name of the temp file is randomly generated, e.g.
Great, thanks.
I have put the "Documentation" button also in error dialogs (see lien aacc100).
Any comment regarding the order of the buttons? For now they are as folllows:
- In confirm dialog: About, Documentation, Yes, No; with About focused first
- In error dialogs: About, Documentation; with About focused first
But actually, I am thinking on how to rewrite this code in case the temp "nvdaDoc" directory creation fails for another reason than "already exists" and have not a clear idea on what should be done in this case.
What about trying to create the directory in the initialize function, and register in the NVDA's log if an exception is raised. In this case, the button to open the documentation would be unavailable for the current session of NVDA.
But actually, I am thinking on how to rewrite this code in case the temp "nvdaDoc" directory creation fails for another reason than "already exists" and have not a clear idea on what should be done in this case.
What about trying to create the directory in the initialize function, and register in the NVDA's log if an exception is raised. In this case, the button to open the documentation would be unavailable for the current session of NVDA.
What you call the initialize function (actually getTempDocDir) is run each time you press the button, not only at NVDA startup as it is done for the config folders. I need to check each time that the folder exists (and recreate it if needed), because the user may clear the temp folder content during an NVDA session. I do not find acceptable to disable the doc button until next NVDA restart.
I do not find acceptable to disable the doc button until next NVDA restart.
I agree with you after your explanation. Then I think that you should show an error message dialog.
Could this target the new add-on store in https://github.com/nvaccess/nvda/pull/13985 instead?
Could this target the new add-on store in #13985 instead?
I will look at it if you wish.
However, is there an ETA for the add-on store? It has been announced for years and has not yet been released. With Reef leaving NVAccess, I wonder when this feature will become a reality (i.e. be merged in alpha). If it is not expected in 2023, I would prefer to have this PR already merged in current NVDA's codebase. Just my opinion though.
Work on the add-on store has resumed. Unless there are further unexpected delays (like a slew of security issues) it should make it into 2023.
I think this might be better left as blocked until the new add-on store is merged to master.
I think this might be better left as blocked until the new add-on store is merged to master.
OK.
@CyrilleB79 hi, i want to request something from you if possible. I don't want you to update this to the current codebace or anything because i don't wish to try this with the last addon store, please just do something so that the ap veyor completes the build again, i want to try the snappshot of this pr. Is it possible for you to do this? because ap veyor doesn't have the build of this, as it is deleted after a month. Thanks
@amirmahdifard, I have tried to push an empty commit to re-trigger a build. Unfortunately, appVeyor does not rebuild, maybe due to merge conflicts (even if I have not merged from master). You'll need to build yourself, sorry.
@CyrilleB79 i have an idea. please, instead of empty commit, for example, update a line of text in the documemtations, or something, so it will build may be. If that also doesn't work, i won't annoy you anymore. Thanks.
i sadly don't know how to build nvda, and that's my only reason why i don't contribute to nvda's code, but i contribute to addons. I know that asking you to build for me is a bit rood, so when ap veyor build fales this time, i will just give up
The empty commit is not the cause; I have already used them to retrigger builds.
If you want to learn how to build NVDA, you can read the documentation to create your development environment or more generally the contributing guide. And if you have questions, subscribe to nvda-devel mailing list and ask there.
@CyrilleB79 ok, Thanks for your try. Sadly, i don't see my self advance enough to be able to build nvda. But if i could, i would implement a lot of cool things for nvda, as i know how to write code but i don't know how to build nvda. Cn you tell me how can i setup ap veyor on one of my github repos? do i need to modify my code? or how the ap veyor can build? Thanks, and i know here is not a right place to talk about this probably
@CyrilleB79 ok, Thanks for your try. Sadly, i don't see my self advance enough to be able to build nvda. But if i could, i would implement a lot of cool things for nvda, as i know how to write code but i don't know how to build nvda. Cn you tell me how can i setup ap veyor on one of my github repos? do i need to modify my code? or how the ap veyor can build? Thanks, and i know here is not a right place to talk about this probably
You don't need to use AppVeyor to build NVDA, the link above already gives the process of building NVDA locally. Unless your local computer is not readily available.
I'm sorry I don't have experience building projects with AppVeyor, you'll need to check the AppVeyor documentation. I haven't found a personal process for building NVDA using AppVeyor in the repository, and NVDA has some of its own AppVeyor scripts, not sure it works out of the box.
Yes there needs to be a clean merge to the latest master for AppVeyor to build, as any PR is tested with the code merged into the target branch. I'd ask that any discussion around building NVDA be moved to a separate GitHub discussion outside of this PR.
This PR pre-dates the add-on store and and fixing merge conflicts is not trivial. I had already tried twice without success. Moreover, with the add-on store, this PR would become much less useful in any case. I have not the time nor any strong motivation to push it forward.
Let's close it for now. If someone else or myself want to work on this, this PR could serve as a source of inspiration but it would probably be better to restart from scratch rather than performing an upmerge.