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

Compute correct widget width for rich text labels #199

Merged
merged 4 commits into from
Mar 24, 2021

Conversation

maweigert
Copy link
Contributor

@maweigert maweigert commented Mar 24, 2021

Currently, the width of label widgets that contain rich text (such as html tags) is computed based on the raw text alone. This may result in excessively large widgets, e.g. when including images.

For instance the following

#download test image 
!wget https://raw.githubusercontent.com/napari/magicgui/master/resources/hotdog.jpg -O very_impressive_flashy_logo_for_my_awesome_napari_plugin.jpg


from magicgui import magicgui

@magicgui(
    label=dict(widget_type='Label', label='<img src="very_impressive_flashy_logo_for_my_awesome_napari_plugin.jpg" width="64">Text'))
def widget(label):
    pass

widget.show()

leads to

Screenshot 2021-03-24 at 16 38 07

The PR fixes how the default size of the label widget is computed for rich text labels. The above code then yields

Screenshot 2021-03-24 at 16 38 16

Copy link
Member

@tlambert03 tlambert03 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome! thanks!

@tlambert03
Copy link
Member

bummer ... mightBeRichText doesn't look to be in the Qt namespace in pyside2... we'll have to figure out what's going on there.

@tlambert03
Copy link
Member

from the qt source code, looks like all mightBeRichText isn't doing much:

This function uses a fast and therefore simple heuristic. It
mainly checks whether there is something that looks like a tag
before the first line break. Although the result may be correct
for common cases, there is no guarantee.

so we can just use regex and do:

def might_be_rich_text(text):
    import re
    return bool(re.search("<[^\n]+>", text))

@codecov
Copy link

codecov bot commented Mar 24, 2021

Codecov Report

Merging #199 (fc42698) into master (c50a7e2) will increase coverage by 0.02%.
The diff coverage is 60.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #199      +/-   ##
==========================================
+ Coverage   90.07%   90.09%   +0.02%     
==========================================
  Files          29       29              
  Lines        2981     2989       +8     
==========================================
+ Hits         2685     2693       +8     
  Misses        296      296              
Impacted Files Coverage Δ
magicgui/backends/_qtpy/widgets.py 86.47% <60.00%> (-0.40%) ⬇️
magicgui/application.py 83.14% <0.00%> (+2.24%) ⬆️
magicgui/backends/_qtpy/application.py 100.00% <0.00%> (+2.77%) ⬆️

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 c50a7e2...fc42698. Read the comment docs.

@maweigert
Copy link
Contributor Author

bummer ... mightBeRichText doesn't look to be in the Qt namespace in pyside2... we'll have to figure out what's going on there.

Mhm, seems mightBeRichText lives in different submodules for both backends:

PySide2.QtGui.Qt.mightBeRichText
PyQt5.QtCore.Qt.mightBeRichText

@tlambert03
Copy link
Member

thanks!

@tlambert03 tlambert03 merged commit f6e0728 into pyapp-kit:master Mar 24, 2021
@tlambert03 tlambert03 added the enhancement New feature or request label Apr 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants