qmd-lab / closeread

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

Add nytimes theme #45

Closed andrewpbray closed 4 months ago

andrewpbray commented 4 months ago

This adds a demo along with a corresponding .scss file that serves as a theme to ship along with the extension.

I haven't been able to figure how to merge this theme (#44) with grid.scss, but for now you can test it by setting theme: nytimes.scss in the _extension.yml.

jimjam-slam commented 4 months ago

Nice! I'm able to enable it by setting theme to [grid.scss, nytimes.scss].

That said, I do think we should consider making a full Quarto theme a separate component — or perhaps we ship a template that includes the additional styles? If we can keep the core styles down to the ones that mechanically make Closeread work, that means users can still use existing Quarto themes to customise it. What do you think, @andrewpbray?

andrewpbray commented 4 months ago

Ah, right, I just learned something that should have been obvious. If you put .scss files in the extension dir, it can be added to docs that use the extension by either adding it to _extension.yml or by hard-coding the path to the file, i.e. _extensions/closeread/nytimes.scss. When I move the scss file into the same dir as the demo, I can get it to appear correctly by using theme: [nytimes.scss].

I'm kinda surprised theme: nytimes.scss doesn't work in that scenario. We're able to put theme: grid in our _extension.yml and it has no problem merging that with the default quarto style sheets even though its not in a yaml array.

My thought was that grid.scss would be the core style sheet that enables closeread functionality but can mesh with whatever bootswatch theme the user wants to use. I was seeing if we could distribute a few themes that were particular to the aesthetics of closeread, primarily modifying padding, fonts, and background colors.

In terms of how to implement that, how about...

  1. We stash themes in _extension/closeread/themes/
  2. The user would use format.closeread.theme: nytimes
  3. The Lua filter (assuming it has access to that field) adds it via quarto.doc.add_html_dependency.

So in the docs, the guidance would be: if you're adding one of the handful of thems that ship with the extension (or could live in a separate closeread-themes extension?) the syntax is theme: mytheme but if you write your own .scss file, include it with theme: [mytheme].

What do you think?

andrewpbray commented 4 months ago

Hmm, when I try to render the nytimes excerpt in the docs gallery, I get this:

Screenshot 2024-07-15 at 9 06 30 AM

This isn't too surprising I guess. In _quarto.yml we have format.html.theme: [cosmo] but since that doc is format: closeread-html it doesn't apply that theme to that page.

Here are a few unsuccessful troubleshooting attempts...

1. Modify project metadata

If you change _quarto.yml to

format:
  html:
    theme: [cosmo]
  closeread-html:
    theme: [cosmo]

It looks like that overrides both grid.scss and nytimes.scss.

Screenshot 2024-07-15 at 9 21 13 AM

2. Modify doc metadata

If instead you change the yaml in the doc to:

format:
  closeread-html:
    theme: [cosmo, nytimes.scss]

It works just fine:

Screenshot 2024-07-15 at 9 33 44 AM

Do you have any ideas why 1 didn't work? I expected it to under the understanding that precedence for metadata went doc > extension > project and that these should be joins, not overwrites.

jimjam-slam commented 4 months ago

I don't think I've seen a project file with multiple formats before! I don't have an intuition about how that works... does it support both, or does one override the other?

jimjam-slam commented 4 months ago

What if we have:

Does that work? Importantly, the themes wouldn't be copied if you simply quarto add the extension, but given they're optional I think that makes sense!

andrewpbray commented 4 months ago

What if we have:

  • grid.scss in the extension folder and specified in the extension YAML with brackets (theme: [grid.scss]), so it's always merged in. 👍
  • separate theme files included in a cr-themes folder in the extension root, so they're copied in when you quarto use template and can be added to the doc with theme: [cosmo, cr-themes/nytimes.scss] 🤔

I'd worry that if we drop them at the root of the project with quarto use then it'd be really easy for a user to muck things up by moving those files (since we'd be hard-coding the path into the Lua filter). It feels like stuff that lives in _extensions.yml is much less likely to be messed with by the user.

That consideration has me slightly favoring a separate cr-themes extension that users would add that'd store the files under _extensions.yml. What do you think?

andrewpbray commented 4 months ago

And about the multiple formats, I've never understood that 🙃

To recap what I do know: format: html both sets the output format for the doc, and also is a necessary key for specifying any format options. That's becomes a problem when you're trying to set the metadata for multiple formats with one file type. I think the architecture they had in mind was that you'd separate docs into dirs by their format, and into each dir put a _metadata.yml file. That'd actually work in our case since all of the demos will be format: closeread-html but I'm not sure about the tradeoff with clarify of the source for anyone who peeps into the repo to look at the document source. It's a pretty innocuous addition - just for the website nav - so I might go ahead and drop in a _metadata.yml file...

andrewpbray commented 4 months ago

In bd376a4 I've moved the shell commands that copy the extension into a pre-render script so that it's one less thing to worry about.

I feel like there's room here for pkgdown-like website template here...

andrewpbray commented 4 months ago

This guy seems to be working as intended, so I'm going to go ahead and merge. 🚀