D19745: Fix system tray UI/UX & refactor

ngraham added a comment. @ratijastk please feel free to submit another patch that does the same thing, but addresses @broulik's concerns. Thanks! REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D19745 To: ratijastk, #vdg, #plasma, broulik, mart, hein,

D19745: Fix system tray UI/UX & refactor

broulik added a comment. Fyi I reverted this change now https://cgit.kde.org/plasma-workspace.git/commit/?id=2594eb1b33b56ec7e0e8eb6c37acb1c7f2a5a1ff REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D19745 To: ratijastk, #vdg, #plasma, broulik, mart, hein,

D19745: Fix system tray UI/UX & refactor

broulik added a comment. Will you address the broken event delivery to the compact applet item? INLINE COMMENTS > ratijastk wrote in AbstractItem.qml:22 > Not exactly. But old layouts used to have a load of restrictions, for sure. "Old layouts"? You only use `Layouts 1.2` features as far as

D19745: Fix system tray UI/UX & refactor

ratijastk added inline comments. INLINE COMMENTS > broulik wrote in AbstractItem.qml:22 > You sure this high version number is neccessary? Not exactly. But old layouts used to have a load of restrictions, for sure. > broulik wrote in AbstractItem.qml:30 > Avoid doing negated disjunctions for

D19745: Fix system tray UI/UX & refactor

ngraham added a comment. @ratijastk, are you interested in sending a follow-up patch to address @broulik's comments? REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D19745 To: ratijastk, #vdg, #plasma, broulik, mart, hein, davidedmundson Cc: aacid, davidre,

D19745: Fix system tray UI/UX & refactor

broulik added a comment. Mouse buttons aren't propagated to the applet anymore, e.g. middle click on volume icon to mute no longer works. INLINE COMMENTS > AbstractItem.qml:22 > import QtQuick 2.1 > +import QtQuick.Layouts 1.12 > import org.kde.plasma.core 2.0 as PlasmaCore You sure this

D19745: Fix system tray UI/UX & refactor

ngraham added a comment. Thanks again. Very nice patch. May it be the first of many! :) REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D19745 To: ratijastk, #vdg, #plasma, broulik, mart, hein, davidedmundson Cc: aacid, davidre, davidedmundson, ngraham,

D19745: Fix system tray UI/UX & refactor

This revision was automatically updated to reflect the committed changes. Closed by commit R120:a73aa33028ef: Fix system tray UI/UX refactor (authored by ratijastk, committed by ngraham). REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE

D19745: Fix system tray UI/UX & refactor

ratijastk added a comment. @davidedmundson Thanks. It is my first contribution to KDE/Plasma, so I **do hope** that I don't have a commit access. INLINE COMMENTS > davidedmundson wrote in CompactApplet.qml:34 > You will enter this branch if the system tray is on the left. > > It

D19745: Fix system tray UI/UX & refactor

ngraham added a subscriber: aacid. ngraham added a comment. In D19745#466760 , @davidedmundson wrote: > > Please, check whether 'LayoutMirroring' works properly. > > To test layout mirroring run "plasmashell --reverse" > Layouts will auto

D19745: Fix system tray UI/UX & refactor

davidedmundson added a comment. > Please, check whether 'LayoutMirroring' works properly. To test layout mirroring run "plasmashell --reverse" Layouts will auto handle most things, so things should be fine. Do you have commit access? REPOSITORY R120 Plasma Workspace BRANCH

D19745: Fix system tray UI/UX & refactor

davidedmundson accepted this revision. davidedmundson added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > ratijastk wrote in CompactApplet.qml:34 > As far as I remember, one of the branches never got executed in my > environment. Could you point to specific

D19745: Fix system tray UI/UX & refactor

ratijastk added a comment. Bump (again) REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D19745 To: ratijastk, #vdg, #plasma, broulik, mart, hein Cc: davidre, davidedmundson, ngraham, ndavis, anthonyfieroni, plasma-devel, jraleigh, GB_2, ragreen, Pitel,

D19745: Fix system tray UI/UX & refactor

ratijastk added a comment. Double checked on my system. Nothing bad happened so far. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D19745 To: ratijastk, #vdg, #plasma, broulik, mart, hein Cc: davidre, davidedmundson, ngraham, ndavis, anthonyfieroni,

D19745: Fix system tray UI/UX & refactor

ratijastk updated this revision to Diff 58123. ratijastk added a comment. Rebase on most recent master REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D19745?vs=53851=58123 BRANCH fix-system-tray REVISION DETAIL

D19745: Fix system tray UI/UX & refactor

ratijastk added a comment. ~~Even with extra-cmake-modules 5.58, kcoreaddons needs to be updated as well... work in progress... please, be patient: i have autism^W slow hardware.~~ 5.58.0 releases just rolled out, upgrading... REPOSITORY R120 Plasma Workspace REVISION DETAIL

D19745: Fix system tray UI/UX & refactor

