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
