D13203: Add Typewriter annotation tool in Okular

Dileep Sankhla noreply at phabricator.kde.org
Sat Jun 2 03:41:51 UTC 2018


dileepsankhla marked an inline comment as done.
dileepsankhla added a comment.


  The revision is closed. I'm unable to update it. Maybe someone with admin privileges should reopen it?

INLINE COMMENTS

> tobiasdeiminger wrote in annotwindow.cpp:262
> `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.

I agree. It should be straightforward without any latent meaning. Updating the revision.

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/20180602/5fd64020/attachment-0001.html>


More information about the Okular-devel mailing list