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

Reply via email to