Differential D16171

Move "erase drawing" action to last position and change its icon
ClosedPublic
Actions

Authored by simgunz on Oct 13 2018, 7:51 AM.

Details

Reviewers
ngraham
Group Reviewers
Okular
VDG
Commits
R223:1547c7834029: Move "erase drawings" action to last position and change its icon
Summary

It is more logical to have the drawing tools first, then the eraser and then the action to delete everything. Also the two actions to erase things are close to each other now.
The icon of "erase drawing" has been changed to draw-eraser-delete-objects to distinguish it from the "Eraser" tool.

Before:

After:

BUG: 399734

Diff Detail

Repository
R223 Okular
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
simgunz created this revision.Oct 13 2018, 7:51 AM
Restricted Application added a project: Okular. · View Herald TranscriptOct 13 2018, 7:51 AM
Restricted Application added a subscriber: okular-devel. · View Herald Transcript
simgunz requested review of this revision.Oct 13 2018, 7:51 AM
simgunz edited the summary of this revision. (Show Details)Oct 13 2018, 7:57 AM
ngraham accepted this revision.Oct 13 2018, 7:06 PM
ngraham added a subscriber: ngraham.

+1, this seems sensible.

Please change Fix BUG: 399734 to just BUG: 399734. See https://community.kde.org/Infrastructure/Phabricator#Add_special_keywords

This revision is now accepted and ready to land.Oct 13 2018, 7:06 PM
simgunz edited the summary of this revision. (Show Details)Oct 14 2018, 1:34 PM
Closed by commit R223:1547c7834029: Move "erase drawings" action to last position and change its icon (authored by simgunz, committed by sander). · Explain WhyDec 21 2018, 11:45 AM
This revision was automatically updated to reflect the committed changes.
sander added a subscriber: sander.Dec 21 2018, 11:48 AM

Thanks! And apologies for not actually merging this much earlier.

Incidentally, while reviewing I noticed that the tooltip says "erase drawings" (plural!), while actually only the current drawing is erased (which is correct). If somebody knows where the text for that tooltip is...

ngraham added a comment.Dec 21 2018, 2:41 PM
In D16171#380336, @sander wrote:

Thanks! And apologies for not actually merging this much earlier.

Incidentally, while reviewing I noticed that the tooltip says "erase drawings" (plural!), while actually only the current drawing is erased (which is correct). If somebody knows where the text for that tooltip is...

https://cgit.kde.org/okular.git/tree/part.cpp#n904

sander added a comment.Dec 21 2018, 6:11 PM

Thanks, fixed!

Revision Contents
Changeset List

PathPackages
Mpart.cpp (2 lines)
Mui/presentationwidget.cpp (11 lines)

Diff 47947

View Options

part.cpp

Loading...
View Options

ui/presentationwidget.cpp

Loading...