D11746: Assign notification area category

[Wojciech Stachurski]

wsdfhjxc added a comment.


  All of them seem to be accepted, but I can't land anything by myself. Feel 
free to do it on your own.

REPOSITORY
  R116 Plasma Network Management Applet

BRANCH
  patch-1

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

To: wsdfhjxc, #plasma, broulik
Cc: Pitel, plasma-devel, ragreen, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D11750: Assign notification area category

[Wojciech Stachurski]

wsdfhjxc created this revision.
wsdfhjxc added a reviewer: Plasma.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
wsdfhjxc requested review of this revision.

REVISION SUMMARY
  This is a supplement for D11352 .

REPOSITORY
  R114 Plasma Addons

BRANCH
  patch-1

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

AFFECTED FILES
  applets/keyboardindicator/metadata.desktop

To: wsdfhjxc, #plasma
Cc: plasma-devel, ragreen, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D11749: Assign notification area category

[Wojciech Stachurski]

wsdfhjxc created this revision.
wsdfhjxc added a reviewer: Plasma.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
wsdfhjxc requested review of this revision.

REVISION SUMMARY
  This is a supplement for D11352 .

REPOSITORY
  R119 Plasma Desktop

BRANCH
  patch-1

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

AFFECTED FILES
  kcms/touchpad/src/applet/qml/metadata.desktop

To: wsdfhjxc, #plasma
Cc: plasma-devel, ragreen, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D11748: Assign notification area category

[Wojciech Stachurski]

wsdfhjxc created this revision.
wsdfhjxc added a reviewer: Plasma.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
wsdfhjxc requested review of this revision.

REVISION SUMMARY
  This is a supplement for D11352 .

REPOSITORY
  R134 Discover Software Store

BRANCH
  patch-1

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

AFFECTED FILES
  notifier/plasmoid/metadata.desktop

To: wsdfhjxc, #plasma
Cc: plasma-devel, ragreen, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D11746: Assign notification area category

[Wojciech Stachurski]

wsdfhjxc created this revision.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
wsdfhjxc requested review of this revision.

REVISION SUMMARY
  This is a supplement for D11352 .

REPOSITORY
  R116 Plasma Network Management Applet

BRANCH
  patch-1

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

AFFECTED FILES
  applet/metadata.desktop

To: wsdfhjxc
Cc: plasma-devel, ragreen, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D11352: [RFC] Auto ordered systray

[Wojciech Stachurski]

wsdfhjxc added a comment.


  Unfortunately, comparing by `item.text` with a non-US language enabled makes 
the result worse than on previous screenshot (especially the order of items 
within the `Hardware` group is very odd). To get it consistent regardless of 
system's language setting it might be better to compare by `item.itemId`. 
Although `StatusNotifierItem` may have it undefined initially, it can be fixed 
with a timer hack and waiting till a proper `Id` is available, then updating 
the `itemId` property and calling `updateItemVisibility()` function.
  
  Modifying the comparing function to look at `item.itemId` instead of 
`item.text` makes the result no better than it was originally. However, if 
compared strings are slightly altered in a specific way, the result might be 
better (something like a seed to get the most satisfying pattern). For example:
  
function compareItems(a, b) {
var categoryDiff = indexForItemCategory(a) - indexForItemCategory(b)
var altA = a.itemId ? a.itemId.replace("org.kde.", 
"").replace("plasma.", "") : ""
var altB = b.itemId ? b.itemId.replace("org.kde.", 
"").replace("plasma.", "") : ""
altA = altA.substring(altA.length / 2, altA.length)
altB = altB.substring(altB.length / 2, altB.length)
return categoryDiff != 0 ? categoryDiff : altA.localeCompare(altB)
}
  
  Screenshot below presents the item ordering result after this modification, 
which is consistent regardless of system's language setting and which looks 
nicer in my opinion.
  F5759917: Items.png 
  
  I don't really have any other ideas on further improvements, so forgive me if 
you find this one utterly stupid, I promise there will bo no more of them :)

REPOSITORY
  R120 Plasma Workspace

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

To: Pitel
Cc: wsdfhjxc, mart, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D11352: [RFC] Auto ordered systray

