quarto-dev / quarto-cli

Open-source scientific and technical publishing system built on Pandoc.
https://quarto.org
Other
3.74k stars 305 forks source link

Stylesheet not merging when deploying to GHA #10383

Open jimjam-slam opened 1 month ago

jimjam-slam commented 1 month ago

Bug description

@andrewpbray and I are developing a Quarto extension, and we've deployed a documentation website that uses it. We're using a strategy where a pre-render script copies the extension from the repo root into /docs (where the site lives).

Among other things, the extension contributes a format that includes an SCSS stylesheet. We want this stylesheet to play nice with whatever theme a user provides, so in the extension we have:

# _extension.yml
title: closeread
contributes:
  formats:
    html:
      filters:
        - closeread.lua
      theme: [grid.scss]

Then, our website's _quarto.yml has the following, so that the pages where we aren't demoing the extension don't include these styles:

format:
  html:
    theme:
      - cosmo

Finally, in /docs/gallery/demos we have a _metadata.yml:

format:
  closeread-html:
    theme: [cosmo]

For good measure, on the individual posts inside /docs/gallery/demos we've thrown in:

# docs/gallery/demos/sticky-block-types/index.qmd
format:
  closeread-html:
    debug-mode: false
# docs/gallery/demos/a-poem-about-suffering/index.qmd
format:
  closeread-html:
    theme: nytimes.scss
    code-tools:
      source: true

format.closeread-html.debug-mode is an option we've added (a Lua filter handles that successfully).

In sticky-block-types/index.qmd (first post), we just want grid.scss to merge with cosmo. In the second, we want grid.scss to merge in with nytimes.scss (which is alongside the .qmd, not in the extension folder), not cosmos.

When we push to GitHub Pages using a GHA (which is using Quarto 1.4.549), it seems that grid.scss is not present in the bundled bootstrap.min.css (we can't find any of the classes we've defined, for example), and those styles don't seem to take effect.

When we render this locally, however (both using the latest Quarto pre-release and Quarto 1.4.549), they seem to be present in bootstrap.min.css and work fine:

https://github.com/user-attachments/assets/da3d0cdc-307c-42f8-820b-508eaf8ac973

We're not too sure if:

  1. We're correctly specifying theme options at each step (extension, project, folder metadata, post frontmatter) to allow themes to be merged by extension users,
  2. What order the SCSS files are interleaved in (they shouldn't be overriding each other, but you never know)
  3. Why Quarto seems to correctly merge them all locally but not on GitHub Pages

Steps to reproduce

No response

Expected behavior

No response

Actual behavior

No response

Your environment

No response

Quarto check output

Locally:

Quarto 99.9.9
[✓] 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: 99.9.9
      commit: be2792dd8d0960f1d205aeb62fa3544ec956e6d3
      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
cderv commented 1 month ago

I wonder if what you are encountering is related to this known issue:

In short, all HTML pages in a website are currently unfortunately using the same bootstrap.min.scss. In your example, you'll see all the pages are using https://qmd-lab.github.io/closeread/site_libs/bootstrap/bootstrap.min.css. Same file for your pages in the website using default them, and the ones in gallery/demos that are expected to use a different one.

This means currently a website can't use distinct theme options for different pages or part of the website. This is because the last page that renders will have its theme configuration merge with Bootstrap using SASS to build a css for the site.

it seems that grid.scss is not present in the bundled bootstrap.min.css (we can't find any of the classes we've defined, for example)

Considering this known issue, I am not surprise seeing that index.qmd is the last document build in your example. It would be the default cosmo for html format you set in _quarto.yml will create the bootstrap.min.css

When we render this locally, however (both using the latest Quarto pre-release and Quarto 1.4.549), they seem to be present in bootstrap.min.css

Same considering the known issue, it could depend locally on how the side is built, and the order of the build. If you quarto preview on file of the gallery/demos it would have your theme and replace the previous bootstrap.min.css. Same if quarto render does end up by rendering one of the file with your theme. Though locally I would not expect that it works, as one of docs/gallery/demos/sticky-block-types/index.html and docs/gallery/demos/a-poem-about-suffering/index.html should not have its expected theme.

Anyhow, I am sure that locally you could confirm this is the same problem by looking at bootstrap.min.css depending on what you render and how.

This is really impactful and we need to fix this for sure. This is just not an easy change as it should be designed carefully; The discussion there as some ideas for resolution https://github.com/quarto-dev/quarto-cli/issues/10087#issuecomment-2183026202

@cscheid 1.6 is quite full already, but this issue keeps hitting us regularly... We should really consider this, especially that with _brand.yml feature for website, this will be a bad experience when a user will be trying to override on a single page something. 🤔

jimjam-slam commented 1 month ago

Oh, interesting! If the page render order is different when we render locally, then as you say, that might explain why we see the different results there... will confirm when I can!

jimjam-slam commented 1 month ago

In the mean time, if this is the issue, that gives @andrewpbray and I two ways we can proceed:

  1. Offer a custom project type that specifies the additional style, so that it gets merged into all pages
  2. Rewrite the additional styles as CSS rather than SCSS. We'd hoped to use SCSS variables, but we might be able to work around this!
cderv commented 1 month ago

Rewrite the additional styles as CSS rather than SCSS. We'd hoped to use SCSS variables, but we might be able to work around this!

yes that is a workaround. Depending or your use case, there could be scenario where you defined your variable in your main SASS theme as CSS variable, and then use that in your custom CSS. Quarto also may set some variable as CSS variables like https://github.com/quarto-dev/quarto-cli/blob/ff56839c621fc1fbac2997a38550d4d4f5a939d6/src/resources/formats/html/_quarto-rules.scss#L584-L591 or https://github.com/quarto-dev/quarto-cli/blob/ff56839c621fc1fbac2997a38550d4d4f5a939d6/src/resources/formats/html/_quarto-rules.scss#L721-L733

Though we don't that for everything, and your case can be quite specific. Just mentioning in case somehow in can help right now for a workaround

jimjam-slam commented 1 month ago

Thanks @cderv! 😊

cscheid commented 1 month ago

@cderv yes I'm planning on fixing this for 1.6.