Combines settings of SNI icons and plasmoids in one list. Ability to disable whole section is removed.
Initial idea was proposed by Nicolas Fella (D22176).
BUG: 360307
FIXED-IN: 5.18.0
davidedmundson | |
broulik | |
ngraham | |
mart |
Plasma: Workspaces | |
Plasma |
Combines settings of SNI icons and plasmoids in one list. Ability to disable whole section is removed.
Initial idea was proposed by Nicolas Fella (D22176).
BUG: 360307
FIXED-IN: 5.18.0
No Linters Available |
No Unit Test Coverage |
Buildable 20964 | |
Build 20982: arc lint + arc unit |
This change is based on the idea from D22176.
It needs review and probably more testing. Is it visually correct?
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...
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 for all items in that section.
Finally, if it's possible to use alternating stripey background colors instead of lines between items, that's generally preferred for multi-column table-style views.
Also German locale strikes again: The shortcut label (Tastatur-Kurzbefehle) is too long here
I don't share this opinion.
A list with dozens of elements is a maze for users.
The categories seems quite helpful when looking for things and relieves the eye in marking breaks.
It really makes the Ui richer.
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. I don't really see a use for it.
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
Is it decided (D26530)? Correct me if I'm wrong, in Kirigami scrollbars are overlaying with transparent background, Kirigami just adds some paddings when needed. Anyway, to have scrollbar seprated it is better to just add one margin to the list (and remove "rightPadding" I added to header, section and list item).
OK, will change the icon. I will disable all items. Or I'll just remove this feature...
Finally, if it's possible to use alternating stripey background colors instead of lines between items, that's generally preferred for multi-column table-style views.
In plasmoidviewer all are white, I don't know why it is not the case in real usage. I will fix that.
OK, I will fix that. My idea is to get max size of all items in the column and apply it to all elements (like in real table).
It is not that it is the first feature in KDE that almost no one uses :) But I lean to the idea of removing it. It really does not make much sense to hide whole category, because it applies to all apps - even those you don't know yet. Definitely, it is confusing. For me, as a develop, it is hard to remove features, it break my heart :) From the other side, I understand that is is equally important to add features and to remove useless ones. Do we have any vote against/to keep this feature?
Note to myself: to remove this, you need to remove all usage of configuration properties: cfg_*Shown.
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.
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!
Finally, if it's possible to use alternating stripey background colors instead of lines between items, that's generally preferred for multi-column table-style views.
It looks bad with alternating background, list items are merged with sections:
fine for me too.
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 rely on that.
if that "no normal apps such as mediaplayers" is deemed an useful feature(which i'm not sure it is) , should probably be a checkbox that says exactly that, rather than having the concept of enabling or disabling a whole category
on the code side, i have mainly only one small gripe
applets/systemtray/package/contents/ui/ConfigEntries.qml | ||
---|---|---|
103 | this should never be necessary and will probably cause bugs. |
OK, I will remove this feature.
applets/systemtray/package/contents/ui/ConfigEntries.qml | ||
---|---|---|
103 | Yes, I don't like this neither. It is a workaround for a problem in Kirigami.AbstractListItem. It automatically calculates paddings when scrollbars are detected: I use plain QQC2.ScrollView, which attaches scrollbars to itself, not ListView. As a result scrollbars are not detected and padding is not added. I don't know how to fix that correctly without duplicating a code in ConfigEntries.qml. I can override leftPadding and rightPadding, but I prefer to use original logic (in case it changes in the future). Can you advice? |
When I try this out, here's what I see:
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 reloaded?
Ah, I see it now.
The alignment for items with shortcuts is a bit odd:
Maybe for the combobox item texts, try this:
Yes, I know. Shortcuts can be really long, you can have some crazy combinations like "Meta+Ctrl+Shift+Z" which is very wide. From the other side, it should not be that bad... I will work on this, let's see what can be achieved.
Maybe for the combobox item texts, try this:
- Shown when relevant
- Always shown
- Always hidden
- Disabled
OK, I forgot about this, I will change that.
Anything else to change/fix? When is the code freeze, this Friday?
applets/systemtray/package/contents/ui/ConfigEntries.qml | ||
---|---|---|
103 | I replaced this hack with explicit paddings settings |
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.
I saw that and hoped that it will apply automatically. I recompiled KDE (with qqc2-desktop-style), restarted but it is not working for me. Is there anything needed on my side to enable D26530?
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. :) I'll let Plasma folks do the final code review.
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
Also the text/icon sometimes overflows:
Should I use import QtQuick.Controls 2.5 as QQC2 or import org.kde.plasma.components 3.0 as PlasmaComponents is preferred? With PlasmaComponents there is no problem with overflow, but there is no frame.
Is the review (by other Plasma members) done? Please let me know when can I push changes :)
This change break settings for me due to currentValue in ConfigEntries.qml conflicting. It works if I rename the variable to something else