quarto-ext / lightbox

Create lightbox treatments for images in your HTML documents.
https://quarto-ext.github.io/lightbox/
MIT License
76 stars 4 forks source link

Set description for lightbox in plot from code chunk #12

Closed DavZim closed 2 years ago

DavZim commented 2 years ago

I tried to use lightbox (which is awesome btw!) to describe the output (= plot) of a code chunk but cannot get it to work.

---
title: Simple Lightbox Example
filters:
   - lightbox
lightbox: auto
---

![A Lovely Image](mv-1.jpg){description="This description works perfectly fine!"}

```{r}
#| description: This description is never shown...
plot(1:10, rnorm(10))


Is this functionality supported or are there any plans to support it?
cderv commented 2 years ago

Currently the way to do that needs to leverage some knitr options that will put the right attributes on the image.

```{r}
#| fig-cap: R plot
#| out-extra: 'description="This description is never shown..."'
plot(1:10, rnorm(10))

To explain, the extension expect this syntax 

Caption{description="some text"}


This is what **knitr** will produce if you set `fig-cap` and `out-extra`. 

Setting `description` here is different than setting in the attribute. 
We would need to improve the handling so that 
````markdown
```{r}
#| description: some text
plot(...)
can be used

@dragonstyle happy to discuss about it.
Are we happy with such workaround ? but what about other language ? 
Do we want to support option in YAML inside chunk for extensions - I believe we could do it in our filter, but at some point, like with **knitr** option, we'll have namespace conflict in option.  Maybe this would be a clearer syntax. 

````markdown
```{r}
#| lightbox-description: some text
plot(...)

or 
````markdown
```{r}
#| lightbox:
#|   description: some text
plot(...)
dragonstyle commented 2 years ago

@cderv Yeah I think this will be a need for Quarto - we have some attributes processing for cells now, but likely need to do some additional work to allow arbitrary attributes of some form.

In the meantime, I did push an update to the extension which will forward the fig-alt (if present) to the Lightbox element title. That is likely a good idea in any event, but may also improve the situation with computational output for the time being.

cderv commented 2 years ago

Just for reference the commit in question 8a10ba368f57c61c5c737a5d5af54c274fb5c33e

You change means it is either a caption or a description right ? You can't really set a caption on the image, and then have a description for the lightbox.

Maybe I missed something but the elseif seems to me it caption or alt text. So one can't set a description, meaning having a title and a description

image

I was trying to thing how to bring equivalent feature from markdown syntax with description attributes, to image generating from chunk.

My understanding is that we pass unknown option to .cells so we could detect and move to image, but that is maybe too generic, that is why I was thinking of some namespaced chunk option.

Anyway,

I think this will be a need for Quarto

I agree with that - we need a way to think of the best way for code chunk could interact with extensions filters. Design choices.

dragonstyle commented 2 years ago

You're correct @cderv - it will provide the title for the Lightbox image from alt or fig-alt, but in the case of a code chunk there is still no way to specify a description. I think that you're right that Quarto itself will need a cross-language way to provide additional attributes that make their way through to the AST so we can handle them properly when we generate the Lightbox.

dragonstyle commented 2 years ago

My understanding is that we pass unknown option to .cells so we could detect and move to image, but that is maybe too generic, that is why I was thinking of some namespaced chunk option.

I think you're right about providing some chunk option for this- we already have some like attr-source and attr-output that target specific portions of the code cells, but no fig-attror tbl-attr for example, which would make sense IMO... It's not quite as generic as supporting general pass through attributes, but it does have the benefit of being a little more clear about what the author intends - not sure what the best approach is!

cderv commented 2 years ago

we already have some likeattr-source and attr-output that target specific portions of the code cells

I believe this is for knitr chunk only. We still need to make this generic (https://github.com/quarto-dev/quarto-cli/issues/2040)

no fig-attror tbl-attr for example, which would make sense IMO

In knitr, it is called out.extra - that is why my example above (https://github.com/quarto-ext/lightbox/issues/12#issuecomment-1263553031) uses it to solve the issue.

Let me clarify a thought that may have not been clear: What I understand of our current way is that, if a chunk option is unknown, we are passing it as-is to the Div of class .cells. So we have a way to pass some chunk option to Lua filter this way currently.

This means from this:

```{r}
#| fig-cap: R plot
#| description: This description is never shown
plot(1:10, rnorm(10))
we get 

::: {.cell description='This description is never shown'}

plot(1:10, rnorm(10))

::: {.cell-output-display} R plot{width=672} ::: :::

and so we can get `description` in our Lua Filter, to set as attribute to the image. right ? 

Only thing is that it is really generic, and if several extensions are activated, how do we insure that we don't have conflict, etc.... 

* We could use prefix option like `lightbox-description:`, that way we can filter in our lightbox filter the option to use 

* We could use list in YAML like 
````markdown
```{r}
#| fig-cap: R plot
#| lightbox:
#|   description: This description is never shown
plot(1:10, rnorm(10))


and then use our `quarto.json.decode` API to get the value. 

This is what I mean by a design choice to discuss in the team. Let me do a PR as proof of concept. 
cderv commented 2 years ago

@DavZim you could try out the new version - it should be easier to add description now.

Thank you for the feedback

DavZim commented 2 years ago

@cderv It works perfectly in my tests. I can even add the other options like the desc-position: right.

Thank you for the quick fix. Highly appreciated!

```{r}
#| fig-cap: R plot
#| lightbox:
#|   description: This description shown
#|   desc-position: right
plot(1:30, cumsum(rnorm(30)), type = "l")
cderv commented 2 years ago

Yes indeed, any supported option can be set at the chunk level !

Glad it works !