D8692: QML mouse cursor KCM and components

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit R119:bd95edf3abd8: QML mouse cursor KCM and components (authored by mart). CHANGED PRIOR TO COMMIT

D8692: QML mouse cursor KCM and components

mart updated this revision to Diff 22884. mart added a comment. - port to frameworks-kcmcontrols REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8692?vs=22829=22884 BRANCH kcm-redesign/cursorTheme REVISION DETAIL

D8692: QML mouse cursor KCM and components

hein added a comment. 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.

D8692: QML mouse cursor KCM and components

hein added a comment. 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

D8692: QML mouse cursor KCM and components

mart updated this revision to Diff 22820. mart added a comment. - remove spurious changes to resync with master - use qqc1 buttons for the icon - adapt to new scrollview behavior - base components: SimpleKCM GridViewKCM REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE

D8692: QML mouse cursor KCM and components

mart updated this revision to Diff 22829. mart added a comment. - correctly manage apply REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8692?vs=22820=22829 BRANCH kcm-redesign/cursorTheme REVISION DETAIL https://phabricator.kde.org/D8692

D8692: QML mouse cursor KCM and components

hein requested changes to this revision. hein added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kcmcursortheme.cpp:150 > +emit selectedThemeRowChanged(); > +setNeedsSave(true); > +updateSizeComboBox(); Incorrect Apply button management >

D8692: QML mouse cursor KCM and components

mart updated this revision to Diff 22658. mart added a comment. - restore Q_DECL_OVERRIDE REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8692?vs=22656=22658 BRANCH kcm-redesign/cursorTheme REVISION DETAIL https://phabricator.kde.org/D8692

D8692: QML mouse cursor KCM and components

mart updated this revision to Diff 22656. mart added a comment. - Merge branch 'master' into kcm-redesign/cursorTheme - remove spurious changes to resync with master REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8692?vs=22649=22656 BRANCH

D8692: QML mouse cursor KCM and components

mart added inline comments. INLINE COMMENTS > davidedmundson wrote in thememodel.cpp:305 > why change these QStringLiterals? Is a rebase needed? yeah, it's so long this review is open that doesn't merge cleanly with master anymore :/ REPOSITORY R119 Plasma Desktop REVISION DETAIL

D8692: QML mouse cursor KCM and components

davidedmundson added inline comments. INLINE COMMENTS > GridView.qml:32 > + > +implicitWidth: Math.max( Math.min(parent.width, cellWidth * 2 + > scroll.scrollBarSpace) > +, Math.floor(view.availableWidth / cellWidth) * cellWidth + > scroll.scrollBarSpace + 4) It never ever

D8692: QML mouse cursor KCM and components

mart updated this revision to Diff 22649. mart added a comment. - don't implement own background REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8692?vs=22534=22649 BRANCH kcm-redesign/cursorTheme REVISION DETAIL

D8692: QML mouse cursor KCM and components

mart updated this revision to Diff 22534. mart added a comment. - two rows toolbar if needed REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8692?vs=22529=22534 BRANCH kcm-redesign/cursorTheme REVISION DETAIL https://phabricator.kde.org/D8692

D8692: QML mouse cursor KCM and components

ngraham added a comment. In https://phabricator.kde.org/D8692#168596, @mart wrote: > or perhaps relayout to 2 lines... Yes, that would work too. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D8692 To: mart, #plasma Cc: aspotashev, januz,

D8692: QML mouse cursor KCM and components

mart updated this revision to Diff 22529. mart added a comment. - internal GridView implementation 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) REPOSITORY R119 Plasma Desktop

D8692: QML mouse cursor KCM and components

mart added a comment. In https://phabricator.kde.org/D8692#168581, @ngraham wrote: > 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

D8692: QML mouse cursor KCM and components

ngraham added a comment. 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. REPOSITORY R119

D8692: QML mouse cursor KCM and components

aspotashev added a comment. >> 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

D8692: QML mouse cursor KCM and components

mart added a comment. In https://phabricator.kde.org/D8692#168408, @aspotashev wrote: > Does it add a horizontal scrollbar when the width is not enough for all the pushbuttons, like in the QWidget-based KCM (screenshot below)? F5494095: Screenshot_20171116_143922.png

D8692: QML mouse cursor KCM and components

aspotashev added a comment. Does it add a horizontal scrollbar when the width is not enough for all the pushbuttons, like in the QWidget-based KCM (screenshot below)? F5494095: Screenshot_20171116_143922.png REPOSITORY R119 Plasma Desktop REVISION

D8692: QML mouse cursor KCM and components

mart added a comment. ping REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D8692 To: mart, #plasma Cc: januz, ngraham, subdiff, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D8692: QML mouse cursor KCM and components

ngraham edited the summary of this revision. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D8692 To: mart, #plasma Cc: januz, ngraham, subdiff, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D8692: QML mouse cursor KCM and components

mart added a comment. In https://phabricator.kde.org/D8692#165286, @subdiff wrote: > - Open the kcm with kcmshell5 kcm_cursortheme opens it in a very small view with only one column can you try it now with the last commit? REPOSITORY R119 Plasma Desktop REVISION DETAIL

D8692: QML mouse cursor KCM and components

mart updated this revision to Diff 22142. mart added a comment. - ensure at least 2 columns are visible REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8692?vs=22026=22142 BRANCH kcm-redesign/cursorTheme REVISION DETAIL

D8692: QML mouse cursor KCM and components

mart added a comment. In https://phabricator.kde.org/D8692#165286, @subdiff wrote: > - Open the kcm with kcmshell5 kcm_cursortheme opens it in a very small view with only one column is the case of just few pixels too little to do a second column, to fix it it would need for

D8692: QML mouse cursor KCM and components

ngraham added a comment. Even though that window is narrow, 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 will make a bit difference in letting System Settings be more compact over time, which is

D8692: QML mouse cursor KCM and components

subdiff added a comment. - Open the kcm with kcmshell5 kcm_cursortheme opens it in a very small view with only one column - There is no white background to the list view REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D8692 To: mart, #plasma Cc: subdiff,

D8692: QML mouse cursor KCM and components

mart updated this revision to Diff 22026. mart added a comment. - make use of the separed kcm modules - use Q_DECL_OVERRIDE REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8692?vs=22024=22026 BRANCH kcm-redesign/cursorTheme REVISION DETAIL

D8692: QML mouse cursor KCM and components

mart updated this revision to Diff 22024. mart added a comment. - use Q_DECL_OVERRIDE REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8692?vs=22023=22024 BRANCH kcm-redesign/cursorTheme REVISION DETAIL https://phabricator.kde.org/D8692

D8692: QML mouse cursor KCM and components

mart created this revision. mart added a reviewer: Plasma. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. REVISION SUMMARY introduce a cursor theme kcm ported to QML, following the new design guidelines, alongside generic components