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] WebviewWidget: WebEngine Don't Grab Focus on setHtml #1983

Merged
merged 1 commit into from
Feb 20, 2017

Conversation

nikicc
Copy link
Contributor

@nikicc nikicc commented Feb 1, 2017

Issue

If running on WebEngine the call to setHtml grabs focus and sets it to WebViewWidget. This is quite annoying behaviour when for example one would like to filter some data set and show the results in WebView. Due to WebViewWidget's focus grabbing the input filter looses focus after every character is typed.

Description of changes

Override setHtml to that the widget's state is first set to disabled, then call super().setHtlm and finally restore the initial state.

Includes
  • Code changes
  • Tests
  • Documentation

@nikicc nikicc added this to the 3.3.11 milestone Feb 1, 2017
@nikicc nikicc force-pushed the webview-focus-fixup branch 2 times, most recently from f135089 to 3e03475 Compare February 1, 2017 17:53
@codecov-io
Copy link

codecov-io commented Feb 1, 2017

Codecov Report

Merging #1983 into master will decrease coverage by -0.01%.
The diff coverage is 80%.

@@            Coverage Diff             @@
##           master    #1983      +/-   ##
==========================================
- Coverage   70.22%   70.21%   -0.01%     
==========================================
  Files         343      343              
  Lines       54092    54097       +5     
==========================================
+ Hits        37984    37985       +1     
- Misses      16108    16112       +4

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 70f0e1b...6a5a8bc. Read the comment docs.

@nikicc nikicc changed the title [FIX] WebviewWidget: WebEngine Don't Grab Focus on setHtml [WIP] [FIX] WebviewWidget: WebEngine Don't Grab Focus on setHtml Feb 2, 2017
@nikicc nikicc changed the title [WIP] [FIX] WebviewWidget: WebEngine Don't Grab Focus on setHtml [FIX] WebviewWidget: WebEngine Don't Grab Focus on setHtml Feb 2, 2017
@astaric astaric modified the milestones: future, 3.3.11 Feb 3, 2017
@lanzagar
Copy link
Contributor

I don't have the mentioned problem (using qt 5.8). But the fix does not seem to break anything for me either. Could you add a TODO or something like that to the comment and mention that this might be fixed in newer versions, so that we can remove the fix in the future if it will not be necessary anymore.

@astaric
Copy link
Member

astaric commented Feb 17, 2017

I could not reproduce this either (I'm using Qt 5.6).

@ajdapretnar
Copy link
Contributor

I have the bug. (Qt 5.6)

@nikicc
Copy link
Contributor Author

nikicc commented Feb 17, 2017

@astaric running on anaconda?

@nikicc
Copy link
Contributor Author

nikicc commented Feb 17, 2017

@lanzagar I added the comment.

@astaric these are the packages I have installed in my conda env. Are you running the same versions and cannot reproduce? Are you running WebKit or WebEngine?

anyqt                     0.0.7                    py35_1    conda-forge
pyqt                      5.6.0                    py35_2
pyqtgraph                 0.10.0                   py35_0    conda-forge
qt                        5.6.2                         0

If running on WebEngine the call to setHtml grabs focus and sets it to WebViewWidget. Prevent such behaviour.
@ajdapretnar ajdapretnar merged commit 8f0ec2f into biolab:master Feb 20, 2017
@nikicc nikicc deleted the webview-focus-fixup branch February 20, 2017 14:12
@astaric astaric modified the milestone: future Apr 19, 2017
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.

5 participants