Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] owcorpusviewer: Avoid 'synchronous' evalJS, use 'runJavaScript' instead #341

Merged
merged 2 commits into from
May 4, 2018

Conversation

ales-erjavec
Copy link
Contributor

Issue

Fixes gh-340 (probably)

Description of changes

Avoid 'synchronous' evalJS, use async runJavaScript instead.

The 'wait' loop for QWebEngineView implemented in gui.WebView seems to
interfere with key press delivery.

This might be platform dependent (tested on macOS).
To duplicate create Corpus (bookexcerpts) -> Corpus Viewer and type e.g. hand really fast. Try multiple times. Notice if any keys are not entered. Switch to canvas and right click on an empty area to bring up the widget menu. The missing pressed keys are entered in the search box (that is the search line edit receives the dropped key events when shown).

Includes
  • Code changes
  • Tests
  • Documentation

The 'wait' loop for QWebEngineView implemented in gui.WebView seems to
interfere with key press delivery.
@ajdapretnar
Copy link
Collaborator

It works for me. 👍
Should I merge despite Travis? It fails on BoW, which seems to be unrelated to this PR.

@ales-erjavec
Copy link
Contributor Author

Not yet. Highlighting does not work when using WebKit backend.

@ales-erjavec ales-erjavec changed the title [FIX] owcorpusviewer: Avoid 'synchronous' evalJS, use 'runJavaScript' instead [WIP][FIX] owcorpusviewer: Avoid 'synchronous' evalJS, use 'runJavaScript' instead May 3, 2018
@ales-erjavec ales-erjavec changed the title [WIP][FIX] owcorpusviewer: Avoid 'synchronous' evalJS, use 'runJavaScript' instead [FIX] owcorpusviewer: Avoid 'synchronous' evalJS, use 'runJavaScript' instead May 3, 2018
@ales-erjavec
Copy link
Contributor Author

Can you please test again.

Re. travis: Yes. That is a different problem (Use Bag of Words widget with Term frequency: Binary)

@codecov-io
Copy link

codecov-io commented May 4, 2018

Codecov Report

Merging #341 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #341   +/-   ##
=======================================
  Coverage   84.92%   84.92%           
=======================================
  Files          33       33           
  Lines        1864     1864           
  Branches      336      336           
=======================================
  Hits         1583     1583           
  Misses        242      242           
  Partials       39       39

@ajdapretnar ajdapretnar merged commit 1617257 into biolab:master May 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants