-
Notifications
You must be signed in to change notification settings - Fork 320
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
Lightbox and RevealJS : image size problem #11022
Comments
You need to re-enable stretch: https://quarto.org/docs/presentations/revealjs/advanced.html#stretch ![Some random image with lightbox](https://cdn.pixabay.com/photo/2023/04/20/12/22/globe-7939725_960_720.jpg){.lightbox .r-stretch} To me, there is no bug. |
Auto-stretch behavior in revealjs is a constant source of confusion, but I agree with @mcanouil here. We don't want auto-stretch on images with any other classes because those classes almost certainly indicate some intended behavior that will interact badly with I'm going to go ahead and close this one. In the future, if you're not certain this is a bug, it helps us if you start with a discussion question first. |
Thanks for your answers. @mcanouil , I already tried to re-enable ![Some random image with lightbox](https://cdn.pixabay.com/photo/2023/04/20/12/22/globe-7939725_960_720.jpg){.lightbox} gives <div class="quarto-figure quarto-figure-center">
<figure>
<p><a href="https://cdn.pixabay.com/photo/2023/04/20/12/22/globe-7939725_960_720.jpg" class="lightbox" data-gallery="quarto-lightbox-gallery-1" title="Some random image with lightbox"><img data-src="https://cdn.pixabay.com/photo/2023/04/20/12/22/globe-7939725_960_720.jpg" alt="Some random image with lightbox"></a></p>
<figcaption>Some random image with lightbox</figcaption>
</figure>
</div> ![Some random image with lightbox](https://cdn.pixabay.com/photo/2023/04/20/12/22/globe-7939725_960_720.jpg){.lightbox .r-stretch} gives <img data-src="https://cdn.pixabay.com/photo/2023/04/20/12/22/globe-7939725_960_720.jpg" class="r-stretch quarto-figure-center" alt="Some random image with lightbox"><p class="caption">Some random image with lightbox</p> From what @cscheid says, I guess it might be the intended behavior but then, if not a bug, the possibility to manually re-enable To me the fact that it was a bug made no doubt but in the future I will start with a discussion. |
I see. For clarity and because that's not what this issue reported, I would open a new issue focussing on the conflict between |
To be clear, the stretch feature is a revealjs feature: https://revealjs.com/layout/#stretch So this would be a Revealjs feature request, until we decide to reimplement the stretch feature ourself. Currently this is not the case and we inherit the stretch feature from Revealjs library. |
Thanks @cderv, I did not know stretch was from Reveal.js. |
Thanks to all for these answers. It seems to be a tricky problem as it involves some frictions between components of your own and upstream features. @mcanouil , do you still think I should open a new bug here? This may be none of my business and my expertise in OSS development is very limited but according to my own experience and what I’ve been able to observe in some communities / businesses, a good rule of thumb is to contribute as upstream as you can as often as you can, precisely to avoid this kind of issue (that can appear over time, with upstream upgrades breaking compatibility with your work). So, instead of reimplementing a feature (=internalizing), I think if I was in your position I would choose to propose lightbox as a native RevealJS feature (=externalising). It may cost more in terms of works of hours in the short term, but I tend to think it is more profitable in the long run. Please forgive me for my two cents. :) |
No, see why in #11022 (comment) |
In general I can see how the idea is appealing. However, in context of Quarto, it is is not that interesting because Lightbox feature is a Quarto feature in the sense it is common to all HTML format we have. So even if it is implemented as revealjs level, we would still need to maintain lightbox working for other HTML format we have. Though it could have benefits to have it included in revealjs directly and just leverage that. Anyhow, unfortunately we don't have the power yet to contribute such work upstream considering our current plans on quarto itself. If anyone contributes it, we'll be happy to work integration in a future revealjs release. For now best workaround is to manually set size when using Lightbox. Thanks again for your feedback @pagiraud ! |
Bug description
The lightbox class can be used for generating RevealJS presentations, which is very convenient to be able to “zoom” on certain images / figures. But when you do that, you lose the “autosize” for your images (I don’t know how it is called, it is the fact that an image, by default, can’t vertically overflow your slide) and so, often, you have to manually put a vertical size (typically between 500 and 550 for me) if you want to avoid a scrollbar. This has drawbacks when exporting to other formats (such as PDF with the article class) because then, the images are quite small.
Steps to reproduce
Example:
Expected behavior
I think the two slides in the example above should look and behave exactly the same, except for the ability to click on the image on the second one.
Actual behavior
The two images are not of the same size.
Your environment
RStudio 2024.09.0 build 375
Quarto check output
Quarto 1.5.57 [✓] Checking versions of quarto binary dependencies... Pandoc version 3.2.0: OK Dart Sass version 1.70.0: OK Deno version 1.41.0: OK Typst version 0.11.0: OK [✓] Checking versions of quarto dependencies......OK [✓] Checking Quarto installation......OK Version: 1.5.57 Path: /opt/quarto/bin [✓] Checking tools....................OK Chromium: 869685 TinyTeX: (not installed) [✓] Checking LaTeX....................OK Using: Installation From Path Path: /usr/bin Version: 2023 [✓] Checking basic markdown render....OK [✓] Checking Python 3 installation....OK Version: 3.12.3 Path: /usr/bin/python3 Jupyter: (None) Jupyter is not available in this Python installation. Install with python3 -m pip install jupyter [✓] Checking R installation...........OK Version: 4.4.1 Path: /usr/lib/R LibPaths: - /home/xxxxxxxxx/R/x86_64-pc-linux-gnu-library/4.4 - /usr/local/lib/R/site-library - /usr/lib/R/site-library - /usr/lib/R/library knitr: 1.48 rmarkdown: 2.28 [✓] Checking Knitr engine render......OK
The text was updated successfully, but these errors were encountered: