General improvements to stamp annotation
AcceptedPublic

Authored by simgunz on Mon, Jun 24, 6:05 AM.

Details

Summary

Configuration:

  • Add push button to select custom stamp image
  • Check if loaded image is usable as stamp or throw error
  • Keep image proportions in previewer
  • Move previewer below the combobox to display larger preview

Annotation tool:

  • Keep stamp image proportion in annotation preview (while left mouse button is down)
  • Adding the annotation with one-click (without holding the left mouse button and dragging) adds the stamp with original proportions

BUG: 370381
BUG: 383652

Closes T8074

TODO:

  • Check if filters in file chooser make sense / propose better alternative
  • Update doc ( @yurchor will do it after we merge this)
Test Plan

From stamp annotation configuration dialog:

  • Click push button next to combo box opens a file selector
  • Selecting a corrupted image file should throw an error
  • Selecting a good image file shows the preview of the image
  • Select a horizontal image shows a large clear preview
  • Select a vertical image file shows a smaller preview without messing up the visual of the config dialog
  • Input a valid icon name in the combobox and the preview of the icon is shown

From page view, select the stamp annotation with horizontal image file (not squared):

  • Click and hold. The preview maintains proportions
  • Single click. The stamp image in the pdf maintains proportions and has the same size of the click and hold preview.
  • Add an annotation of the Okular custom stamps (internal SVG so treated slightly differently) do not create problems

Diff Detail

Repository
R223 Okular
Branch
improve-stamp-annotation
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 13302
Build 13320: arc lint + arc unit
simgunz created this revision.Mon, Jun 24, 6:05 AM
Restricted Application added a project: Okular. · View Herald TranscriptMon, Jun 24, 6:05 AM
Restricted Application added a subscriber: okular-devel. · View Herald Transcript
simgunz requested review of this revision.Mon, Jun 24, 6:05 AM
simgunz edited the summary of this revision. (Show Details)Mon, Jun 24, 6:09 AM
simgunz edited the test plan for this revision. (Show Details)


Okular embedded stamp


File chooser filters


Horizontal image


Vertical image


Icon name


Error message

This is fantastic! I tested it out and it works perfectly. Huge improvement.

@yurchor Are you willing to update the documentation regarding this modification after we merge this? (I saw you changed the doc regarding the refactor of the annotation tools config dialogs, and it seems you have experience with the documentation aspect).

ngraham edited the summary of this revision. (Show Details)Tue, Jun 25, 7:36 AM
ngraham added inline comments.Tue, Jun 25, 7:42 AM
ui/annotationwidgets.cpp
161

Are you sure the list of file formats should be localized?

433

If this was an enum, you wouldn't need to add an inline comment explaining what it does :)

ui/annotationwidgets.h
35–37

Instead of adding a new bool argument, how about an enum instead for clarity so the code is easy to read?

@yurchor Are you willing to update the documentation regarding this modification after we merge this? (I saw you changed the doc regarding the refactor of the annotation tools config dialogs, and it seems you have experience with the documentation aspect).

Sure.

simgunz updated this revision to Diff 60660.Tue, Jun 25, 6:05 PM
  • Define enum for preview position
simgunz marked 2 inline comments as done.Tue, Jun 25, 6:08 PM

Added the enum, much better now.

@yurchor Great.

ui/annotationwidgets.cpp
161

I have copied the code from KIconLoader. I guess that "Icon Files" can be translated. Any suggestion to improve it?

https://api.kde.org/frameworks/kiconthemes/html/kicondialog_8cpp_source.html#l00605

simgunz edited the summary of this revision. (Show Details)Tue, Jun 25, 6:09 PM
ngraham accepted this revision.Tue, Jun 25, 6:53 PM

Looks good to me, but I'd like a review from an Okular person, too.

ui/annotationwidgets.cpp
161

Ehh, looks like this is actually fine.

This revision is now accepted and ready to land.Tue, Jun 25, 6:53 PM
simgunz added a subscriber: aacid.Fri, Jun 28, 6:08 AM

@aacid Can you also have a look at this revision when you have the time?