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

[Roman Gilg]

subdiff added a comment.


  In https://phabricator.kde.org/D4751#89222, @Zren wrote:
  
  > Looks like the icon is really small in plasmoidviewer?
  
  
  Using scaling factor 2 on 4K. Have no idea why it looks this way for you.
  
  > Just wondering why you used a IconItem+MouseArea pattern for the button instead of `PlasmaComponents.ToolButton`?
  
  Was copy-paste of another one. But in general I don't like the frame on mouse over for a ToolButton. If you want the ToolButton though, we can talk about it.

INLINE COMMENTS

> Zren wrote in ListItemBase.qml:283
> Definitely go for the explicit `var > 0` as the latter could be read as "if this variable exists". That's usually used as a JS minimisation trick.

> Please move literal to right side

I like to read my inequalities from lower value to higher one. Easier to read the code this way. Besides style is there something else speaking against that? What @Zren said?

> ...as the latter could be read as "if this variable exists".

Please explain what you mean by this.

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, drosca, Zren
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/8f8bcfea/attachment-0001.html>