D26256: [Applet] Make 'raise maximum volume' global

[Nathaniel Graham]

ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  Works great. Just a few code change requests:

INLINE COMMENTS

> main.qml:546
> +                id: raiseMaximumVolumeCheckbox
> +                spacing: units.smallSpacing * 1.5
> +                Layout.leftMargin: units.smallSpacing * 0.75 + 4

Don't override this; if it looks bad with the default value, we should fix that in the checkbox control itself

> main.qml:547
> +                spacing: units.smallSpacing * 1.5
> +                Layout.leftMargin: units.smallSpacing * 0.75 + 4
> +                checked: plasmoid.configuration.raiseMaximumVolume

This could evaluate to a fractional number. You probably want `units.largeSpacing` here (the spacing doesn't have to be //literally// identical to the mockup; it's more important to use standard spacing values)

> main.qml:549
> +                checked: plasmoid.configuration.raiseMaximumVolume
> +                onCheckedChanged: {
> +                    plasmoid.configuration.raiseMaximumVolume = checked

Always use `onToggled` to handle user input, never `onCheckedChanged`

If you're deliberately using `onCheckedChanged` because it has the side effect of evaluating the logic in here when the applet is first loaded, then don't do that, and instead explicitly call that logic in an `onCompleted` block

REPOSITORY
  R115 Plasma Audio Volume Applet

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

To: gvgeo, ngraham, #vdg, #plasma, drosca
Cc: drosca, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20200104/128d07d0/attachment-0001.html>