D26586: [SystemTray] Rework system tray settings

carewolf added a comment. In D26586#600864 , @kmaterka wrote: > In D26586#600853 , @carewolf wrote: > > > This change break settings for me due to currentValue in ConfigEntries.qml conflicting. It

D26586: [SystemTray] Rework system tray settings

kmaterka added a comment. In D26586#600853 , @carewolf wrote: > This change break settings for me due to currentValue in ConfigEntries.qml conflicting. It works if I rename the variable to something else Are you using Qt 5.14?

D26586: [SystemTray] Rework system tray settings

carewolf added a comment. This change break settings for me due to currentValue in ConfigEntries.qml conflicting. It works if I rename the variable to something else REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D26586 To: kmaterka, #plasma_workspaces,

D26586: [SystemTray] Rework system tray settings

This revision was automatically updated to reflect the committed changes. Closed by commit R120:e2bfeb160bad: [SystemTray] Rework system tray settings (authored by kmaterka). REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26586?vs=73504=73629

D26586: [SystemTray] Rework system tray settings

ngraham added a comment. Push it! Push it real good! :) REPOSITORY R120 Plasma Workspace BRANCH master REVISION DETAIL https://phabricator.kde.org/D26586 To: kmaterka, #plasma_workspaces, #plasma, davidedmundson, broulik, ngraham, mart Cc: ahiemstra, mart, meven, nicolasfella,

D26586: [SystemTray] Rework system tray settings

kmaterka added a comment. Is the review (by other Plasma members) done? Please let me know when can I push changes :) REPOSITORY R120 Plasma Workspace BRANCH master REVISION DETAIL https://phabricator.kde.org/D26586 To: kmaterka, #plasma_workspaces, #plasma, davidedmundson, broulik,

D26586: [SystemTray] Rework system tray settings

ngraham added a comment. Use the QQC2 ScrollView, and we should fix that elsewhere. REPOSITORY R120 Plasma Workspace BRANCH master REVISION DETAIL https://phabricator.kde.org/D26586 To: kmaterka, #plasma_workspaces, #plasma, davidedmundson, broulik, ngraham Cc: ahiemstra, mart,

D26586: [SystemTray] Rework system tray settings

kmaterka added a comment. In D26586#594236 , @nicolasfella wrote: > There's a minor problem (that I've seen in other place as well). When scrolling the content overflows the frame on the top a bit: Should I use `import QtQuick.Controls

D26586: [SystemTray] Rework system tray settings

nicolasfella added a comment. There's a minor problem (that I've seen in other place as well). When scrolling the content overflows the frame on the top a bit: See how the top frame is white while the side is blue F7883066: Screenshot_20200114_170622.png

D26586: [SystemTray] Rework system tray settings

kmaterka added a comment. In D26586#594211 , @ngraham wrote: > Some of the code looks a bit hairy to me, but I don't see anything catastrophic. :) What exactly? I can quickly fix/refactor that. REPOSITORY R120 Plasma Workspace BRANCH

D26586: [SystemTray] Rework system tray settings

ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. Yep, the freeze is on Friday. I think we can get this in before then. UI and behavior look good. Some of the code looks a bit hairy to me, but I don't see anything catastrophic. :)

D26586: [SystemTray] Rework system tray settings

kmaterka marked an inline comment as done. kmaterka added a comment. In D26586#593966 , @ahiemstra wrote: > There should be no need to explicitly specify paddings now that D26530 has landed. But there's a

D26586: [SystemTray] Rework system tray settings

ahiemstra added a comment. There should be no need to explicitly specify paddings now that D26530 has landed. But there's a bunch of workarounds that we still need to remove, so you may have some intermittent weird results. REPOSITORY R120 Plasma

D26586: [SystemTray] Rework system tray settings

kmaterka marked 2 inline comments as done. kmaterka added a comment. Anything else to change/fix? When is the code freeze, this Friday? INLINE COMMENTS > kmaterka wrote in ConfigEntries.qml:114 > Yes, I don't like this neither. It is a workaround for a problem in >

D26586: [SystemTray] Rework system tray settings

kmaterka updated this revision to Diff 73504. kmaterka added a comment. Replaced hack with explicit padding settings REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26586?vs=73470=73504 BRANCH master REVISION DETAIL

D26586: [SystemTray] Rework system tray settings

kmaterka updated this revision to Diff 73470. kmaterka added a comment. Fixes for column size, it is calculated dynamically now REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26586?vs=73414=73470 BRANCH master REVISION DETAIL

D26586: [SystemTray] Rework system tray settings

kmaterka added a comment. In D26586#593549 , @ngraham wrote: > The alignment for items with shortcuts is a bit odd: F7881593: Screenshot_20200113_130125.png Yes, I know. Shortcuts can be really

D26586: [SystemTray] Rework system tray settings

ngraham added a comment. Ah, I see it now. The alignment for items with shortcuts is a bit odd: F7881593: Screenshot_20200113_130125.png Maybe for the combobox item texts, try this: - Shown when relevant - Always shown - Always hidden

D26586: [SystemTray] Rework system tray settings

kmaterka added a comment. @mart @ngraham I see heated discussion in D26530 . What should I do with scrollbars? I can change it to something like this: F7880950: image.png REPOSITORY R120 Plasma Workspace

D26586: [SystemTray] Rework system tray settings

kmaterka updated this revision to Diff 73414. kmaterka added a comment. Removed feature that allowed to hide whole category. REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26586?vs=73395=73414 BRANCH master REVISION DETAIL

D26586: [SystemTray] Rework system tray settings

