Change panel edit mode icon from from a hamburger icon to a configure icon
ClosedPublic

Authored by GB_2 on Dec 2 2018, 6:42 PM.

Details

Summary

Changes the panel edit mode icon from a hamburger icon to a configure icon (as suggested in this Phabricator task: https://phabricator.kde.org/T10047).

Test Plan

Look at the panel.

Diff Detail

Repository
R119 Plasma Desktop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
GB_2 created this revision.Dec 2 2018, 6:42 PM
Restricted Application added a subscriber: plasma-devel. · View Herald TranscriptDec 2 2018, 6:42 PM
GB_2 requested review of this revision.Dec 2 2018, 6:42 PM
ngraham added a subscriber: ngraham.Dec 2 2018, 6:54 PM

For the panel edit mode button, I think this is 100% appropriate since it opens a configuration UI. For the toolbox, I'm not as convinced yet, since clicking on it opens a menu with a number of non-configuration-related menu items. It's not the biggest deal, but it does feel a bit odd.

Thoughts?

GB_2 added a comment.Dec 2 2018, 6:55 PM

For the panel edit mode button, I think this is 100% appropriate since it opens a configuration UI. For the toolbox, I'm not as convinced yet, since clicking on it opens a menu with a number of non-configuration-related menu items. It's not the biggest deal, but it does feel a bit odd.

Thoughts?

Oh, now that I think about it, I can say that I agree.

GB_2 updated this revision to Diff 46739.Dec 2 2018, 6:57 PM
GB_2 edited the summary of this revision. (Show Details)

Only change panel toolbox icon.

GB_2 edited the summary of this revision. (Show Details)Dec 2 2018, 7:00 PM
ngraham accepted this revision.Dec 2 2018, 7:09 PM

I'm happy now! Plasma folks, do you agree?

This revision is now accepted and ready to land.Dec 2 2018, 7:09 PM
ngraham retitled this revision from Change toolbox icon from from a hamburger icon to a configure icon to Change panel edit mode icon from from a hamburger icon to a configure icon.Dec 2 2018, 7:10 PM
ngraham edited the summary of this revision. (Show Details)
ngraham edited the test plan for this revision. (Show Details)

Plasma folks, any comments, or shall we land this?

davidedmundson requested changes to this revision.Dec 14 2018, 10:17 PM
davidedmundson added a subscriber: davidedmundson.

If Nate and Andy say to use this icon, go for it.

One code change

toolboxes/paneltoolbox/contents/ui/main.qml
58

This doesn't make sense..

We're re-evaulating because hasElement might have changed when switching themes.

This revision now requires changes to proceed.Dec 14 2018, 10:17 PM

Thanks Dave!

GB_2 requested review of this revision.Dec 14 2018, 11:01 PM
GB_2 added inline comments.
toolboxes/paneltoolbox/contents/ui/main.qml
58

So just remove the line?

GB_2 updated this revision to Diff 47615.Dec 15 2018, 12:06 PM

Remove unneeded line.

GB_2 marked 2 inline comments as done.Dec 15 2018, 12:07 PM
davidedmundson accepted this revision.Dec 15 2018, 5:46 PM
This revision is now accepted and ready to land.Dec 15 2018, 5:46 PM
This revision was automatically updated to reflect the committed changes.