D26386: Kicker/RecentDocuments: add icons to actions

This revision was automatically updated to reflect the committed changes. Closed by commit R120:4dca72bdf59a: Kicker/RecentDocuments: add icons to actions (authored by meven). REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26386?vs=73035&id=73036 REV

D26386: Kicker/RecentDocuments: add icons to actions

meven updated this revision to Diff 73035. meven added a comment. This revision is now accepted and ready to land. Make icon non-optionnal in Kicker::createActionItem, add a missing icon REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26386?vs=72804

D26386: Kicker/RecentDocuments: add icons to actions

hein accepted this revision. hein added a comment. Good stuff! REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D26386 To: meven, #plasma, ngraham, hein Cc: hein, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBo

D26386: Kicker/RecentDocuments: add icons to actions

meven planned changes to this revision. meven added inline comments. INLINE COMMENTS > actionlist.cpp:62 > > +if (!icon.isEmpty()) { > +map[QStringLiteral("icon")] = icon; Remove this if > recentcontactsmodel.cpp:93 > > -actionList << Kicker::createActionItem(i18n("Show

D26386: Kicker/RecentDocuments: add icons to actions

meven added a comment. @hein is it fine for you ? REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D26386 To: meven, #plasma, ngraham, hein Cc: hein, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, a

D26386: Kicker/RecentDocuments: add icons to actions

ngraham accepted this revision. ngraham added a comment. Nice! REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D26386 To: meven, #plasma, ngraham, hein Cc: hein, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBo

D26386: Kicker/RecentDocuments: add icons to actions

meven updated this revision to Diff 72804. meven marked 2 inline comments as done. meven added a comment. Add icon identity for showContactInfo action REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26386?vs=72755&id=72804 BRANCH arcpatch-D26386_

D26386: Kicker/RecentDocuments: add icons to actions

ngraham added inline comments. INLINE COMMENTS > meven wrote in contactentry.cpp:117 > I missing an icon for this action, I am open for suggestion. Maybe `user-identity` or `identity`, depending on what it does REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D2

D26386: Kicker/RecentDocuments: add icons to actions

meven added inline comments. INLINE COMMENTS > contactentry.cpp:117 > > -actionList << Kicker::createActionItem(i18n("Show Contact > Information..."), QStringLiteral("showContactInfo")); > +actionList << Kicker::createActionItem(i18n("Show Contact > Information..."), QString(), QStrin

D26386: Kicker/RecentDocuments: add icons to actions

meven updated this revision to Diff 72755. meven added a comment. Use view-hidden icon for Hide Application action for consistency REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26386?vs=72754&id=72755 BRANCH arcpatch-D26386_1 REVISION DETAIL

D26386: Kicker/RecentDocuments: add icons to actions

meven updated this revision to Diff 72754. meven added a comment. Clean unwanted change REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26386?vs=72752&id=72754 BRANCH arcpatch-D26386_1 REVISION DETAIL https://phabricator.kde.org/D26386 AFFECT

D26386: Kicker/RecentDocuments: add icons to actions

meven updated this revision to Diff 72752. meven added a comment. Change signature of Kicker::createActionItem and update all its users REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26386?vs=72697&id=72752 BRANCH arcpatch-D26386_1 REVISION DET

D26386: Kicker/RecentDocuments: add icons to actions

meven added a comment. In D26386#587169 , @hein wrote: > It should be `iconName` after `label`. I like this suggestion. It entails a lot of error-prone code change. Will do. REPOSITORY R120 Plasma Workspace REVISION DETAIL https:

D26386: Kicker/RecentDocuments: add icons to actions

hein requested changes to this revision. hein added a comment. This revision now requires changes to proceed. It should be `iconName` after `label`. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D26386 To: meven, #plasma, ngraham, hein Cc: hein, plasma-deve

D26386: Kicker/RecentDocuments: add icons to actions

ngraham added a comment. Yeah, it definitely bloats this diff, but maybe it would make subsequent ones smaller and cleaner? #plasma folks, what do you think? REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D26386

D26386: Kicker/RecentDocuments: add icons to actions

meven added a comment. I am not too fond of this version growing the number of parameters of Kicker::createActionItem, and given its third parameter before icon is QVariant this becomes error-prone. There are a lot more usage of this method that I haven't yet updated, if we want to follow

D26386: Kicker/RecentDocuments: add icons to actions

meven updated this revision to Diff 72697. meven added a comment. Add an icon parameter to Kicker::createActionItem REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26386?vs=72673&id=72697 BRANCH arcpatch-D26386 REVISION DETAIL https://phabrica

D26386: Kicker/RecentDocuments: add icons to actions

meven added a comment. In D26386#587025 , @ngraham wrote: > Is there no way to set the icon during initialization so these can stay const? I would have needed to edit `Kicker::createActionItem`, it might be interesting. But not keeping

D26386: Kicker/RecentDocuments: add icons to actions

ngraham added a comment. Is there no way to set the icon during initialization so these can stay const? REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D26386 To: meven, #plasma, ngraham Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampal

D26386: Kicker/RecentDocuments: add icons to actions

meven created this revision. meven added reviewers: Plasma, ngraham. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. meven requested review of this revision. REPOSITORY R120 Plasma Workspace BRANCH master REVISION DETAIL https://phabricator.kde.org/D26386 AFFECTED