Skip to content

Fix appVolumeAdjuster when state is default#17321

Merged
SaschaCowley merged 5 commits into
nvaccess:masterfrom
mltony:fix_ava_state
Nov 3, 2024
Merged

Fix appVolumeAdjuster when state is default#17321
SaschaCowley merged 5 commits into
nvaccess:masterfrom
mltony:fix_ava_state

Conversation

@mltony

@mltony mltony commented Oct 24, 2024

Copy link
Copy Markdown
Contributor

Link to issue number:

Fixes #17300

Summary of the issue:

When AppVolumeAdjuster state is set to default, pressing OK or Apply button in settings sets the volume of all apps to 100%.

Description of user facing changes

Bug fixed. Now when AppVolumeAdjuster state is set to default, application volumes are not changed.

Description of development approach

Checking for state==default in the right place.

Testing strategy:

Manually tested use case from the issue above.

Known issues with pull request:

N/A

Code Review Checklist:

  • Documentation:
    • Change log entry
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • API is compatible with existing add-ons.
  • Security precautions taken.

@coderabbitai summary

@mltony mltony requested a review from a team as a code owner October 24, 2024 02:51
@mltony mltony requested a review from SaschaCowley October 24, 2024 02:51
Comment thread source/audio/appsVolume.py Outdated
Comment thread source/audio/appsVolume.py Outdated
Comment thread source/audio/appsVolume.py Outdated
Co-authored-by: Sascha Cowley <16543535+SaschaCowley@users.noreply.github.com>
@seanbudd seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Oct 28, 2024

@SaschaCowley SaschaCowley left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@mltony this currently fails because you're getting an instance of AppsVolumeAdjusterFlag, not FeatureFlag, in AudioPanel.onSave. You need to use FeatureFlagCombo ._getControlCurrentFlag, not FeatureFlagCombo._getControlCurrentValue.
https://github.com/mltony/nvda/blob/562eebb936e452011987f410afdd5931d5424d9a/source/gui/settingsDialogs.py#L3216

@mltony

mltony commented Nov 3, 2024

Copy link
Copy Markdown
Contributor Author

@SaschaCowley, fixed and tested.

@SaschaCowley SaschaCowley left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for this, @mltony

@SaschaCowley SaschaCowley merged commit d3d0755 into nvaccess:master Nov 3, 2024
@github-actions github-actions Bot added this to the 2025.1 milestone Nov 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Apllying audio settings resets volume level for apps

4 participants