[Wojciech Stachurski]

wsdfhjxc added a comment.


  They are `org.kde.plasma.keyboardindicator` and `org.kde.discovernotifier`.

REPOSITORY
  R120 Plasma Workspace

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

To: Pitel
Cc: wsdfhjxc, mart, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D11352: [RFC] Auto ordered systray

[Wojciech Stachurski]

wsdfhjxc added a comment.


  Thanks for a fix.
  
  I've changed the category order to
  
  - `UnknownCategory`
  - `ApplicationStatus`
  - `Communications`
  - `SystemServices`
  - `Hardware`
  
  and updated `metadata.desktop` for some Plasma components
  
  - Networks  -> `Hardware`
  - KDE Connect   -> `Hardware`
  - Touchpad  -> `Hardware`
  - Keyboard Indicator-> `Hardware`
  - Discover Notifier -> `SystemServices`
  
  Does it make any sense? F5759406: Items.png 


REPOSITORY
  R120 Plasma Workspace

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

To: Pitel
Cc: wsdfhjxc, mart, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D11352: [RFC] Auto ordered systray

[Wojciech Stachurski]

wsdfhjxc added a comment.


  In D11352#227154 , @mart wrote:
  
  > yes, categories of those items need to be fixed, this was kinda forgotten 
as in plasma5 unfortunately the systray wasn't really using them
  
  
  It would be great if that happened. The more precise item categories are, the 
more sensible auto ordering will be.
  
  In D11352#227210 , @mart wrote:
  
  > did you just copy over the qml files on an old installation or rebuilt all 
workspace?
  
  
  I've applied the patch on a clean branch and rebuilt the `plasma-workspace` 
(even from scratch). If it works for you, then something else is definitely 
broken on my side, so never mind me.

REPOSITORY
  R120 Plasma Workspace

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

To: Pitel
Cc: wsdfhjxc, mart, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D11352: [RFC] Auto ordered systray

[Wojciech Stachurski]

wsdfhjxc added a comment.


  > That seems like you are getting some javascript error in 
`updateItemVisibility`. Can look at what plasma shell is reporting (or paste it 
here)?
  
  There are no other errors except of `ConfigEntries.qml:228:34: Unable to 
assign [undefined] to QKeySequence` spam which is unrelated. Not sure what I 
did wrong.
  
  > I do not like the idea of large per applet order list because it will 
always be incomplete and give ugly results if user chooses to replace e.g. 
usual battery applet with some custom one.
  
  Yes, that would have worked only if the user didn't replace embedded items 
with some custom ones for the same indication purpose. So, you are right, it's 
probably a no-go.
  
  > If we conclude that sort based on categories + text is no sufficient I 
would strongly prefer using D11292  
possibly with a larger default config rather hardcoding half random long list 
into `main.qml`. (Long list in default config is fine by me because user can 
modify that to their wishes.)
  
  Judging by previous responses, manually arrangeable system tray is not going 
to be a thing. Although, having it consistent is a nice improvement. What 
bothers me is those groups being vague and also that some default items (such 
as Networks, Touchpad, Updates) are assigned to `UnknownCategory` instead of 
one of specialized ones. Yes, this makes the system tray consistent, but the 
ordering gives a messy result where perceptually unrelated items might be 
placed between related ones. Still, better than random first-in-first-shown 
behavior.

REPOSITORY
  R120 Plasma Workspace

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

To: Pitel
Cc: wsdfhjxc, mart, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D11352: [RFC] Auto ordered systray

[Wojciech Stachurski]

wsdfhjxc added a comment.


  Doesn't seem to work for me. Only Notifications item is visible by default 
while there are multiple items set to be shown in the configuration. Also, 
changing the visibility state doesn't make any difference and the items are 
neither in panel nor in hidden panel, they just disappear. Only after going 
through configuration and manually switching every item to hidden and then to 
shown makes most of them visible.
  
  As for the item order, those categories are rather broad and sorting the 
items by their ids or texts within them will always produce odd results. Given 
there is a limited pool of known embedded items, maybe it would be better to 
predefine ordering values for some of them (battery=0, volume=1, networks=2, 
..., kdeconnect=5, devicenotifier=6, printers=7, etc.) to make the actual 
groups more sensible for humans?

