D22064: General improvements to stamp annotation

[Nathaniel Graham]

ngraham added inline comments.

INLINE COMMENTS

> annotationwidgets.cpp:153
> +    const QString customStampFile = QFileDialog::getOpenFileName(this, i18n("Select custom stamp symbol"),
> +        QString(), i18n("*.ico *.png *.xpm *.svg *.svgz | Icon Files (*.ico *.png *.xpm *.svg *.svgz)") );
> +    if ( !customStampFile.isEmpty() )

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

> annotationwidgets.cpp:425
>  
> -    m_pixmapSelector = new PixmapPreviewSelector( widget );
> +    m_pixmapSelector = new PixmapPreviewSelector( widget, true /* previewBelow */ );
>      formlayout->addRow( i18n( "Stamp symbol:" ), m_pixmapSelector );

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

> annotationwidgets.h:35
>  public:
> -    explicit PixmapPreviewSelector( QWidget * parent = nullptr );
> +    explicit PixmapPreviewSelector( QWidget * parent = nullptr, bool previewBelow = false );
>      virtual ~PixmapPreviewSelector();

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

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D22064

To: simgunz, #okular
Cc: yurchor, ngraham, okular-devel, fbampaloukas, joaonetto, tfella, darcyshen, aacid
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20190625/0a601e07/attachment-0001.html>