davidre added a comment. In D19745#465569 , @ratijastk wrote: > So, I've merged done git rebase on master — just one conflicting line, rewritten after me in a better way. But now I have no way to test it, because CMake Error `Could not find a

D19745: Fix system tray UI/UX & refactor

ratijastk added inline comments. INLINE COMMENTS > davidedmundson wrote in main.qml:66 > Not really. > Using objectNames is a bit of an anti pattern, especially when QML has so the > built-in component scope hierachy. > > We use it in the system tray already, and it's arguably no worse than

D19745: Fix system tray UI/UX & refactor

ratijastk added a comment. So, I've merged done git rebase on master — just one conflicting line, rewritten after me in a better way. But now I have no way to test it, because CMake Error `Could not find a configuration file for package "ECM" that is compatible with requested version

D19745: Fix system tray UI/UX & refactor

anthonyfieroni added inline comments. INLINE COMMENTS > davidedmundson wrote in PlasmoidItem.qml:45 > Huh? how did this work before? Applets do it by itself, when it not do, it does not work like Weather Widget embed in systray. REPOSITORY R120 Plasma Workspace REVISION DETAIL

D19745: Fix system tray UI/UX & refactor

davidedmundson added a comment. I love use of layouts for everything, so in general ++ INLINE COMMENTS > CompactApplet.qml:34 > subText: plasmoid.toolTipSubText > -location: if (plasmoid.parent && plasmoid.parent.parent.objectName === > "hiddenTasksColumn" && plasmoid.location !==

D19745: Fix system tray UI/UX & refactor

ratijastk added a comment. Oh, nevermind. I forgot I should be using `arc` indeed. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D19745 To: ratijastk, #vdg, #plasma, broulik, mart, hein Cc: ngraham, ndavis, anthonyfieroni, plasma-devel, jraleigh, GB_2,

D19745: Fix system tray UI/UX & refactor

ngraham added a comment. What you need to do is rebase the branch for your patch on current master: `git pull --rebase origin master` Then fix up the merge conflicts. Then run `arc diff` again. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D19745

D19745: Fix system tray UI/UX & refactor

ratijastk added a comment. My parent commit was `169238848136`; current master is `f8d5e1a2`. While on master, I am doing `git diff --summary 169238848136`. There is no intersection in changed files with my pull request. Still patch (as you aforementioned) can not be applied to master

D19745: Fix system tray UI/UX & refactor

ngraham added a comment. I will review this, sorry it's taken so much time. I had to solve a minor merge conflict when rebasing this on master. Could you do that and update the patch? Thanks! REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D19745 To:

D19745: Fix system tray UI/UX & refactor

ratijastk added a comment. I'm afraid the more we wait, the more code base is diverging, which will require more patches for this pull request to be accepted. Btw, I'm still stuck with pinning old version in my package manager. REPOSITORY R120 Plasma Workspace REVISION DETAIL

D19745: Fix system tray UI/UX & refactor

ngraham added reviewers: mart, hein. ngraham added a comment. A few more days of waiting may be required since most of the Plasma developers are quite busy right now. But it will get reviewed, don't worry! REPOSITORY R120 Plasma Workspace REVISION DETAIL

D19745: Fix system tray UI/UX & refactor

ratijastk added a comment. Hello, Any progress on this one? Does it usually take long to review? Sorry for impatience. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D19745 To: ratijastk, #vdg, #plasma, broulik Cc: ngraham, ndavis, anthonyfieroni,

D19745: Fix system tray UI/UX & refactor

ratijastk added a comment. In D19745#430958 , @ngraham wrote: > Pretty nice stuff! While you're at it, how about implementing a UI change so that while the popup is open, its contents change as you slide the mouse along the icons (but only when

D19745: Fix system tray UI/UX & refactor

ngraham added a comment. Pretty nice stuff! While you're at it, how about implementing a UI change so that while the popup is open, its contents change as you slide the mouse along the icons (but only when there's a single row/column)? REPOSITORY R120 Plasma Workspace REVISION DETAIL

D19745: Fix system tray UI/UX & refactor

ndavis added a comment. +1 to this. Fitts' law comes to mind. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D19745 To: ratijastk, #vdg, #plasma, broulik Cc: ndavis, anthonyfieroni, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai,

D19745: Fix system tray UI/UX & refactor

ratijastk added a comment. In D19745#430695 , @anthonyfieroni wrote: > Video or screenshot will be helpful. Here they are, "debug" mode via colored rectangles with `anchors.fill: parent`. F6690427: old.mp4

D19745: Fix system tray UI/UX & refactor

anthonyfieroni added reviewers: VDG, Plasma, broulik. anthonyfieroni added a comment. Video or screenshot will be helpful. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D19745 To: ratijastk, #vdg, #plasma, broulik Cc: anthonyfieroni, plasma-devel,

D19745: Fix system tray UI/UX & refactor

ratijastk created this revision. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. ratijastk requested review of this revision. REVISION SUMMARY System tray widget had the following UX problem: Icons are laid out in a Flow QML layout, using minimal amount of space,