Differential D28208 Diff 78253 applets/systemtray/package/contents/ui/items/StatusNotifierItem.qml

Changeset View

Standalone View

View Options

applets/systemtray/package/contents/ui/items/StatusNotifierItem.qml

Show All 21 Lines
22 22​
23AbstractItem { 23​AbstractItem {
24 id: taskIcon 24​ id: taskIcon
25 25​
26 itemId: model.Id 26​ itemId: model.Id
27 text: model.Title 27​ text: model.Title
28 mainText: model.ToolTipTitle !== "" ? model.ToolTipTitle : model.Title 28​ mainText: model.ToolTipTitle !== "" ? model.ToolTipTitle : model.Title
29 subText: model.ToolTipSubTitle 29​ subText: model.ToolTipSubTitle
30 icon: model.ToolTipIcon !== "" ? model.ToolTipIcon : model.Icon ? model.Icon : model.IconName 30​ icon: model.ToolTipIcon !== "" ? model.ToolTipIcon : model.Icon ? model.Icon : model.IconName
kmaterkaUnsubmitted
Done

You need to change here as well.

kmaterka: You need to change here as well.
31 textFormat: Text.AutoText 31​ textFormat: Text.AutoText
32 32​
33 iconItem: iconItem 33​ iconItem: iconItem
34 34​
35 PlasmaCore.IconItem { 35​ PlasmaCore.IconItem {
36 id: iconItem 36​ id: iconItem
37 source: { 37​ source: {
38 if (model.status === PlasmaCore.Types.NeedsAttentionStatus) { 38​ if (model.status === PlasmaCore.Types.NeedsAttentionStatus) {
39 if (model.AttentionIcon) { ​
40 return model.AttentionIcon ​
41 } ​
42 if (model.AttentionIconName) { 39​ if (model.AttentionIconName) {
43 return model.AttentionIconName 40​ return model.AttentionIconName
44 } 41​ }
42​ if (model.AttentionIconPixmap) {
43​ return model.AttentionIconPixmap
44​ }
45 } 45​ }
46 return model.Icon ? model.Icon : model.IconName 46​ return model.IconName ? model.IconName : model.IconPixmap
47 } 47​ }
48 48​
49 width: Math.min(parent.width, parent.height) 49​ width: Math.min(parent.width, parent.height)
50 height: width 50​ height: width
51 active: taskIcon.containsMouse 51​ active: taskIcon.containsMouse
52 52​
53 anchors { 53​ anchors {
54 left: parent.left 54​ left: parent.left
55 verticalCenter: parent.verticalCenter 55​ verticalCenter: parent.verticalCenter
56 } 56​ }
57 } 57​ }
58​ // IconItem.overlays only supports names so we need a second item for the overlay, using the same
kmaterkaUnsubmitted
Not Done

Should overlay pixmaps be supported in PlasmaCore.IconItem? I guess that would require changes in KIconLoader as well.

kmaterka: Should overlay pixmaps be supported in `PlasmaCore.IconItem`? I guess that would require…
59​ // positioning that KIconLoader::drawOverlays uses that IconItem uses internally
60​ PlasmaCore.IconItem {
61​ id: overlayIconItem
kmaterkaUnsubmitted
Done

add parent: iconItem.parent

kmaterka: add `parent: iconItem.parent`
62​ source: {
63​ if (model.OverlayIconName) {
64​ return model.OverlayIconName
65​ }
66​ if (model.OverlayIconPixmap) {
67​ return model.OverlayIconPixmap
68​ }
69​ return "";
70​ }
71​ width: {
72​ if (iconItem.width < 32) {
kmaterkaUnsubmitted
Not Done

In StatusNotifierItemSource::overlayIcon() it is "width <= 32". There is a pre-existing inconsistency between StatusNotifierItemSource and IconItem/KIconLoader.

kmaterka: In [[ https://phabricator.kde.org/source/plasma…
73​ return 8;
74​ }
75​ if (iconItem.width <= 48) {
76​ return 16
77​ }
78​ if (iconItem.width <= 96) {
79​ return 22;
80​ }
81​ if (iconItem.width < 256) {
82​ return 32
83​ }
84​ return 64;
ngrahamUnsubmitted
Done

Most of these numbers correspond to standard icon sizes (e.g. units.iconSize.smallMedium == 22); maybe use those where possible

ngraham: Most of these numbers correspond to standard icon sizes (e.g. `units.iconSize.smallMedium` ==…
davidreAuthorUnsubmitted
Done

Good to know! That was a straight copy from https://cgit.kde.org/kiconthemes.git/tree/src/kiconloader.cpp#n478 :D
I'm sure we can find better sizing though, I think this is to small, for sure.

davidre: Good to know! That was a straight copy from https://cgit.kde.org/kiconthemes.
kmaterkaUnsubmitted
Done

StatusNotifierItemSource::overlayIcon() also has some sizes hard coded. I quickly checked, looks like the values are the same.
You can use the same syntax here:

if (iconItem.width <= units.iconSize.medium) {
    return units.iconSize.small / 2;
}
if (iconItem.width <= units.iconSize.large) {
    return units.iconSize.small;
}
kmaterka: [[ https://phabricator.kde.org/source/plasma…
85​ }
86​ height: width
87​ anchors {
88​ right: iconItem.right
89​ bottom: iconItem.bottom
90​ bottomMargin: 0.05 * iconItem.width
ngrahamUnsubmitted
Done

always round when multiplying by a non-integer value

ngraham: always round when multiplying by a non-integer value
kmaterkaUnsubmitted
Done

In StatusNotifierItemSource::overlayIcon() it has a different logic to position overlay. Margin is always the size of overlay icon. Another pre-existing inconsistency. Which one to choose? I think that KIconLoader is more important.

kmaterka: In [[ https://phabricator.kde.org/source/plasma…
davidreAuthorUnsubmitted
Done

Hmm to me it looks it doesn't even have an margin. Or am I reading p.drawPixmap(QRect(m_iconPixmap.width()-size, m_iconPixmap.height()-size, size, size), overlay->pixmap(size, size), QRect(0,0,size,size)); wrong?

davidre: Hmm to me it looks it doesn't even have an margin. Or am I reading ` p.drawPixmap(QRect…
kmaterkaUnsubmitted
Done

Yes, you are correct, my mistake. So StatusNotifierItemSource has no margin and KIconLoader has 5% of icon size - another inconsistency.
What about 'always round when multiplying by a non-integer value'? I don't know exactly why, but AFAIK not-rounded number can cause inconsistent behavior in QML.
@ngraham can you give a word of explanation?

kmaterka: Yes, you are correct, my mistake. So `StatusNotifierItemSource` has no margin and `KIconLoader`…
ngrahamUnsubmitted
Done

Yes, when you have a positional value that isn't an integer, the engine can end up positioning the item on half-pixels and everything looks blurry.

ngraham: Yes, when you have a positional value that isn't an integer, the engine can end up positioning…
brunsUnsubmitted
Done

The 0.05 * iconSize in KIconLoader has an implicit math.floor by the int conversion.

bruns: The `0.05 * iconSize` in KIconLoader has an implicit `math.floor` by the int conversion.
91​ rightMargin: anchors.bottomMargin
92​ }
93​ }
58 94​
59 onContextMenu: { 95​ onContextMenu: {
60 openContextMenu(plasmoid.nativeInterface.popupPosition(taskIcon, mouse.x, mouse.y)) 96​ openContextMenu(plasmoid.nativeInterface.popupPosition(taskIcon, mouse.x, mouse.y))
61 } 97​ }
62 98​
63 onClicked: { 99​ onClicked: {
64 var pos = plasmoid.nativeInterface.popupPosition(taskIcon, mouse.x, mouse.y); 100​ var pos = plasmoid.nativeInterface.popupPosition(taskIcon, mouse.x, mouse.y);
65 101​
▲ Show 20 Lines • Show All 62 Lines • Show Last 20 Lines