Figure inside .column-screen steals that class

[jimjam-slam]

Bug description

When we place an image with a caption inside a fenced div with .column-screen, the image gets .column-screen and the section does not, which causes various kinds of layout havoc (the rest of the section doesn't get the enlarged width, and .page-columns propagates up from the image through the rest of the document, which makes placing one's own CSS grids inside the section extremely difficult).

Steps to reproduce

Test case: https://github.com/jimjam-slam/test-quarto-cr-img-classes

---
title: Test test test
format: html
---

Oh hello! Here are some images with classes:

::::{.column-screen}

This text is inside the section. But the section itself doesn't seem to get `.column-screen`...

:::{#working}
This works: ![a caption](trees.png)
:::

This also works (no caption):

:::{#alsoworking}
![](trees.png)
:::

But this doesn't:

:::{#notworking}
![This one is breaks things](trees.png)
:::

::::

Expected behavior

The fenced div in the input document that has .column-screen should have it in the output HTML, and the image inside #notworking should not. The other ancestors of the image should also not have .page-fill .page-columns.

Actual behavior

The image receives .column-screen and goes to full width. Other elements in the section don't. Any nested grids declared inside the broken section tend to get overridden, as .page-columns is on all ancestors of the image inside.

Your environment

• OS: macOS Sequoia 15.0
• VSCode 1.93.1

Quarto check output

Check file:///Users/rensa/code/quarto-cli/src/quarto.ts
Quarto 99.9.9
[✓] Checking versions of quarto binary dependencies...
      Pandoc version 3.4.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: 99.9.9
      commit: 2d0331c755c74544539cec2706df4bc4d378694d
      Path: /Users/rensa/code/quarto-cli/package/dist/bin

[✓] Checking tools....................OK
      TinyTeX: (external install)
      Chromium: (not installed)

[✓] Checking LaTeX....................OK
      Using: TinyTex
      Path: /Users/rensa/Library/TinyTeX/bin/universal-darwin
      Version: 2021

[✓] Checking basic markdown render....OK

[✓] Checking Python 3 installation....OK
      Version: 3.9.7 (Conda)
      Path: /Users/rensa/miniforge3/bin/python
      Jupyter: (None)

      Jupyter is not available in this Python installation.
      Install with conda install jupyter

[✓] Checking R installation...........OK
      Version: 4.2.1
      Path: /Library/Frameworks/R.framework/Versions/4.2-arm64/Resources
      LibPaths:
        - /Users/rensa/Library/R/arm64/4.2/library
        - /Library/Frameworks/R.framework/Versions/4.2-arm64/Resources/library
      knitr: 1.42
      rmarkdown: 2.21

[✓] Checking Knitr engine render......OK

EDIT: added a screenshot of the output, and also a link to previous discussion:

[jimjam-slam]

I should note that if you remove the offending image in #notworking, the layout works fine!

[cscheid]

Thanks, I can repro.

You can work around in the meantime by moving the offending image to its own div.

I believe the issue arises from our expectation that .column-screen would be used to wrap a single element at a time. (You can infer this from the way we use it in our docs, but we never actually describe this and I think everything should just work.)

[jimjam-slam]

That makes sense! I've reached for it in the past to build things like website hero sections, so if it's possible for it to work with more complex layouts, that would be fantastic (but I understand that you hadn't intended it for that kind of use).

Absolute worst case for us over on Closeread, we could potentially write our own CSS to re-implement the .column-* class 🤔 (cc @andrewpbray)

[cscheid]

Consider this slightly different repro:

---
format: html
---

:::{.column-screen}

Not working

:::{}
![This one is breaks things](trees.png)
:::

:::

:::{.column-screen}

Working

:::{}
![](trees.png)
:::

:::

Here's a screenshot of the result:

It's hard to see, but notice how the image in the bottom, isn't really "working": it doesn't span the whole screen. Notice also how the caption of the figure on the first div is not flush left: that's on purpose.

I think that the reason the figure steals the class is that if it didn't, then the nested layouts would cause havoc.

An image with a caption becomes a figure, and figures are indeed handled quite differently from other elements in our layouts. I think the best we'll be able to do is to ensure that a figure lives by itself inside a column-* div, by splitting the div into multiples as needed.

I don't know if this would break closeread or not, though.

[jimjam-slam]

Mmmm, I wondered whether the caption positioning betrayed some intentionality there. This is good to know! We probably need to try out a variety of figure possibilities to think holistically about how they should be laid out, but I might also try manually assigning the grid styles to our .column-screen sections to see how well that works.

For us, the problem isn't so much that the figure image gets it but more that (a) the section as a whole loses it and (b) .page-columns propagates up, overruling our nested grid (we use two: one for the section and one to stack sticky content on the z-axis). If we can stop that latter code from triggering on page load by avoiding the use of .column-screen, that's probably an easier solution that is mindful of your original intentions for it.

[cscheid]

It's easier to have a discussion if we can see a concrete example of what happens in closeread when this goes bad. Can you share one?

[jimjam-slam]

It's easier to have a discussion if we can see a concrete example of what happens in closeread when this goes bad. Can you share one?

@cscheid Absolutely! https://github.com/qmd-lab/closeread/blob/bugfix/81/docs/gallery/demos/sticky-blocks/index.qmd

Here it is as in the PR currently:

https://github.com/user-attachments/assets/57d410e4-9abc-4065-bd3e-e48148608674

And here it is again with the offending content removed:

https://github.com/user-attachments/assets/422840c9-54ef-4a93-a217-e4d93f397493

[cscheid]

Ok, thanks. This is my understanding of how we end up in a bad state:

• Users create a document with a {.cr-section} div and many entries in it
• You convert a {.cr-section} div to a {.column-screen} div (plus a lot of other work)
• Quarto removes the .column-screen treatment because of the included figure

Is that correct?

My original idea to fix Quarto's {.column-screen} treatment was to add a process of "subdividing" all the entries in a column-screen div to many sequential {.column-screen} divs, each with a single entry. That would cause many different {.cr-section} divs to be created. Would that break your filter?

[jimjam-slam]

I think it would likely break it, to be honest. The basic idea of Closeread is to take the children of .cr-section and to interleave them into two new children: a narrative column and a sticky column. These two columns we lay out on a new grid (which is why .page-columns propagating up from the figure breaks the layout). The sticky column has a child that is pinned to the top of the section with position: sticky as the narrative column children scroll past.

It sounds like we might be better off manually styling .cr-section to sit across Quarto's grid columns rather than using .column-screen, to be honest! That way we still get the benefits of Quarto's grid but don't risk .column-screen relocating to a figure.

[cscheid]

It sounds like we might be better off manually styling .cr-section to sit across Quarto's grid columns rather than using .column-screen, to be honest! That way we still get the benefits of Quarto's grid but don't risk .column-screen relocating to a figure.

I think that's accurate. If nothing else, because I think I will eventually need to fix the bug you found, and that will break closeread even more! In that vein, can you let me know when you made the change so I can wait and not break your extension? Thank you!

[jimjam-slam]

No worries — I would anticipate that to be over the next couple of weeks 😊

[cscheid]

I'm going to push this to 1.7 because our release date is nearing.

[jimjam-slam]

No stress at all, @cscheid — I think we have an MVP fix for this implemented (though we won't actually release until early next week), but we need to take a bit more time considering different layout combos anyway. Having a bit of time to let things settle and then review any impact from a 1.7 prerelease change isn't bad at all :)

[jimjam-slam]

I forgot to mention that the fix for this has been implemented! (We're in the process of prepping our new release today/tomorrow.)

FYI, the fix Andrew and I have gone with essentially replicates the ensureInGrid() function in Quarto, placing .page-column.page-full on the direct parent of our .cr-section (it doesn't go all the way up to main as Quarto's does; it seemed the direct parent was enough — but we're monitoring this to see how it goes.) parents up to main.

As we discussed, we also manually reimplement the .column-screen class to avoid triggering ensureInGrid() on the Closeread section itself.