D12459: [Icon KCM] Port to new design

kossebau added inline comments. INLINE COMMENTS > main.cpp:393 > +for (int i = 0; i < KIconLoader::LastGroup; i++) { > +KIconLoader::emitChange(KIconLoader::Group(i)); > + Is KIconLoader::emitChange also for kde4 apps only? Asking as KIconLoader of KIconThemes seems to have

D12459: [Icon KCM] Port to new design

This revision was automatically updated to reflect the committed changes. Closed by commit R119:9496728fcb6c: [Icon KCM] Port to new design (authored by broulik). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D12459?vs=33086=33206#toc REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST

D12459: [Icon KCM] Port to new design

hein accepted this revision. hein added a comment. This revision is now accepted and ready to land. Go go go REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D12459 To: broulik, #plasma, #vdg, hein Cc: lueck, schwarzer, hein, zzag, abetts, ngraham,

D12459: [Icon KCM] Port to new design

schwarzer added inline comments. INLINE COMMENTS > broulik wrote in iconsmodel.cpp:41 > The parent is valid for sub levels in a tree but this is a flat model so > there will never be a parent as we only have a root layer. Ah, ok. Thanks for the explanation. :) REPOSITORY R119 Plasma Desktop

D12459: [Icon KCM] Port to new design

broulik added inline comments. INLINE COMMENTS > schwarzer wrote in iconsmodel.cpp:41 > Is this logic the right way around? The parent is valid for sub levels in a tree but this is a flat model so there will never be a parent as we only have a root layer. REPOSITORY R119 Plasma Desktop

D12459: [Icon KCM] Port to new design

schwarzer added inline comments. INLINE COMMENTS > iconsmodel.cpp:41 > +{ > +if (parent.isValid()) { > +return 0; Is this logic the right way around? REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D12459 To: broulik, #plasma, #vdg Cc: lueck,

D12459: [Icon KCM] Port to new design

schwarzer added a subscriber: lueck. schwarzer added a comment. In D12459#253942 , @broulik wrote: > @schwarzer I'm changing the catalog name to be more consistent with the other KCMs, is that an issue? There have been some discussions

D12459: [Icon KCM] Port to new design

broulik added a subscriber: schwarzer. broulik added a comment. @schwarzer I'm changing the catalog name to be more consistent with the other KCMs, is that an issue? REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D12459 To: broulik, #plasma, #vdg Cc:

D12459: [Icon KCM] Port to new design

hein added a comment. Model looks good! :) REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D12459 To: broulik, #plasma, #vdg Cc: hein, zzag, abetts, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D12459: [Icon KCM] Port to new design

broulik updated this revision to Diff 33086. broulik added a comment. - Use custom model class REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12459?vs=32871=33086 REVISION DETAIL https://phabricator.kde.org/D12459 AFFECTED FILES

D12459: [Icon KCM] Port to new design

hein added a comment. In general: This code would be cleaner if it wasn't using QStandardItemModel but just a QAbstractListModel subclass. Then stuff like the "selected theme index" could use either a role or a QItemSelectionModel, and the role enum wouldn't need to live outside of the

D12459: [Icon KCM] Port to new design

zzag added a comment. In D12459#252290 , @broulik wrote > Looks just like compression artefact in the screenshot. The theme cannot be uninstalled as it's system-wide and the thin lines seemed to throw it off. Yeah, it makes sense.

D12459: [Icon KCM] Port to new design

broulik added a comment. In D12459#252269 , @zzag wrote: > Delete button is not visible in the attached video at 0:14 (when you hover Breeze). Looks just like compression artefact in the screenshot. The theme cannot be uninstalled as

D12459: [Icon KCM] Port to new design

zzag added a comment. Delete button is not visible in the attached video at 0:14 (when you hover Breeze). REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D12459 To: broulik, #plasma, #vdg Cc: zzag, abetts, ngraham, plasma-devel, ragreen, Pitel, ZrenBot,

D12459: [Icon KCM] Port to new design

broulik updated this revision to Diff 32871. broulik retitled this revision from "WIP: [Icon KCM] Port to new design" to "[Icon KCM] Port to new design". broulik edited the summary of this revision. broulik edited the test plan for this revision. broulik added a comment. - Kill old KCM code