introduce a cursor theme kcm ported to QML, following the new
design guidelines, alongside generic components that will be reused for
modules that are mostly big grid views, such as look and feel.
BUG: 375106
hein |
Plasma |
introduce a cursor theme kcm ported to QML, following the new
design guidelines, alongside generic components that will be reused for
modules that are mostly big grid views, such as look and feel.
BUG: 375106
tested on different devices with different dpi
No Linters Available |
No Unit Test Coverage |
Even though that window is narrow, it looks like we have room for another column; we should use it. If this view is capable of resizing itself to fit in very narrow windows, that's great, and it will make a bit difference in letting System Settings be more compact over time, which is important because the default layout has an omnipresent left sidebar that takes up space.
is the case of just few pixels too little to do a second column, to fix it it would need for thumbnails to be resizable...
perhaps i should make thumbnails flexible when the view is small enough and wouldn't let 2 columns
- There is no white background to the list view
the white background depends from https://phabricator.kde.org/D8526
Does it add a horizontal scrollbar when the width is not enough for all the pushbuttons, like in the QWidget-based KCM (screenshot below)?
this screenshot if from the old module, you have to actually unistall it or will keep loading that one
Does it add a horizontal scrollbar when the width is not enough for all the pushbuttons, like in the QWidget-based KCM (screenshot below)?
this screenshot if from the old module, you have to actually unistall it or will keep loading that one
I know. The question was about the new KCM. Some buttons may become hard to discover if there is no scroolbar anymore.
A scrollbar to show hidden buttons is a pretty bad UX. We should aim for something better in the new version. Perhaps the buttons should lose their text and become icons-only when the window is too narrow to show them all with icons and text.
put the whole logic for cell sizes and the background painting as well in a
separed control (todo: will then need to be different in plasma mobile)
kcmcontrols/GridView.qml | ||
---|---|---|
32 ↗ | (On Diff #22649) | It never ever makes sense for an item's implicitSize to be based on the parent's current size. It's up to the parent to increase this item's actual width to be bigger than the implicitWidth |
kcms/cursortheme/xcursor/thememodel.cpp | ||
305 | why change these QStringLiterals? Is a rebase needed? | |
321 | This seems unrelated, adn I'm not sure is right. |
kcms/cursortheme/xcursor/thememodel.cpp | ||
---|---|---|
305 | yeah, it's so long this review is open that doesn't merge cleanly with master anymore :/ |
I don't think we can put the components into plasma-desktop, especially SimpleKCM if you really intend it to be used by all KCMs as root item. That would make any KCM requiring something like this depend on plasma-desktop. We have many KCMs in frameworks (e.g. KIO and Sonnet) which can't depend on plasma-desktop. Even working out the tiering in KF5 itself may be a little tricky.
Related to the above: We also have various cases where applications bring up KCMs via kcmshell, e.g. KIO's Web Shortcuts KCM. That means also at runtime none of this can depend on plasma-desktop in any way. KCMs need to work outside of Plasma with a sane dep tree. Styling needs to work outside of Plasma and Breeze, too.
Also - not strictly related to this review, but I did testing for D8911 via kcmshell5, and there's an enormous amout of QML ReferenceErrors going on with Qt Quick KCM code currently.