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

Updated user guide with missing key commands and details to log file and app modules #15460

Merged
merged 31 commits into from Oct 16, 2023

Conversation

Adriani90
Copy link
Collaborator

@Adriani90 Adriani90 commented Sep 17, 2023

Link to issue number:

fixes #14410
fixes #1506
fixes #5261

Summary of the issue:

Some commands wore not documented in the user guide although they are useful for users as well.

Description of user facing changes

Added documentation for ctrl+nvda+f1 and ctrl+nvda+shift+f1 in the coresponding sections.

Testing strategy:

Checked that key commands reference is updated as well.

Known issues with pull request:

None

Change log entries:

Updates to the user guide (#14410, #1506)

@Adriani90 Adriani90 requested a review from a team as a code owner September 17, 2023 08:27
…istent with other sections in the user guide
@AppVeyorBot
Copy link

See test results for failed build of commit df7eadaf10

user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
Adriani90 and others added 4 commits September 17, 2023 18:33
Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>
Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>
@Adriani90
Copy link
Collaborator Author

I see some failed tests related to symbol pronounciation which aparently led to failed system tests. I didn't touch those files actually. What should I do?

@CyrilleB79
Copy link
Collaborator

Thanks for the changes.

Actually, there are still things to polish. And to make the "Log viewer" paragraph more understandable, it should be reorganized and rephrased. I need to think at it and I will make a full proposal later.

Also one difference to keep in mind: the menu command NVDA -> Tools -> Log viewer only shows the log, whereas NVDA+control+F1 show the log and the nav object's properties.

@Adriani90
Copy link
Collaborator Author

Thanks @CyrilleB79 that's very appreciated. We have a lot of educated users who are interested in more detail about this, but on the other hand they are not as skilled as a developer to go through the developers guide. So my motivation for this PR was giving some details to such users via the user guide.

Hmm it seems a ghost made the checks to pass :-) creepy github.

user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
@seanbudd
Copy link
Member

For reference, the user guide standards document may be helpful: https://github.com/nvaccess/nvda/blob/master/projectDocs/dev/userGuideStandards.md

@seanbudd seanbudd marked this pull request as draft September 20, 2023 04:12
Adriani90 and others added 2 commits September 20, 2023 13:58
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
@AppVeyorBot
Copy link

See test results for failed build of commit e51716c8d7

Adriani90 and others added 4 commits September 20, 2023 21:03
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
@Adriani90
Copy link
Collaborator Author

@seanbudd thanks for the review, very appreciated.
By the way the link for the documentation system guidelines for reference materials (https://documentation.divio.com/reference/) gives a 404 error on my end.
This link is referenced in the user guide standards document.

user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
@@ -2785,10 +2785,19 @@ You can browse installed incompatible add-ons using the [incompatible add-ons ta

Copy link
Collaborator

Choose a reason for hiding this comment

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

This paragraph is very clear and well phrased. Thanks @Adriani90 and @seanbudd. Do not modify anymore!

user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
@seanbudd
Copy link
Member

Ah, the link should be https://documentation.divio.com/reference.html. I'll fix this later

user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
Adriani90 and others added 2 commits September 21, 2023 09:59
Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>
@Adriani90 Adriani90 marked this pull request as ready for review September 21, 2023 22:54
@AppVeyorBot
Copy link

See test results for failed build of commit 7f257f4b90

user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
Copy link
Contributor

@XLTechie XLTechie left a comment

Choose a reason for hiding this comment

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

Better, but a few more comments for your consideration.

user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
user_docs/en/userGuide.t2t Show resolved Hide resolved
user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
Adriani90 and others added 5 commits September 22, 2023 03:04
Co-authored-by: Luke Davis <8139760+XLTechie@users.noreply.github.com>
Co-authored-by: Luke Davis <8139760+XLTechie@users.noreply.github.com>
Co-authored-by: Luke Davis <8139760+XLTechie@users.noreply.github.com>
Co-authored-by: Luke Davis <8139760+XLTechie@users.noreply.github.com>
@AppVeyorBot
Copy link

See test results for failed build of commit 2cbbf06473

@seanbudd seanbudd added this to the 2024.1 milestone Sep 22, 2023
user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
@Adriani90
Copy link
Collaborator Author

some may be confused, and expect it as a submenu under NVDA Menu, tools, logs, ?

I'd say this might happen but not because of the sentence as it stands. Actually we say already this is the log sub menu in the log viewer so it should be clear enough that the log viewer should be opened first and then you can access the log sub menu.
Alternatively we could rename that sub menu to "file" instead of "log". Is this something which should be considered?

@CyrilleB79
Copy link
Collaborator

Alternatively we could rename that sub menu to "file" instead of "log". Is this something which should be considered?

No "Log" is fine. "Refresh" applies to the log visible in the log viewer, but the log file is not refreshed.

@cary-rowen
Copy link
Contributor

Please take @seanbudd's advice and @CyrilleB79's clear and precise explanation, I think it's pretty clear.

Co-authored-by: Sean Budd <seanbudd123@gmail.com>
@Adriani90
Copy link
Collaborator Author

After giving it a thought I agree with menu instead of submenu since this is more common in the day to day language when talking about such areas of menu toolbars. So I agree with Sean and Cyrille.

Copy link
Member

@seanbudd seanbudd left a comment

Choose a reason for hiding this comment

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

Thanks @Adriani90

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.

All looks good. My only comment before approving is:

As these are the only mentions of "reloading plugins" and "app modules", is it worth a sentence explaining what they are?

@seanbudd seanbudd marked this pull request as draft September 28, 2023 00:00
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
@Adriani90 Adriani90 marked this pull request as ready for review October 12, 2023 12:22
@CyrilleB79
Copy link
Collaborator

@Adriani90, seems that this PR also fixes #5261. Is it correct?

@Adriani90
Copy link
Collaborator Author

seems that this PR also fixes #5261. Is it correct?

that issue talks also about.a command for the developer info, so this should be the info of the focused element which is displayed below the carret when opening the log file.
I am not aware for a coresponding command for reporting or displaying the developer info only.
If there is a hidden command, I am more than happy to add it to the user guide.

@CyrilleB79
Copy link
Collaborator

seems that this PR also fixes #5261. Is it correct?

that issue talks also about.a command for the developer info, so this should be the info of the focused element which is displayed below the carret when opening the log file. I am not aware for a coresponding command for reporting or displaying the developer info only. If there is a hidden command, I am more than happy to add it to the user guide.

No, there is only one command that reports Developers info, it's NVDA+F1. And it operates on the nav object, not the focus.

#5261 is not very precise about this command but it can be no other command than NVDA+F1. Thus, it's fixed with the current PR.

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.

Userguide looks good

@seanbudd seanbudd merged commit 041477f into nvaccess:master Oct 16, 2023
1 check was pending
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants