qmd-lab / closeread

https://closeread.dev
MIT License
130 stars 5 forks source link

Code appears before plots when using figure layouts #41

Open andrewpbray opened 4 months ago

andrewpbray commented 4 months ago

In format: html, the following document will first show the echoed code, then a two column layout with a total of three plots and a data frame. In format: closeread-html, the echoed code shows up after the two column layout.


---
title: "Orphaned code output test"
format: closeread-html
---

:::{.cr-layout}

Test test test

:::{.cr-crossfade cr-to="big-code"}
Test test test
:::

Test test test

```{r}
#| echo: true
#| cr-id: big-code
#| layout-ncol: 2

a <- rnorm(100)
b <- 1 + rexp(100)
c <- a + b

a <- rnorm(100)
b <- 1 + rexp(100)
c <- a + b

a <- rnorm(100)
b <- 1 + rexp(100)
c <- a + b

plot(mtcars$mpg, mtcars$dist)

plot(mtcars$hp, mtcars$dist)

plot(mtcars$hp, mtcars$mpg)

mtcars
\```

:::
andrewpbray commented 4 months ago

Initial diagnosis

When our lua filter encounters the layout div, in the AST the code cell looks like:

Div {.cell cr-id="big-code" layout-ncol="2"} Blocks:

  1. CodeBlock {.r .cell-code}
  2. Div {.cell-output-display} Blocks:
    1. Image
  3. CodeBlock {.r .cell-code}
  4. Div {.cell-output-display} Blocks:
    1. Image
  5. CodeBlock {.r .cell-code}
  6. Div {.cell-output-display} Blocks:
    1. Image
  7. CodeBlock {.r .cell-code}
  8. Div {.cell-output-display} Blocks:
    1. CodeBlock

That is, it separates the code into different CodeBlocks based on the lines that produce output, then interleaves those CodeBlocks with output (3 images and a CodeBlock to print the dataframe).

At the end of our lua filter, the whole DivAbove is wrapped into two more Divs:

Div {.sticky-col} Blocks:

  1. Div {.sticky-col-stack} Blocks: DivAbove

That looks about right! The fact that in the flat html format the CodeBlock is not separated and interleaved into the output suggests the filter for layout-ncol runs after our filter does to rearrange those blocks. That rearrangement must be breaking out Div wrapper structure.

Next steps: write a separate lua filter that runs late to see if we can catch the AST post-layout filter.

andrewpbray commented 4 months ago

@jimjam-slam Oy, this was a deep cut:

https://github.com/quarto-dev/quarto-cli/discussions/10174

I think the issue is that we're looking for cr-id's which no longer are parents to the CodeBlock, so it gets treated as a separate thing.

jimjam-slam commented 4 months ago

Dang, nice work on the diagnosis!

jimjam-slam commented 1 month ago

Circling back to this and updating the test case following the shift to Quarto cross-ref style trigger references:

---
title: "Orphaned code output test"
format:
  closeread-html:
    theme: cosmo
---

:::{.cr-section}

Test test test

Test test trigger big code @cr-big-code

Test test test

:::{#cr-big-code}
```{r}
#| echo: true
#| layout-ncol: 2

a <- rnorm(100)
b <- 1 + rexp(100)
c <- a + b

a <- rnorm(100)
b <- 1 + rexp(100)
c <- a + b

a <- rnorm(100)
b <- 1 + rexp(100)
c <- a + b

plot(mtcars$mpg, mtcars$dist)
plot(mtcars$hp, mtcars$dist)
plot(mtcars$hp, mtcars$mpg)
mtcars

:::

:::

All done!



On Safari:

<img width="1570" alt="Screenshot 2024-09-24 at 09 42 28" src="https://github.com/user-attachments/assets/2ceaf17b-2532-42be-9dc1-4fd7cd18a9ab">

So there's still a layout problem even though we encourage folks to wrap the entire code cell in a fenced div (rather than adding `id: cr-...` directly to the code cell.

But the ID is in the right place, so it's no longer a problem with the figure layout processing — I'm assuming this is one of the many possible instances of #78 (ie. what we do with mixed content stickies).