-
-
Notifications
You must be signed in to change notification settings - Fork 637
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 an "on-demand" speech mode #15804
Conversation
See test results for failed build of commit 15f0b897a9 |
Great work on this! Really highly appreciated. Does include also application specific commands like changing formating in Ms Office etc.? These should be reported as well in my view. |
In my view say all could be implemented on a later stage, if the users demand it. It should not prevent this PR from being merged. |
Could the speech modes be made opt out, at least the beeps and on demant modes? That would make the no beeps mode add-on obsolete, but I think that would be the best solution. |
Thanks @LeonarddeR and @Adriani90 Answering to all:
If you are speaking about Office commands such as toggle bold (control+U), toggle italic (control+I), etc., they are not reported in "on demand" mode. Indeed these scripts perform an action. In the "on demand" mode however, only the script whose goal is to report an information are speaking. The scripts that perform an action indeed do not speak. The use case is the following. You are in a meeting so you do not want NVDA to speak when you write notes in Word. Thus NVDA does not speak when you type or when you change formatting (bold, etc.) so that you can perform these actions without being bothered by NVDA speech. If you explicitly want to know font formatting, you still can request it with NVDA+F. The same way, the time command (NVDA+F12) speaks because you have no reason to press this command without any speech feedback.
For the same reason as before, they are not included, because you may want to toggle a parameter without being bothered by a speech feedback.
I do not contest the usefulness of the beep mode for some users. I was just worrying of the increasing number of speech mode, when the vast majority of users will use only two of them: normal speech and either off or "on demand".
OK thanks for the positive feedback.
Agree with this plan if NV Access also agrees since this PR is already an improvement. Still, I will try to investigate the say all issue in the next days.
In the specific case of Windows Magnifier add-on, all the commands speak after an associated action, e.g. move something, change a setting, etc. Thus they should be muted in the "on demand" mode. More generally however, the add-on authors may use the "speakOnDemand" parameter of the script decorator to indicate if their scripts should speak when speech mode is "on demand".
The use or not of NVDA key is and should not be a criterion since keys could be reassigned. More generally, you seem to have an idea of something more verbose than I regarding the "speech on demand" mode. The mode as I have implemented it seems to match more or less the one in Jaws; thus, I imagine (since it is not so new), that the use case has been confirmed among users. If there is a need though, we may imagine different levels of verbosity for the "on demand" mode. To define a different verbosity, please provide a concrete use case as well as a precise criterion to determine if a given command (script) should speak or not.
Technically, all can be done. I am asking from a UX point of view. Is it better to add a new GUI control so that people can choose which speech mode is available in NVDA+S command? Or is it just cluttering the GUI with a parameter considered obscure for most users? In the latter case, the "no beep speech mode" add-on could be improved to provide this feature for those who need it (cc @ABuffEr). |
Thanks for this work @CyrilleB79.
I will review the specifics on Monday or Tuesday, depending on time.
I would like to say that I agree with @LeonarddeR. Some checkboxes in the speech
category to hide beeps mode, and to hide on-demand speech mode, both unchecked
by default.
Doing that, makes including these in speech modes to be reasonable. Which is
where I think they should be.
Given how many people desired the no beeps mode add-on over the years, making a
checkbox for this finally does seem warranted, though I personally love beeps
mode.
|
I have just added say all scripts and input help in the "on demand" mode and updated the dev doc. And I have updated the initial description according to these last changes and also included relevant information from our discussion. Regarding a configuration to filter the speech modes available in NVDA+S loop, I prefer to wait for NV Access review before implementing something. I am not totally convinced that it's the best way to handle the "too many speech modes" issue. |
Hi, |
I would personally go for a checkable list, similar to the NVDA modifier keys list. |
I have created #15806 to discuss configurability of speech modes switching. |
I think this might indeed make things to complicated. My use case was not really well developped yet in my mind, I think your approach suits better as it is. I guess if users need more commands to speak in on demand mode, then this can be implemented later on.
I like this idea as well. The less we have to tab through dialogs, the better. :-) However, there is a risk with these checkboxes: imagine a new user disables by accident the speech mode speak and presses nvda+s afterwards, NVDA would change to one of the less verbose or no verbose modes. There is no way of enabling the speak mode again unless you know the menu structure by hard or you use on demand mode to report the currently focused object with a command to include speech mode speak again into the nvda+s command but this does not feel intuitive. Another better alternative in my view would be to let both speech mode speak and speech mode off as they are, and include also a speech mode category in the synth settings ring which contains all 4 speech modes. Then, in case you enable speech mode on demand, beeps or off from the synth settings ring, pressing nvda+s switches to speech mode speak again. So speech mode speak has always priority when pressing nvda+s. |
Ofcourse my proposal would mean we have to enhance the synth settings ring to a speech and synth settings ring but this would improve things anyway. |
"on demand" |
In the end, I have added a paragraph in section 4 of the User Guide. Not in section 12 since section 12 describes the GUI (menus and dialogs) whereas this option is not configurable through dialogs. Since the description of each mode is now in the User Guide, I have shortened the input help which was way too long: I have removed the descriptions of each speech mode from input help. @XLTechie, I have taken into account your review on the input help to write the paragraph in the User Guide. |
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.
Thanks @CyrilleB79
user_docs/en/userGuide.t2t
Outdated
* Talk: NVDA will just speak normally. This is the default speech mode. | ||
* On-demand: NVDA will only speak when calling commands whose goal is to speak an information (e.g. report the title of the window); but it will not speak in reaction to actions such as moving the focus or the cursor. | ||
* Off: NVDA will not speak anything. | ||
* Beeps: NVDA will replace normal speech with quiet beeps. |
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 believe these lists also need a terminator. Is there a reason *
was used over -
?
* Beeps: NVDA will replace normal speech with quiet beeps. | |
* Beeps: NVDA will replace normal speech with quiet beeps. | |
* |
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.
Thanks for the formatting fix. No reason to use * instead of -. I'll change it.
user_docs/en/userGuide.t2t
Outdated
* Talk: NVDA will just speak normally. This is the default speech mode. | ||
* On-demand: NVDA will only speak when calling commands whose goal is to speak an information (e.g. report the title of the window); but it will not speak in reaction to actions such as moving the focus or the cursor. | ||
* Off: NVDA will not speak anything. | ||
* Beeps: NVDA will replace normal speech with quiet beeps. |
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 believe these lists also need a terminator. Is there a reason *
was used over -
?
* Beeps: NVDA will replace normal speech with quiet beeps. | |
* Beeps: NVDA will replace normal speech with quiet beeps. | |
* |
user_docs/en/userGuide.t2t
Outdated
* Off: NVDA will not speak anything. | ||
* Beeps: NVDA will replace normal speech with quiet beeps. | ||
|
||
A gesture allows to cycle through the various speech modes: |
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.
A gesture allows to cycle through the various speech modes: | |
A gesture allows cycling through the various speech modes: |
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'm pretty sure this command should no longer be called a toggle. You can't toggle between multiple options, not in the normal usage of that word (N.B. I am aware of three position toggle switches, but that's not how people normally understand it, I think).
source/globalCommands.py
Outdated
@@ -2008,18 +2023,15 @@ def script_review_currentSymbol(self,gesture): | |||
@script( | |||
description=_( | |||
# Translators: Input help mode message for toggle speech mode command. |
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.
Shouldn't this now be "cycle speech modes command", or "Switch to next speech mode command"? It is no longer a toggle.
# Translators: Input help mode message for toggle speech mode command. | |
# Translators: Input help mode message for switch to next speech mode command. |
source/globalCommands.py
Outdated
"When set to off NVDA will not speak anything. " | ||
"If beeps then NVDA will simply beep each time it its supposed to speak something. " | ||
"If talk then NVDA will just speak normally." | ||
"Toggles between the speech modes of off, beep, talk and on-demand." | ||
), | ||
category=SCRCAT_SPEECH, | ||
gesture="kb:NVDA+s" | ||
) | ||
def script_speechMode(self,gesture): |
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.
Can this be linted?
@@ -10,6 +10,8 @@ What's New in NVDA | |||
- Added support for Bluetooth Low Energy HID Braille displays. (#15470) | |||
- The Add-on Store now supports installing add-ons in bulk by selecting multiple add-ons. (#15350) | |||
- From the add-on store, it's possible to open a dedicated webpage to see or provide feedback about the selected add-on. (#15576, @nvdaes) | |||
- A new "on-demand" speech mode has been added. | |||
When speech is on-demand, NVDA does not speak automatically (e.g. when moving the cursor) but still speaks when calling commands whose goal is explicitly to report something (e.g. report window title). (#481, @CyrilleB79) |
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.
Should this be referencing the issue number or the PR number?
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 have put the issue number as it is usually done. If you see a good reason to use the PR number here instead, please let it know.
In any case, someone who want to find the PR will find the reference in the issue.
user_docs/en/userGuide.t2t
Outdated
|
||
There are four speech modes available. | ||
* Talk: NVDA will just speak normally. This is the default speech mode. | ||
* On-demand: NVDA will only speak when calling commands whose goal is to speak an information (e.g. report the title of the window); but it will not speak in reaction to actions such as moving the focus or the cursor. |
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.
Since I suspect you will change this to a hyphenated list, I have used that in the below to make it easy to apply my suggestion if you otherwise like it.
I have removed "an information" because "information" is plural in English. I have replaced "calling" (which is often understood in technical circles but may not be how users think of command use), with a more user friendly term.
Lastly, I have added some further clarification about what this mode does.
* On-demand: NVDA will only speak when calling commands whose goal is to speak an information (e.g. report the title of the window); but it will not speak in reaction to actions such as moving the focus or the cursor. | |
- On-demand: NVDA will only speak when you use commands which speak information (e.g. report the title of the window); but it will not speak in reaction to actions such as moving the focus or the cursor; and will not speak any screen changes, notifications, etc. |
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'll take this comment into account. But I want to keep the idea that the goal of the command is to report information. Writing "a command which speaks information" is too broad in my view.
user_docs/en/userGuide.t2t
Outdated
* Talk: NVDA will just speak normally. This is the default speech mode. | ||
* On-demand: NVDA will only speak when calling commands whose goal is to speak an information (e.g. report the title of the window); but it will not speak in reaction to actions such as moving the focus or the cursor. | ||
* Off: NVDA will not speak anything. | ||
* Beeps: NVDA will replace normal speech with quiet beeps. |
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 know "quiet" was my word, but it was a mistake.
Also, I have added some explanatory paragraphs about beeps and on-demand modes, since people are always asking what beeps mode is good for.
Lastly, I have used a hyphen for the list marker, and closed the list, since I think you will likely replace the stars used on the other items.
* Beeps: NVDA will replace normal speech with quiet beeps. | |
- Beeps: NVDA will replace normal speech with short beeps. | |
- | |
Beeps mode may be useful when some very verbose output is scrolling in a terminal window, but you don't care what it is, only that it is continuing to scroll; or in other circumstances when the fact that there is output is more relevant than the output itself. | |
The on-demand mode may be valuable when you don't need constant feedback about what is happening on screen or on the computer, but you periodically need to check particular things using review commands, etc. | |
Examples include while recording audio, when using screen magnification, or as an alternative to beeps mode. |
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.
Thanks. I'll include it and add one more example: during a meeting or a call.
user_docs/en/userGuide.t2t
Outdated
@@ -4049,7 +4062,7 @@ Please see the display's documentation for descriptions of where these keys can | |||
| Route to braille cell | ``routing`` | | |||
| Report text formatting under braille cell | ``secondary routing`` | | |||
| Toggle the way context information is presented in braille | ``attribute1+attribute3`` | | |||
| Toggles between the speech modes of off, beep and talk | ``attribute2+attribute4`` | | |||
| Toggles between the speech modes of off, beep, speech and on-demand | ``attribute2+attribute4`` | |
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.
| Toggles between the speech modes of off, beep, speech and on-demand | ``attribute2+attribute4`` | | |
| Toggles between the speech modes of off, beeps, talk and on-demand | ``attribute2+attribute4`` | | |
It is recommended that the table introducing the Toggles the speech mode shortcut keys in the user guide be simplified into three columns: Name, Key, and Description to enhance the reading experience. |
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. I made one small suggestion
user_docs/en/userGuide.t2t
Outdated
++ Speech modes ++[SpeechModes] | ||
|
||
There are four speech modes available. | ||
* Talk: NVDA will just speak normally. This is the default speech mode. |
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.
Small thing, but I don't like using "just" here. Someone will be reading the user guide because they don't know how things work and "just", to me, implies it is something they should have already known (there are articles on this such as https://www.excitant.co.uk/just-is-a-dangerous-word/ ). Perhaps:
Talk: The Default speech mode, NVDA speaks in reaction to actions such as moving the focus, notifications, or issuing commands.
Also, for consistency, should this group be in the same order as the command itself and the list on line 4065 (last change in this PR)
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 agree with @Qchristensen here. I had removed "just" in my original suggestion on the input help text for this, but it slipped back in to the user guide version. I figured @CyrilleB79 really wanted it there, so I didn't comment further.
* Talk: NVDA will just speak normally. This is the default speech mode. | |
- Talk: NVDA will speak normally in reaction to screen changes, notifications, and actions such as moving the focus, or issuing commands. |
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.
@XLTechie, I had missed this from your feedback, but keeping "just" was not intended. Thanks for having insisted on this point.
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.
@Qchristensen wrote:
Also, for consistency, should this group be in the same order as the command itself and the list on line 4065 (last change in this PR)
Yes for consistency, provided "Talk" is in first position since it is the normal mode.
user_docs/en/userGuide.t2t
Outdated
There are four speech modes available. | ||
* Talk: NVDA will just speak normally. This is the default speech mode. | ||
* On-demand: NVDA will only speak when calling commands whose goal is to speak an information (e.g. report the title of the window); but it will not speak in reaction to actions such as moving the focus or the cursor. | ||
* Off: NVDA will not speak anything. |
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.
* Off: NVDA will not speak anything. | |
- Off: NVDA will not speak anything, however unlike sleep mode it will silently react to commands. |
Can you change the sentence for on-demand mode in something like this to make it more readable for people who need easy language?
- On-demand: NVDA will speak only when pressing a key command with a reporting function (e.g. report the title of the window)
- It will not speak in reaction to actions such as moving the focus or copying content to clipboard.
Von: Cyrille Bougot ***@***.***>
Gesendet: Donnerstag, 23. November 2023 21:38
An: nvaccess/nvda ***@***.***>
Cc: Adriani90 ***@***.***>; Mention ***@***.***>
Betreff: Re: [nvaccess/nvda] Add a speech "on demand" mode (PR #15804)
@CyrilleB79 commented on this pull request.
_____
In user_docs/en/userGuide.t2t <#15804 (comment)> :
@@ -519,6 +518,20 @@ When the menu comes up, You can use the arrow keys to navigate the menu, and the
| Report clipboard text | NVDA+c | Reports the Text on the clipboard if there is any. |
%kc:endInclude
+++ Speech modes ++[SpeechModes]
+
+There are four speech modes available.
+* Talk: NVDA will just speak normally. This is the default speech mode.
+* On-demand: NVDA will only speak when calling commands whose goal is to speak an information (e.g. report the title of the window); but it will not speak in reaction to actions such as moving the focus or the cursor.
+* Off: NVDA will not speak anything.
+* Beeps: NVDA will replace normal speech with quiet beeps.
Thanks for the formatting fix. No reason to use * instead of -. I'll change it.
—
Reply to this email directly, view it on GitHub <#15804 (comment)> , or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGVCP4MUKWBE3Z7YM3XUPODYF6XZNAVCNFSM6AAAAAA7SCE7C2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTONBWHE3TAMJXHE> .
You are receiving this because you were mentioned. <https://github.com/notifications/beacon/AGVCP4J3ZGJAEI6AXXSVXULYF6XZNA5CNFSM6AAAAAA7SCE7C2WGG33NNVSW45C7OR4XAZNRKB2WY3CSMVYXKZLTORJGK5TJMV32UY3PNVWWK3TUL5UWJTTIECTEG.gif> Message ID: ***@***.*** ***@***.***> >
|
Thanks @XLTechie, @Qchristensen, @Adriani90, @wmhn1872265132 and @seanbudd for all these valuable ideas and comments to improve the documentation. I have tried to take into account the ideas from everyone which were very valuable. Let me know if some point is still missing. |
See test results for failed build of commit 38283b3e1a |
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.
One final comment for your possible consideration, but either way LGTM!
Thanks for going back and forth with me and others over the details.
Co-authored-by: Luke Davis <8139760+XLTechie@users.noreply.github.com>
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.
Look good, great work everyone!
…15873) Closes #15806 Summary of the issue: NVDA allows to change the currently used speech mode by pressing NVDA+s. By default this gesture cycles between 'talk', 'beeps', 'on-demand' and 'no speech'. For many users having ability to switch to beeps or on demand is unnecessary, and they have no need for modes different than speech and no speech. For other beeps mode is certainly useful, but it is much more important to be able to quickly switch between speech and no speech, so beeps mode just makes the goal slower to achieve. With the introduction of fourth speech mode in #15804 it has been decided that users should be given ability to disable speech modes they do not need to use. Description of user facing changes The new checkable list containing all speech modes (all are initially checked)has been added into the Speech settings panel. When switching between modes with NVDA+s only modes which are checked in the list are included. When applying settings selected modes are validated, to make sure at least two modes are checked. When the 'talk' mode is disabled user is warned that they may be left without any speech. Description of development approach Configuration spec has been expanded to store list of disabled modes - this ensures that if any new mode is added to NVDA it is enabled by default in the cycling script. Speech settings panel now contains a list of all the speech modes, only enabled one are checked. When introducing validation for the speech settings panel it has been discovered that code responsible for checking panel's validity displays a warning about it being invalid, prevents saving invalid config, yet after dismissing the warning settings dialog gets closed. Since this is not optimal validation for settings dialog has been modified, so that after warning is dismissed settings remain open, and the invalid control is focused. The cycling script has been modified to only cycle between enabled modes. Unit tests verifying its behavior, both when no modes are disabled (the default) and some are disabled, were added. To make them work it has been necessary to perform a small modification to the method which lists configuration profiles in the config module, it assumed that profiles directory always exists, which is not true in unit tests. Now when the directory is missing appropriate warning is logged, and empty list is returned. User guide has been expanded to describe newly added control.
Link to issue number:
Closes #481
Summary of the issue:
In some situations, e.g. meetings or when an alternative information channel (braille, visual with magnification), the user does not want NVDA to provide speech feedback on actions such as moving the cursor or focus. But they still want to have NVDA speaking on specific request.
Description of user facing changes
Add a new speech "on demand" mode in the NVDA+S toggle command. When speech on demand is selected, NVDA does not provide a speech feedback for actions, but still speak on explicit request.
The criterion to determine if a command should speak in "on demand" mode is:
Note: although say all commands move the cursor, they are speaking in the "on demand" mode since there is no point running them if speech is totally off.
Description of development approach
documentBase.py
, do not use script decorator as per Adding commands to jump to first/last row/column in tables #13435 (comment)Notes for reviewers
Testing strategy:
Manual tests of the modified scripts to support "on demand" mode:
Foobar not tested.
Tested other scripts to check that they do not speak in "on demand" mode.
Known issues with pull request:
None
Code Review Checklist: