[Differential] [Commented On] D4751: Button and Context Menu to Mute, Set Default Sink/Source, Active Port

[David Rosca]

drosca added inline comments.

INLINE COMMENTS

> ListItemBase.qml:137
> +                        source: "application-menu"
> +                        Layout.maximumHeight: parent.height * 0.85
> +                        Layout.maximumWidth: Layout.maximumHeight

Use Layout.preferredHeight instead of maximumHeight.

> ListItemBase.qml:148
> +                            onReleased: parent.opacity = 1
> +                            onCanceled: parent.opacity = 1
> +                        }

parent.opacity: mouseArea.containsMouse ? 0.8 : 1.0

> ListItemBase.qml:264
> +            menuItem.checkable = true;
> +            menuItem.checked = PulseObject.muted;
> +            menuItem.clicked.connect(function() {

I know it was like that in the original code, but please rewrite it to use directly the properties (`Muted` in this case) instead of accessing it through `PulseObject`.

> ListItemBase.qml:283
> +            // Ports
> +            if (PulseObject.ports && 0 < PulseObject.ports.length) {
> +                contextMenu.addMenuItem(newSeperator());

Please move literal to right side of comparison: `PulseObject.ports.length > 0` or even in this case you can just write `PulseObject.ports.length`.

But actually make it `Ports.length` as said in previous comment.

> ListItemBase.qml:316
> +
> +    MouseArea {
> +        acceptedButtons: Qt.RightButton

If we now have a button for showing this menu, I think we can completely remove the right click context menu.

REPOSITORY
  R115 Plasma Audio Volume Applet

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: subdiff, #plasma, Zren, drosca
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20170223/cab93d1d/attachment-0001.html>