REPOSITORY
  R120 Plasma Workspace

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

To: Pitel
Cc: wsdfhjxc, mart, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D11292: [RFC] Ordered system tray v2

[Wojciech Stachurski]

wsdfhjxc added a comment.


  Currently dragging active items into the trash bin and saving changes results 
in setting auto visibility and displaying them at the end of both the system 
tray and configuration list, so it's not really deleting them from user's point 
of view. I was wondering, shouldn't the trash bin be accessible only while 
dragging inactive items (assuming `statusNotifierModel` will be fixed)?
  
  Hidden items could be displayed below non-hidden items, because it's a little 
bit messy right now. But I see the whole list is dependant solely on 
`cfg_itemOrder` which contains mixed entries, so I guess this would require a 
bit more code to split them. Not sure what's your opinion on this, as it would 
bring additional complexity, but I think it would be better to somehow 
differentiate those two types and reduce confusion.

REPOSITORY
  R120 Plasma Workspace

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

To: Pitel, #plasma, #vdg
Cc: zzag, davidedmundson, anthonyfieroni, wsdfhjxc, ngraham, plasma-devel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D11292: [RFC] Ordered system tray v2

[Wojciech Stachurski]

wsdfhjxc added a comment.


  I can no longer reproduce the issue with incorrect visibility state in 
checkboxes. It seems to be working correctly after last patch.
  
  Also, I've added some minor styling suggestions, so it looks a little bit 
more polished in my opinion.

INLINE COMMENTS

> ConfigEntries.qml:201
> +QtLayouts.Layout.preferredWidth: 1
> +QtLayouts.Layout.fillHeight: true
> +}

height: measureButton.height

> ConfigEntries.qml:319
> +
> +Rectangle {
> +color: "black"

Rectangle {
  SystemPalette {
  id: sysPalette
  colorGroup: SystemPalette.Active
  }
  
  color: sysPalette.mid
  opacity: 0.85
  anchors.fill: parent
  }

> ConfigEntries.qml:328
> +anchors.margins: Math.round(units.gridUnit * 0.2)
> +icon: "trash-empty"
>  }

icon: "albumfolder-user-trash"

REPOSITORY
  R120 Plasma Workspace

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

To: Pitel, #plasma, #vdg
Cc: zzag, davidedmundson, anthonyfieroni, wsdfhjxc, ngraham, plasma-devel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D11292: [RFC] Ordered system tray v2

[Wojciech Stachurski]

wsdfhjxc added a comment.


  I think it would be better to place new items on the left side of the system 
tray, as this is closer to default behavior. Also, sometimes the visibility 
state is updated incorrectly after moving items back and forth (showing hidden 
state for visible items). Other than that, works very nicely. It's indeed much 
easier to drag the items around instead of clicking their arrows multiple 
times. Thanks for tackling this!

REPOSITORY
  R120 Plasma Workspace

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

To: Pitel, #plasma, #vdg
Cc: davidedmundson, anthonyfieroni, wsdfhjxc, ngraham, plasma-devel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D11292: [RFC] Ordered system tray v2

[Wojciech Stachurski]

wsdfhjxc added a comment.


  In D11292#224949 , @Pitel wrote:
  
  > I am aware of `getFixedItemId`, only forgot to mention it in summary. Right 
now it is not included mostly because it is another hack (but fixing stupid 
behavior of someone else so unavoidable) and I do not use DropBox, but if this 
gets any closer to landing, we should reintroduce it.
  
  
  As you said, that was a hack, and honestly I don't think it should be 
included because it's not the system tray applet's job to fix item 
identificators. Maybe some other component, e.g. related to StatusNotifier item 
registration, would be the place to look into. If not, the Dropbox client would 
be exceptionally placed always at the last position, but still there is a 
problem with previous entries stalled in the config property.

REPOSITORY
  R120 Plasma Workspace

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

To: Pitel
Cc: wsdfhjxc, ngraham, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D11292: [RFC] Ordered system tray v2

[Wojciech Stachurski]