kmaterka added a comment. In D26586#593154 , @ngraham wrote: > There are no sections and everything's disabled. Hmm, there is one small change in the C++ model (category added), maybe you need to restart/logout so that C++ library is

D26586: [SystemTray] Rework system tray settings

ngraham added a comment. When I try this out, here's what I see: F7880888: Screenshot_20200113_072934.png There are no sections and everything's disabled. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D26586

D26586: [SystemTray] Rework system tray settings

kmaterka added a comment. In D26586#593114 , @mart wrote: > fine for me too. OK, I will remove this feature. INLINE COMMENTS > mart wrote in ConfigEntries.qml:114 > this should never be necessary and will probably cause bugs. > It looks

D26586: [SystemTray] Rework system tray settings

ngraham added a comment. In D26586#593114 , @mart wrote: > the reason for it in the beginning was t just be able to say "i don't want normal apps to be able to spam my systray" tough probably things aren't categorize well enough to be able to

D26586: [SystemTray] Rework system tray settings

mart added a comment. on the code side, i have mainly only one gripe INLINE COMMENTS > ConfigEntries.qml:114 > +Layout.fillHeight: true > +QQC2.ScrollBar.vertical: scrollView.QQC2.ScrollBar.vertical // > For Kirigami.AbstractListItem > this should never be

D26586: [SystemTray] Rework system tray settings

mart added a comment. In D26586#593025 , @kmaterka wrote: > Final decision: should I remove feature which allows to hide whole category? fine for me too. the reason for it in the beginning was t just be able to say "i don't want normal

D26586: [SystemTray] Rework system tray settings

kmaterka added a comment. Final decision: should I remove feature which allows to hide whole category? REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D26586 To: kmaterka, #plasma_workspaces, #plasma, davidedmundson, broulik, ngraham Cc: meven, nicolasfella,

D26586: [SystemTray] Rework system tray settings

kmaterka updated this revision to Diff 73395. kmaterka added a comment. Workaround for Kirigami.AbstractListItem REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26586?vs=73389=73395 BRANCH master REVISION DETAIL

D26586: [SystemTray] Rework system tray settings

kmaterka updated this revision to Diff 73389. kmaterka added a comment. Review fixes: Key shortcut header width Background Few other small changes REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26586?vs=73284=73389 BRANCH master REVISION

D26586: [SystemTray] Rework system tray settings

kmaterka added a comment. > The discussion about overlay vs non-overlay scrollbars is unrelated to whether or not to show a frame and background behind a scrollview. :) Just add the background for now I think. Oh, true, my mistake. It looks much better with background, thanks! >

D26586: [SystemTray] Rework system tray settings

ngraham added a comment. In D26586#592707 , @kmaterka wrote: > In D26586#592050 , @ngraham wrote: > > > Much better! The scrollview needs a frame around it though. You can do this by adding this to

D26586: [SystemTray] Rework system tray settings

kmaterka added a comment. In D26586#592050 , @ngraham wrote: > Much better! The scrollview needs a frame around it though. You can do this by adding this to it: Is it decided (D26530 )? Correct me if

D26586: [SystemTray] Rework system tray settings

nicolasfella added a comment. I'm not against keeping the separators, but the button to en/disable the whole section seems overkill to me. Chances are that the set of entries you want to have does not exactly align with the categories anyway REPOSITORY R120 Plasma Workspace REVISION

D26586: [SystemTray] Rework system tray settings

ngraham added a comment. I agree that separating the list into categories makes it a lot less visually overwhelming to look at. If @nicolasfella was saying that maybe we could remove the feature that allows disabling all elements within a section at once, that I might be able to get behind.

D26586: [SystemTray] Rework system tray settings

meven added a comment. In D26586#592053 , @nicolasfella wrote: > I would just kill the per-section control. There's no point to that IMHO I don't share this opinion. A list with dozens of elements is a maze for users. The categories

D26586: [SystemTray] Rework system tray settings

nicolasfella added a comment. Also German locale strikes again: The shortcut label (Tastatur-Kurzbefehle) is too long here F7877547: Screenshot_20200111_182731.PNG REPOSITORY R120 Plasma Workspace REVISION DETAIL

D26586: [SystemTray] Rework system tray settings

nicolasfella added a comment. I would just kill the per-section control. There's no point to that IMHO REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D26586 To: kmaterka, #plasma_workspaces, #plasma, davidedmundson, broulik, ngraham Cc: nicolasfella,

D26586: [SystemTray] Rework system tray settings

ngraham added a comment. Also for toggling individual sections, I would recommend making the button say "disable all" or "hide all" (depending on what's most accurate) and use the `view-hidden` icon. Then when all items in that category are disabled/hidden, disable all controls and labels

D26586: [SystemTray] Rework system tray settings

ngraham added a comment. Much better! The scrollview needs a frame around it though. You can do this by adding this to it: Component.onCompleted: scrollview.background.visible = true; Why this isn't the default, I have no idea... REPOSITORY R120 Plasma Workspace REVISION

D26586: [SystemTray] Rework system tray settings

kmaterka added a comment. This changes is based on the idea from D22176 . It needs review and probably more testing. Is it visually correct? REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D26586 To: kmaterka,

D26586: [SystemTray] Rework system tray settings

kmaterka created this revision. kmaterka added reviewers: Plasma: Workspaces, Plasma, davidedmundson, broulik, ngraham. kmaterka added a project: Plasma. kmaterka requested review of this revision. REVISION SUMMARY Combines settings of SNI icons and plasmoids in one list. BUG: 360307