D17983: StatusItemNotifier: fix overlays by name with icons by name

2019-01-28 Thread Pino Toscano
pino closed this revision. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D17983 To: pino, apol Cc: davidedmundson, apol, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, mart

D17983: StatusItemNotifier: fix overlays by name with icons by name

2019-01-28 Thread David Edmundson
davidedmundson added a comment. > Thanks -- which branch should I push this to? > Plasma/5.12 If there's no risk of regression, yes. That's a judgement call I think you're in the best position to make. (IMHO it should be fine, the absolutely worst case is some slightly wrong

D17983: StatusItemNotifier: fix overlays by name with icons by name

2019-01-27 Thread Pino Toscano
pino added a comment. In D17983#388160 , @pino wrote: > Thanks -- which branch should I push this to? `Plasma/5.12`, `Plasma/5.14`, or only `master` for now? ping? REPOSITORY R120 Plasma Workspace BRANCH sni-overlays (branched from

D17983: StatusItemNotifier: fix overlays by name with icons by name

2019-01-07 Thread Pino Toscano
pino added a comment. Thanks -- which branch should I push this to? `Plasma/5.12`, `Plasma/5.14`, or only `master` for now? REPOSITORY R120 Plasma Workspace BRANCH sni-overlays (branched from master) REVISION DETAIL https://phabricator.kde.org/D17983 To: pino, apol Cc: apol,

D17983: StatusItemNotifier: fix overlays by name with icons by name

2019-01-07 Thread Pino Toscano
pino added inline comments. INLINE COMMENTS > apol wrote in statusnotifieritemsource.cpp:296 > Can this bit be removed then? No, this is needed for the "overlay by pixmap" case: `overlayNames` is empty (because no overlay name is available), while `overlay` is not null (as read by the pixmap)

D17983: StatusItemNotifier: fix overlays by name with icons by name

2019-01-07 Thread Aleix Pol Gonzalez
apol added a comment. +1 Makes a lot of sense overall, I'd just like to make sure we're not missing something. INLINE COMMENTS > statusnotifieritemsource.cpp:296 > > if (overlayNames.isEmpty() && !overlay.isNull()) { > overlayIcon(, ); Can

D17983: StatusItemNotifier: fix overlays by name with icons by name

2019-01-05 Thread Pino Toscano
pino created this revision. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. pino requested review of this revision. REVISION SUMMARY Setting an overlay by name results in a QIcon for it created; OTOH, this icon is never used to create the final image in case the