D5695: Applet: Automatically raise maximum volume when over defined maximum volume

2017-05-06 Thread David Rosca
This revision was automatically updated to reflect the committed changes.
Closed by commit R115:d78689251344: Applet: Automatically raise maximum volume 
when over defined maximum volume (authored by drosca).

REPOSITORY
  R115 Plasma Audio Volume Applet

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5695?vs=14109=14203

REVISION DETAIL
  https://phabricator.kde.org/D5695

AFFECTED FILES
  applet/contents/ui/ListItemBase.qml

To: drosca, #plasma, sebas
Cc: sebas, Zren, plasma-devel, spstarr, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, apol, lukas


D5695: Applet: Automatically raise maximum volume when over defined maximum volume

2017-05-06 Thread David Rosca
drosca added inline comments.

INLINE COMMENTS

> sebas wrote in ListItemBase.qml:282
> Should be "volume", since that's the property. Or perhaps we don't need 
> "volume" or "Volume".

"Volume" is a role from the model, so assigning to the "volume" property 
instead only makes it go through one indirection level. The "volume" property 
should be made readonly as it is there only for property changes.

REPOSITORY
  R115 Plasma Audio Volume Applet

BRANCH
  auto-raise-volume (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D5695

To: drosca, #plasma, sebas
Cc: sebas, Zren, plasma-devel, spstarr, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, apol, lukas


D5695: Applet: Automatically raise maximum volume when over defined maximum volume

2017-05-06 Thread Sebastian Kügler
sebas added a comment.


  You're right, forgot to submit the cosmetical comment.

INLINE COMMENTS

> ListItemBase.qml:282
> +if (!slider.forceRaiseMaxVolume && Volume > 
> PulseAudio.NormalVolume) {
> +Volume = PulseAudio.NormalVolume;
> +}

Should be "volume", since that's the property. Or perhaps we don't need 
"volume" or "Volume".

REPOSITORY
  R115 Plasma Audio Volume Applet

BRANCH
  auto-raise-volume (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D5695

To: drosca, #plasma, sebas
Cc: sebas, Zren, plasma-devel, spstarr, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, apol, lukas


D5695: Applet: Automatically raise maximum volume when over defined maximum volume

2017-05-06 Thread David Rosca
drosca added a comment.


  In https://phabricator.kde.org/D5695#106996, @sebas wrote:
  
  > Cosmetical comment aside, looks good.
  
  
  What's that cosmetical comment? Did you forget to send it?

REPOSITORY
  R115 Plasma Audio Volume Applet

BRANCH
  auto-raise-volume (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D5695

To: drosca, #plasma, sebas
Cc: sebas, Zren, plasma-devel, spstarr, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, apol, lukas


D5695: Applet: Automatically raise maximum volume when over defined maximum volume

2017-05-05 Thread Sebastian Kügler
sebas accepted this revision.
sebas added a comment.
This revision is now accepted and ready to land.


  Cosmetical comment aside, looks good.

REPOSITORY
  R115 Plasma Audio Volume Applet

BRANCH
  auto-raise-volume (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D5695

To: drosca, #plasma, sebas
Cc: sebas, Zren, plasma-devel, spstarr, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, apol, lukas


D5695: Applet: Automatically raise maximum volume when over defined maximum volume

2017-05-03 Thread David Rosca
drosca updated this revision to Diff 14109.
drosca added a comment.


  Fix issue pointed by Zren

REPOSITORY
  R115 Plasma Audio Volume Applet

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5695?vs=14105=14109

BRANCH
  auto-raise-volume (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D5695

AFFECTED FILES
  applet/contents/ui/ListItemBase.qml

To: drosca, #plasma
Cc: Zren, plasma-devel, spstarr, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, lukas


D5695: Applet: Automatically raise maximum volume when over defined maximum volume

2017-05-03 Thread David Rosca
drosca added a comment.


  In https://phabricator.kde.org/D5695#106714, @Zren wrote:
  
  > I'm guessing the side effect of this method is that if you uncheck "force 
boost" when the volume is 150%, it will stay at 150% volume until you manually 
slide it down to below 100%? If it does, wouldn't it then set the 
slider.maxValue to 100%, causing the volume to jump from 100% down to 66% 
(since you'd be 1/3 of the way down the slider when the max jumps from 150% to 
100%).
  >
  > If that does happen, we probably need a `if (!forced && volume > 100%) 
volume = 100%` when you uncheck it.
  
  
  Good catch, thanks.

REPOSITORY
  R115 Plasma Audio Volume Applet

REVISION DETAIL
  https://phabricator.kde.org/D5695

To: drosca, #plasma
Cc: Zren, plasma-devel, spstarr, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, lukas


D5695: Applet: Automatically raise maximum volume when over defined maximum volume

2017-05-03 Thread Chris Holland
Zren added a comment.


  Hmmm, this seems better than my solution since mine stays in the boosted 
state (slide.maxValue=150%) after the volume when back below 100%.
  
  I'm guessing the side effect of this method is that if you uncheck "force 
boost" when the volume is 150%, it will stay at 150% volume until you manually 
slide it down to below 100%? If it does, wouldn't it then set the 
slider.maxValue to 100%, causing the volume to jump from 100% down to 66% 
(since you'd be 1/3 of the way down the slider when the max jumps from 150% to 
100%).
  
  If that does happen, we probably need a `if (!forced && volume > 100%) volume 
= 100%` when you uncheck it.

REPOSITORY
  R115 Plasma Audio Volume Applet

REVISION DETAIL
  https://phabricator.kde.org/D5695

To: drosca, #plasma
Cc: Zren, plasma-devel, spstarr, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, lukas


D5695: Applet: Automatically raise maximum volume when over defined maximum volume

2017-05-03 Thread David Rosca
drosca created this revision.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.

REVISION SUMMARY
  When volume is above the maximum volume set in applet, moving the volume
  slider will only lower it to the maximum applet volume.
  This change automatically raises maximum volume when over defined
  maximum volume (eg. when the volume is set by other volume control app
  or when set by the applications itself like VLC), so it is possible to
  control volume over the defined maximum value without first raising maximum 
volume.

TEST PLAN
  Raised volume over 100% in VLC, applet automatically raised max volume for 
the stream.
  Lowered the volume to 100%, stream in applet went back to global maximum 
volume.

REPOSITORY
  R115 Plasma Audio Volume Applet

BRANCH
  auto-raise-volume (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D5695

AFFECTED FILES
  applet/contents/ui/ListItemBase.qml

To: drosca, #plasma
Cc: plasma-devel, spstarr, progwolff, Zren, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, lukas