-
-
Notifications
You must be signed in to change notification settings - Fork 720
Shorten indentation beeps #18898
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
Shorten indentation beeps #18898
Conversation
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.
Pull Request Overview
This PR reduces the duration of line indentation beeps in NVDA to make them less distracting, especially at high speech rates. The beeps are now the same length as progress bar beeps.
- Replaced the hardcoded
IDT_TONE_DURATION
constant with a configurable duration - Added a new configuration setting for indentation tone duration with a default of 40ms
- Created a getter function to access the configurable duration value
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
user_docs/en/changes.md | Added changelog entry documenting the API change |
source/speech/speech.py | Removed constant, added getter function, and updated usage |
source/speech/init.py | Updated exports to replace constant with getter function |
source/config/configSpec.py | Added new configuration setting for indentation tone duration |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
While I like the change that this becomes configurable, I'm not sure about changing the length. Note that indentation beeps is especially helpful to people with perfect pitch, as they can hear the exact indentation level based on the note of the beep. Distinguishing differences becomes harder when shortening the beep. I myself can distinguish them just fine, but it definitely needs more attention. |
I personally don't agree, if the beeps are long enough for progress bar beeps, something I assume perfect pitch could also be used for to tell what percent you're at, it should be good enough for coding. |
@trypsynth seems like a matter of personal preference. As said, I can personally deal with it just fine, but I'm not sure about others. |
@LeonarddeR I currently don't use indentation beeps because I find them disruptively long. In conversation, @trypsynth also mentioned that they find them too long. I fail to understand how 40ms can be long enough for progress bar tones but not for indentation tones. |
Because recognizing an exact pitch value, absolutely or relatively, is much more useful for indent size than for progress bar:
In any case, I have not yet any opinion on the duration. Making this configurable will allow to test it. |
@CyrilleB79 explained this much better than I ever could. Thanks! |
@LeonarddeR, @CyrilleB79 would you be more comfortable if we retained a tone duration of 80ms for the moment and added a means of adjusting the length in NVDA's settings? That would solve mine and @trypsynth's problem, while not changing things for folks who prefer the current duration. |
@SaschaCowley I wouldn't necessarily want to go that far. I just wanted to share a concern that might be a show stopper for others. Happy if the default becomes 40ms. We can always make it configurable when people start complaining afterwards. |
can this get a changelog entry under changes? |
@seanbudd done |
I don't either have an opinion with the current duration of this beep, neither have heard any concern from someone else about it. Making it configurable, at least in config spec, will help to experiment. I'd say that you may shorten it during alpha stage to test. Unless there is a real demand, I'd not make it configurable in the GUI to avoid cluttering the GUI with any single little parameter. Let's try to choose a value that suits everyone if possible. |
When I designed this feature, I picked a length that sounded good to me at the time. There was no rhyme or reason to it. How hard would it be to scale the length down with the speed of speech somehow, on a per synth method, or expose something under advanced settings? I like it the way it is, but could probably get used to shorter beeps. Keep in mind that people who don't have perfect pitch may want a longer beep to get a good feel on the current pitch relative to the other pitch. |
Link to issue number:
None
Summary of the issue:
NVDA can be configured to report line/paragraph indentation with beeps. However, the beeps are distractingly long, especialy at high speech rates.
Description of user facing changes:
The duration of line indentation beeps has been reduced. They are now the same length as progress bar beeps.
Description of developer facing changes:
The
IDT_TONE_DURATION
constant has been removed.Description of development approach:
Moved line indentation beep duration to config, so this customisation may be surfaced if desired in future. Replaced the constant with a getter method.
Testing strategy:
Navigated code with line indentation set to beeps.
Known issues with pull request:
None
Code Review Checklist: