Kicker/RecentDocuments: add icons to actions
ClosedPublic

Authored by meven on Jan 3 2020, 11:01 AM.

Diff Detail

Repository
R120 Plasma Workspace
Branch
arcpatch-D26386_1
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 20623
Build 20641: arc lint + arc unit
meven created this revision.Jan 3 2020, 11:01 AM
Restricted Application added a project: Plasma. · View Herald TranscriptJan 3 2020, 11:01 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
meven requested review of this revision.Jan 3 2020, 11:01 AM
meven edited the test plan for this revision. (Show Details)Jan 3 2020, 11:01 AM

Is there no way to set the icon during initialization so these can stay const?

meven added a comment.Jan 3 2020, 4:40 PM

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 the const is not too important those variables have a limited lifetime anyway.
Not having const here does not add a copy or anything.

meven updated this revision to Diff 72697.Jan 3 2020, 4:53 PM

Add an icon parameter to Kicker::createActionItem

meven added a comment.Jan 3 2020, 4:57 PM

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 this course.

Yeah, it definitely bloats this diff, but maybe it would make subsequent ones smaller and cleaner?

Plasma folks, what do you think?

hein requested changes to this revision.Jan 3 2020, 7:01 PM
hein added a subscriber: hein.

It should be iconName after label.

This revision now requires changes to proceed.Jan 3 2020, 7:01 PM
meven added a comment.Jan 3 2020, 7:27 PM
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.

meven updated this revision to Diff 72752.Jan 4 2020, 3:54 PM

Change signature of Kicker::createActionItem and update all its users

meven updated this revision to Diff 72754.Jan 4 2020, 3:56 PM

Clean unwanted change

meven updated this revision to Diff 72755.Jan 4 2020, 4:04 PM

Use view-hidden icon for Hide Application action for consistency

meven added inline comments.Jan 5 2020, 12:51 PM
applets/kicker/plugin/contactentry.cpp
117

I missing an icon for this action, I am open for suggestion.

ngraham added inline comments.Jan 5 2020, 4:10 PM
applets/kicker/plugin/contactentry.cpp
117

Maybe user-identity or identity, depending on what it does

meven updated this revision to Diff 72804.Jan 5 2020, 4:22 PM
meven marked 2 inline comments as done.

Add icon identity for showContactInfo action

ngraham accepted this revision.Jan 5 2020, 5:21 PM

Nice!

meven added a comment.Jan 6 2020, 9:06 PM

@hein is it fine for you ?

meven planned changes to this revision.Jan 7 2020, 5:05 PM
meven added inline comments.
applets/kicker/plugin/actionlist.cpp
62

Remove this if

applets/kicker/plugin/recentcontactsmodel.cpp
93

Missing icon

hein accepted this revision.Jan 7 2020, 11:21 PM

Good stuff!

meven updated this revision to Diff 73035.Jan 8 2020, 8:33 AM

Make icon non-optionnal in Kicker::createActionItem, add a missing icon

This revision is now accepted and ready to land.Jan 8 2020, 8:33 AM
meven marked 2 inline comments as done.Jan 8 2020, 8:34 AM
This revision was automatically updated to reflect the committed changes.