[Applet] Unify 'raise maximum volume' and 'maximum volume'
AbandonedPublic

Authored by gvgeo on Dec 26 2019, 1:35 PM.

Details

Reviewers
drosca
Group Reviewers
VDG
Plasma
Summary

Unify 'raise maximum volume' menu option and 'maximum volume' configuration setting.

Make default 'maximum volume' 150.
For volume control check 'raise maximum volume' option, to use 'maximum volume'.
Volume slider use 'maximum volume', if 'raise maximum volume' is enabled.

BUG: 385041
FIXED-IN: 5.18.0
Depends on D26212

Test Plan

Before:
Volume control from shortcuts will use 'maximum volume' for all devices.
Slider will set maximum range 150 for 'raise maximum volume' option.
After:
'Raise maximum volume' respects 'maximum volume' and,
'maximum volume' does not work without 'Raise maximum volume'.

Diff Detail

Repository
R115 Plasma Audio Volume Applet
Branch
max4
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 20284
Build 20302: arc lint + arc unit
gvgeo created this revision.Dec 26 2019, 1:35 PM
Restricted Application added a project: Plasma. · View Herald TranscriptDec 26 2019, 1:35 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
gvgeo requested review of this revision.Dec 26 2019, 1:35 PM

I think there is a deeper UI issue here in that there are two ways to raise the maximum volume that live in different places: one globally in the settings page, and another on a per-app/device basis accessed from the app/device's hamburger menu. Even if all the bugs with the individual modes are fixed (thanks for that), having two ways to do the same thing, with only differences in scope, seems like unnecessary specificity that is not likely to help most users. I wonder if we should instead unify on one or another, or at least only show the config UI for raising the maximum volume in one place rather than two.

What do you think?

Actually I spaced for a moment and just remembered that we already have a mockup in T10470 that shows how we'd like this to work in the future:

Basically the global "raise maximum volume" setting would be moved out of the settings window and into a footer on the main UI.

I get what you mean global, but I'll try to clarify for others, too.
Currently, the option in configuration, only works from shortcuts or mouse wheel on task icon.
And affects only the default output device, that is selected, at the moment of volume change.

From what I get, you'd like to become really global, without controlling the Max value.
The mockup in this case would have no more 115% maximum volume, and they all get 100% line mark.
The same would be for all the applications?

But I wonder, why need to have Raise maximum volume so accessible.
If it is because one application or device is low, instead of changing it all the time, it would make more sense to have separate mode.

About this patch:
In essence, does what you said in the first comment.
But keeps the separate mode instead of the global.
Maximum volume becomes just a setting, for the raise maximum volume.
Could hardcode 150% and remove the setting. Almost nothing would change for this patch.
But I recall a request that enabled values below 100%.

anthonyfieroni added inline comments.
applet/contents/config/main.xml
10

I don't think the default should be anything but 100

gvgeo planned changes to this revision.Dec 27 2019, 3:34 PM

Either I missing something, or I need to make it more clear.

With this patch, the maximum volume setting is for the devices/apps that have enabled the raise maximum option.
By default users will be limited to 100. Will need to enable raise maximum volume for some application or device first, for the default value of 150 to have any effect.
Even then, the rest devices and applications will keep using 100 as maximum value.

This could take more fine tuning, but was trying to keep patches small and clean:
renaming 'maximum volume' to 'custom maximum volume'(or something better to explain the difference),
adding an 'raise maximum volume for all devices/applications' option,
make label change to 'lower maximum volume' when lower than 100% is selected.

I cannot see any solution that will have as default 100 and not complicate the raise maximum volume even more.
In any mode(separate or global), if default maximum volume will be kept 100, user will need to enable raise maximum volume and change the maximum volume.
Even more important in case of removal of the setting. Will need to predefined a value.
Right now, the slider already uses 150 which users cannot be change, does not cause any issue(except the existence of two modes).

Marking as planned changes, no need to appear in review queues, if global mode is the way forward.

meven added a subscriber: meven.Dec 30 2019, 1:15 PM

I feel the "raise maximum volume" button does not need to be always visible as most users won't ever use it, instead the mute toggle could use the space.

ndavis added a subscriber: ndavis.Dec 30 2019, 10:22 PM

I feel the "raise maximum volume" button does not need to be always visible as most users won't ever use it, instead the mute toggle could use the space.

You'd be surprised how many non-technical people need it. A lot of laptops have very wimpy maximum volume, so users have to raise the maximum volume.

I feel the "raise maximum volume" button does not need to be always visible as most users won't ever use it, instead the mute toggle could use the space.

You'd be surprised how many non-technical people need it. A lot of laptops have very wimpy maximum volume, so users have to raise the maximum volume.

^^My exact use case

I feel the "raise maximum volume" button does not need to be always visible as most users won't ever use it, instead the mute toggle could use the space.

You'd be surprised how many non-technical people need it. A lot of laptops have very wimpy maximum volume, so users have to raise the maximum volume.

^^My exact use case

Still how often is it used ?
Should we set this by audio output, like only for internal speakers (as opposed to say a bluetooth speaker), so this settings never has to change once set ?
It feels like a workaround feature for a minority of our users will affect all users.

Anyway I am using my skepticism cap, I am fine trusting you.

gvgeo abandoned this revision.Jan 20 2020, 10:00 AM

So this isn't needed if we do D26256?

gvgeo added a comment.Jan 21 2020, 4:39 AM

D26256 removes these two settings. There is nothing for this patch to unify.