wsdfhjxc added a comment.


  As for the getFixedItemId function (which was connected to an id trimming 
method in my example), it was because of Dropbox client's StatusNotifier item 
having some random number suffix. Without this, at every boot it was duplicated 
in the new config list-type property with a new id and the previous one was 
stalled because of its id mismatch. So, I just wanted to let you know that 
there might be a problem for items identified with some randomly generated 
names. Although, it seems to be extremely rare, as I've only noticed Dropbox 
client doing this.

REPOSITORY
  R120 Plasma Workspace

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

To: Pitel
Cc: wsdfhjxc, ngraham, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D11233: Ordered system tray

[Wojciech Stachurski]

wsdfhjxc updated this revision to Diff 29307.
wsdfhjxc added a comment.


  - Wait for the item id before inserting it into tray
  - Sort the tray items by categories and ids

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11233?vs=29275&id=29307

BRANCH
  consistent-systemtray

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

AFFECTED FILES
  applets/systemtray/package/contents/ui/items/StatusNotifierItem.qml
  applets/systemtray/package/contents/ui/main.qml

To: wsdfhjxc, #plasma_workspaces, #plasma, #vdg
Cc: broulik, ngraham, anthonyfieroni, Pitel, alexeymin, plasma-devel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D11233: Ordered system tray

[Wojciech Stachurski]

wsdfhjxc updated this revision to Diff 29275.
wsdfhjxc added a comment.


  - Squashed commit of the following:
  - Use theme based icons instead of char glyph labels for buttons
  - Use QRegularExpression instead of QRegExp

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11233?vs=29236&id=29275

BRANCH
  ordered-systemtray

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

AFFECTED FILES
  applets/systemtray/package/contents/config/main.xml
  applets/systemtray/package/contents/ui/ConfigEntries.qml
  applets/systemtray/package/contents/ui/items/AbstractItem.qml
  applets/systemtray/package/contents/ui/items/StatusNotifierItem.qml
  applets/systemtray/package/contents/ui/main.qml
  applets/systemtray/systemtray.cpp
  applets/systemtray/systemtray.h

To: wsdfhjxc, #plasma_workspaces, #plasma, #vdg
Cc: ngraham, anthonyfieroni, Pitel, alexeymin, plasma-devel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D11240: Introduce a new config entry for auto and shown item ordered list

[Wojciech Stachurski]

wsdfhjxc abandoned this revision.
wsdfhjxc added a comment.


  Sorry, I've messed something while playing with Arcanist.

REPOSITORY
  R120 Plasma Workspace

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

To: wsdfhjxc
Cc: plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D11240: Introduce a new config entry for auto and shown item ordered list

[Wojciech Stachurski]

wsdfhjxc created this revision.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
wsdfhjxc requested review of this revision.

REVISION SUMMARY
  Introduce a new abstract item ordered list position property
  
  Provide a method for registering an item in the ordered item list
  
  Place active items in the visible tray at their remembered positions
  
  Register an active item in the ordered list if it isn't there yet
  
  Try to update the item order and visibility if its position changes
  
  Add the new config entry reference
  
  Add global buffers for shown, hidden and ordered items
  
  Add methods for initializing, accessing and rearranging items
  
  Add methods for saving the items in config
  
  Initialize and swap table content data retrieving method
  
  Create a new table column with positioning controls
  
  Use item buffers to determinate the current item state
  
  Use proper methods for switching the item state
  
  Visually distinct disabled (yet present) items
  
  Fix indentation
  
  Implement regexp based item id trimming
  
  Make position property read only
  
  Remove unnecessary conditional skip
  
  Wait for proper item id before registering it
  
  Prevent pushing idless items into configuration list
  
  Reimplement support for dynamically updated configuration
  
  Move timer definition
  
  Remove redundant method calls
  
  Update the config page on delayed notifier appearance
  
  Change positioning buttons ids
  
  Change disabled property name to inactive
  
  Put forced model view refresh code into a function
  
  Add an option to forget inactive item entries
  
  Revert "Update the config page on delayed notifier appearance"
  
  This reverts commit 6ee666c777501a9d7c554c59375ae6b77d3d518a.
  
  Keep reloading the item config entries until idless items get ids
  
  Merge branch 'master' into systemtray-ordered
  
  Don't bother with hidden state if all items are force shown
  
  Fix item list view scroll after moving an item
  
  Use theme based icons instead of char glyph labels for buttons
  
  Use QRegularExpression instead of QRegExp

