D15204: Add typewriter annotation tool

Tobias Deiminger noreply at phabricator.kde.org
Tue Sep 4 19:16:33 BST 2018


tobiasdeiminger added a comment.


  In D15204#320195 <https://phabricator.kde.org/D15204#320195>, @sander wrote:
  
  > Would it a good idea to split those changes that deal with the color alpha channel into a separate patch?  That would make reviewing easier, and lead to more legible git history.
  
  
  Good idea. Alpha channel changes for other annotations are small in LOC and independent from typewriter. We can easily shift that to a new revision.

INLINE COMMENTS

> sander wrote in parttest.cpp:45
> Is this change really needed for the Typewriter tool, or is it 'just' related cleanup of the `CloseDialogHelper` class? If the latter, can we move it into a separate patch?

> Is this change really needed for the Typewriter tool

It was indeed required. `qApp->activeModalWidget()`was the simples way to lookup the modal dialog. As a consequence, `Okular::Part *p` was no longer required. `QMetaObject::invokeMethod(button, "click", Qt::QueuedConnection)` was required to avoid crashes.

Though the latter is probably a workaround for a hidden bug in Okulars `PickPointEngine::addTextNote`. Try this: Fire up okular in KDABs gammaray, select inline note tool, and click into a page. Now 20..50 QInputDialogs pop up immediately, instead of 1. That's basically the same as what we encountered in the test prior using `invokeMethod`.

REPOSITORY
  R223 Okular

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

To: tobiasdeiminger
Cc: sander, okular-devel, ngraham, aacid
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20180904/6882da01/attachment-0001.html>


More information about the Okular-devel mailing list