Create audio panel for new sound settings #15472
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
seanbudd
requested review from
Qchristensen and
michaelDCurran
and removed request for
a team
6 tasks
|
Now that there is an independent audio settings page, should the "Output Device" option also be moved to this page? |
done |
michaelDCurran
approved these changes
Sep 20, 2023
|
So, is it still necessary to retain the original speech synthesizer dialog? Can the combo box for selecting a speech synthesizer be an item in the speech settings panel? No need to open a separate dialog. |
|
@cary-rowen - I think so yes, changing the synthesizer requires some special handling that we want to keep separate from the rest of the panel |
Qchristensen
approved these changes
Sep 20, 2023
This was referenced Sep 20, 2023
|
@seanbudd Removing "output device" from the NVDA+Control+s dialog will both break muscle memory for long-time NVDA users and make it very hard to blindly (without audible speech or connected Braille) change sound devices in the event of unexpected device switching. Please duplicate the combo box in both dialogs (my preference) or make this gesture open the new panel. |
5 tasks
seanbudd
pushed a commit
that referenced
this pull request
Sep 20, 2023
Follow-up of #15472. Summary of the issue: If NVDA is started with WASAPI disabled in config and if you enable WASAPI, the new audio options to control NVDA sounds along with voice and volume of NVDA sound are available. That's confusing because modifying them will not have any effect until NVDA is restarted. Description of user facing changes Enabling or disabling audio options linked to WASAPI will be done looking at current state of WASAPI usage rather than looking at the state configured for next restart. Also added an indication in the User Guide that these options can be unavailable so that the user does not look for them when they are greyed out. Description of development approach Rather than checking the config, check the player currently used to determine if the WASAPI related options need to be disabled or not.
seanbudd
added a commit
that referenced
this pull request
Sep 20, 2023
|
@codeofdusk couldn't that be solved by providing a gesture to cycle through
output devices? That is something we've needed for a long time anyway.
|
|
I have code for cycling through output devices forward and backward
and am going to open a pr soon.
…On 9/21/23, Bill Dengler ***@***.***> wrote:
@XLTechie It could be, but we'll probably get pushback for that at this
point in the 2023.3 release cycle. I've opened #15486 (targetting beta) and
will look into something else for 2024.1.
--
Reply to this email directly or view it on GitHub:
#15472 (comment)
You are receiving this because you are subscribed to this thread.
Message ID: ***@***.***>
--
with best regards Beqa Gozalishvili
Tell: +995593454005
Email: ***@***.***
Web: https://gozaltech.org
Skype: beqabeqa473
Telegram: https://t.me/gozaltech
facebook: https://facebook.com/gozaltech
twitter: https://twitter.com/beqabeqa473
Instagram: https://instagram.com/beqa.gozalishvili
|
|
Hello.
#15493 is created.
…On 9/21/23, Beqa Gozalishvili ***@***.***> wrote:
I have code for cycling through output devices forward and backward
and am going to open a pr soon.
On 9/21/23, Bill Dengler ***@***.***> wrote:
> @XLTechie It could be, but we'll probably get pushback for that at this
> point in the 2023.3 release cycle. I've opened #15486 (targetting beta)
> and
> will look into something else for 2024.1.
>
> --
> Reply to this email directly or view it on GitHub:
> #15472 (comment)
> You are receiving this because you are subscribed to this thread.
>
> Message ID: ***@***.***>
--
with best regards Beqa Gozalishvili
Tell: +995593454005
Email: ***@***.***
Web: https://gozaltech.org
Skype: beqabeqa473
Telegram: https://t.me/gozaltech
facebook: https://facebook.com/gozaltech
twitter: https://twitter.com/beqabeqa473
Instagram: https://instagram.com/beqa.gozalishvili
--
with best regards Beqa Gozalishvili
Tell: +995593454005
Email: ***@***.***
Web: https://gozaltech.org
Skype: beqabeqa473
Telegram: https://t.me/gozaltech
facebook: https://facebook.com/gozaltech
twitter: https://twitter.com/beqabeqa473
Instagram: https://instagram.com/beqa.gozalishvili
|
|
@seanbudd could you add the shortcut ctrl+NVDA+u for this settings category and document it properly in the user guide and key commands reference? |
5 tasks
LeonarddeR
reviewed
Sep 23, 2023
seanbudd
added a commit
that referenced
this pull request
Sep 25, 2023
Fixup of #15472 Summary of the issue: A default gesture to open the new audio panel is desirable Description of user facing changes Adds nvda+ctrl+u as a command to open the audio panel Commands for opening settings panels are now included in the key commands doc Description of development approach Adds nvda+ctrl+u as a command to open the audio panel Adds keyCommands to the appveyor build for easier review of changes. Updates to user guide
seanbudd
pushed a commit
that referenced
this pull request
Sep 25, 2023
Link to issue number: Partially reverts #15472. Summary of the issue: The "output device" option has been removed from the NVDA+Control+s dialog. This breaks muscle memory for long-time NVDA users and makes it very hard to blindly (without audible speech or connected Braille) change sound devices in the event of unexpected device switching. Description of how this pull request fixes the issue: Selectively reverts the change in #15472 that removes the option from the synth selection dialog. Testing strategy: Verified that the "output device" option in both dialogs works as expected
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Link to issue number:
Fix #8711
Summary of the issue:
WASAPI is now enabled by default, so the new features for setting the volume of NVDA sounds should be moved out of the advanced setting panel.
Description of user facing changes
A new audio settings panel is created for:
Description of development approach
Move settings, update user guide
Testing strategy:
Test change the various settings.
Ensure the settings are toggled as expected when WASAPI is toggled.
Known issues with pull request:
None
Change log entries:
Refer to diff
Code Review Checklist: