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

PDF hover tooltip fix #248

Merged
merged 4 commits into from
Sep 15, 2023
Merged

PDF hover tooltip fix #248

merged 4 commits into from
Sep 15, 2023

Conversation

trevorcampbell
Copy link
Contributor

Closes #175

But I can't test it yet. @joelostblom #171 appears to be a problem still even after the PR in #218

Was there a regression somewhere? Or did the new Dockerfile build that just happened for #240 break things?

@trevorcampbell trevorcampbell marked this pull request as draft September 15, 2023 05:38
@joelostblom
Copy link
Collaborator

Hmm, it seems to be working for me with the latest updates on main and a cleared build directory. Any chance there is something in old in your build directory that interferes with building the new pdf file?

@trevorcampbell
Copy link
Contributor Author

trevorcampbell commented Sep 15, 2023

You're right -- I needed to clean out my _build -- turns out if you build the html copy of the book and then the PDF, the build doesn't display altair plots properly in the PDF. Probably related to jbook caching.

This PR should be good to review now

@trevorcampbell trevorcampbell marked this pull request as ready for review September 15, 2023 17:00
Copy link
Collaborator

@joelostblom joelostblom left a comment

Choose a reason for hiding this comment

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

lgtm!

glue('can_lang_plot_tooltip', Image('img/viz/languages_with_mouse.png'), display=False)
else:
# Increasing the dimensions makes all the ticks fit in jupyter book (the fit with the default dimensions in jupyterlab)
glue('can_lang_plot_tooltip', can_lang_plot_tooltip.properties(height=320, width=420), display=False)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice fix

@trevorcampbell
Copy link
Contributor Author

@joelostblom in the PDF version the image is too big. I can't seem to shrink it...shrinking the actual file itself didn't help, nor did setting width=... in the Image(...) and nor did setting :figwidth: in the glue directive. I'm a bit stumped...

@joelostblom
Copy link
Collaborator

Are you able to add a ppi attribute to the figure? That is what worked for us in altair with the latex PDFs, see an example of the output here vega/vl-convert#85 (comment) and here vega/altair#3163.

If that is not possible, is there any latex directive that could be used? Like setting the image size globally just before this figure and then setting it back?

@trevorcampbell
Copy link
Contributor Author

I've tried a few other things, and nothing seems to work -- either the image disappears entirely, or it appears but is too big.

I am going to merge this anyway because the HTML build is fine, and the PDF build is improved but not perfect yet. Once the changes are limited to just "make the PDF build work for print" I'll return to this.

@trevorcampbell trevorcampbell merged commit dbf525f into main Sep 15, 2023
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.

Fix render of interactive images in PDF
2 participants