REPOSITORY
  R120 Plasma Workspace

BRANCH
  systemtray-ordered

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

AFFECTED FILES
  applets/systemtray/package/contents/config/main.xml
  applets/systemtray/package/contents/ui/ConfigEntries.qml
  applets/systemtray/package/contents/ui/items/AbstractItem.qml
  applets/systemtray/package/contents/ui/items/StatusNotifierItem.qml
  applets/systemtray/package/contents/ui/main.qml
  applets/systemtray/systemtray.cpp
  applets/systemtray/systemtray.h

To: wsdfhjxc
Cc: plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D11233: Ordered system tray

[Wojciech Stachurski]

wsdfhjxc created this revision.
wsdfhjxc added a reviewer: Plasma: Workspaces.
wsdfhjxc added a project: Plasma: Workspaces.
Restricted Application edited projects, added Plasma; removed Plasma: 
Workspaces.
Restricted Application added a subscriber: plasma-devel.
wsdfhjxc requested review of this revision.

REVISION SUMMARY
  Hello.
  
  I've switched to Plasma quite recently and I'm really liking it, but the one 
thing that irritates me a little bit is the system tray not remembering its 
item positions. This results in a random icon placement, mostly for 
applications other than embedded plasmoids (e.g. KMail, Dropbox, Redshift and 
other StatusNotifier items), at every boot. I guess this behavior doesn't seem 
to be an issue for most users, as there is not much community pressure on this 
one, but only a couple of topics I've found on Reddit and maybe another single 
one on KDE Community Forums. Still, the system tray feels more comfortable to 
use if it can be consistent all the time and if its items can be placed the way 
one wants to, as it may have influence on workflow and desktop's visual 
aesthetics. Furthermore, even Microsoft Windows allows system tray item order 
configuration.
  
  Having said that, I've done some research and managed to get the system tray 
positioning and configuration changed in a few places, so it remembers item 
positions and lets the user manually arrange the order of non-hidden items. In 
short, a new config list-type property is introduced to store all items with 
shown or auto visibility state and in user defined order. Items that are no 
longer loaded (and which are inactive at a time) are also remembered, so they 
can be positioned properly when they are loaded eventually. The elements tab in 
system tray settings contains positioning buttons for each non-hidden item in 
the list. The item list is refreshed every time a new plasmoid or 
StatusNotifier is registered or unregistered (well, maybe not every time, there 
is still some inconsistency). The overall result of the above changes is 
visible in the video attached below.
  
  F5749559: Ordered system tray.mp4 
  
  Now, I obviously do realize that this patch in its form is unacceptable to 
get into upstream, as its all hackish workaround to an existing system tray 
codebase. Honestly, my intention here is not really to contribute code of 
dubious quality, but to indicate the issue and provide some ideas and a working 
example to resolve it. I hope you will investigate this further at some point 
of Plasma development and maybe someone with appropriate knowledge and skills 
will consider implementing such a mechanism in a proper way, so the users could 
benefit from it.
  
  Thanks in advance!
  
  (BTW, both Kubuntu 17.10 with backports and KDE Neon Developer Edition seem 
to suffer from plasmashell crash after closing the system tray configuration 
window with elements tab beign open and clicking on any plasmoid in system tray 
afterwards. This is probably caused by bug 383828)

REPOSITORY
  R120 Plasma Workspace

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

AFFECTED FILES
  applets/systemtray/package/contents/config/main.xml
  applets/systemtray/package/contents/ui/ConfigEntries.qml
  applets/systemtray/package/contents/ui/items/AbstractItem.qml
  applets/systemtray/package/contents/ui/items/StatusNotifierItem.qml
  applets/systemtray/package/contents/ui/main.qml
  applets/systemtray/systemtray.cpp
  applets/systemtray/systemtray.h

To: wsdfhjxc, #plasma_workspaces
Cc: plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart