D13203: Add Typewriter annotation tool in Okular

[Tobias Deiminger]

tobiasdeiminger added inline comments.

INLINE COMMENTS

> annotwindow.cpp:262
>  {
> -    const QColor newcolor = m_annot->style().color().isValid() ? m_annot->style().color() : Qt::yellow;
> +    const QColor newcolor = m_annot->style().color().isValid() ? QColor(m_annot->style().color().name()) : Qt::yellow;
>      if ( newcolor != m_color )

`QColor(m_annot->style().color().name())` is a bit arcane way of saying "force alpha of the popup window background color to 1.0" 😄 If that's what you've actually intended?

How about removing const of newcolor (not bad, local scope only) and do `newcolor.setAlpha(1.);`?
Or something like

  const QColor newcolor = m_annot->style().color().isValid() ? QColor(m_annot->style().color().red(), m_annot->style().color().green(), m_annot->style().color().blue(), 255) : Qt::yellow;

if you want to keep const. That would be more explicit and obvious.

Btw., sorry for commenting on an older revision. But ui/annotwindow.cpp is not included in the latest revision. Phabricator seems to be a bit too smart for our intended workflow.

REPOSITORY
  R223 Okular

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

To: dileepsankhla
Cc: ltoscano, ngraham, tobiasdeiminger, aacid, okular-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20180601/82757762/attachment.html>