D24846: Port kcm icons to kconfigxt

This revision was automatically updated to reflect the committed changes. Closed by commit R119:38ba450787ae: Port kcm icons to kconfigxt (authored by bport). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D24846?vs=72189=72446#toc REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST

D24846: Port kcm icons to kconfigxt

bport updated this revision to Diff 72189. bport added a comment. Update according to ervin feedback REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24846?vs=71956=72189 REVISION DETAIL https://phabricator.kde.org/D24846 AFFECTED FILES

D24846: Port kcm icons to kconfigxt

ervin added a comment. Just a couple of stylistic issues left AFAICT INLINE COMMENTS > iconsmodel.h:47 > public: > -IconsModel(QObject *parent); > +IconsModel(QObject *parent, IconsSettings *iconsSettings); > ~IconsModel() override; Kind of a nitpick in this context, but

D24846: Port kcm icons to kconfigxt

bport updated this revision to Diff 71956. bport added a comment. Remone unused include REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24846?vs=71668=71956 REVISION DETAIL https://phabricator.kde.org/D24846 AFFECTED FILES

D24846: Port kcm icons to kconfigxt

crossi added inline comments. INLINE COMMENTS > iconsmodel.cpp:29 > #include > +#include > include not used REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D24846 To: bport, ervin, mart, #plasma, crossi Cc: broulik, plasma-devel, LeGast00n,

D24846: Port kcm icons to kconfigxt

bport updated this revision to Diff 71668. bport added a comment. Add theme to kconfigXT REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24846?vs=71127=71668 REVISION DETAIL https://phabricator.kde.org/D24846 AFFECTED FILES

D24846: Port kcm icons to kconfigxt

bport updated this revision to Diff 71127. bport marked 5 inline comments as done. bport added a comment. Take in consideration ervin feedback REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24846?vs=71117=71127 REVISION DETAIL

D24846: Port kcm icons to kconfigxt

bport updated this revision to Diff 71117. bport added a comment. Take in consideration feedbacks REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24846?vs=71110=71117 REVISION DETAIL https://phabricator.kde.org/D24846 AFFECTED FILES

D24846: Port kcm icons to kconfigxt

bport updated this revision to Diff 71110. bport added a comment. Fix REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24846?vs=70452=71110 REVISION DETAIL https://phabricator.kde.org/D24846 AFFECTED FILES kcms/icons/CMakeLists.txt

D24846: Port kcm icons to kconfigxt

ervin requested changes to this revision. ervin added a comment. This revision now requires changes to proceed. Looks like you didn't address a faire number of comments, or they ended up in the wrong commit (by the look of D24847 ). INLINE COMMENTS >

D24846: Port kcm icons to kconfigxt

bport updated this revision to Diff 70452. bport added a comment. Take in consideration feedback REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24846?vs=69036=70452 REVISION DETAIL https://phabricator.kde.org/D24846 AFFECTED FILES

D24846: Port kcm icons to kconfigxt

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > main.cpp:101 > > -connect(m_model, ::selectedThemeChanged, this, [this] { > -m_selectedThemeDirty = true; > -setNeedsSave(true); > -

D24846: Port kcm icons to kconfigxt

bport updated this revision to Diff 69036. bport added a comment. Take in consideration feedback (cache kicon call) Port to ManagedConfigModule REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24846?vs=68498=69036 REVISION DETAIL

D24846: Port kcm icons to kconfigxt

ervin requested changes to this revision. ervin added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > broulik wrote in main.cpp:141 > Does this `KIconTheme` instance need caching? I recall creating those parses > a tonne of files and directories and is quite

D24846: Port kcm icons to kconfigxt

bport added inline comments. INLINE COMMENTS > broulik wrote in IconSizePopup.qml:45 > Do you not need to leave this (or a similar) connect in place? This one is replaced (and that fix bug described above by call onVisibleChange) and with kconfigxt property handling > broulik wrote in

D24846: Port kcm icons to kconfigxt

broulik added inline comments. INLINE COMMENTS > main.cpp:141 > { > -return > KIconLoader::global()->theme()->querySizes(static_cast(group)); > +KIconTheme theme(m_model->selectedTheme()); > +return theme.querySizes(static_cast(group)); Does this `KIconTheme` instance need

D24846: Port kcm icons to kconfigxt

bport created this revision. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. bport requested review of this revision. REVISION SUMMARY Also fix 2 bugs: - keyboard navigation on size category didn't work as expected, we didn't saved what we see (open size