search

qmd-lab / closeread

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

add grid.scss to docs sitewide yaml #53

Closed andrewpbray closed 3 months ago

andrewpbray commented 3 months ago

@jimjam-slam The most recent commit switches from grid.scss to grid.css but strangely it doesn't appear to be finding and including the style sheet. I've checked the docs for syntax on including css files, but I haven't been able to find the error. Maybe I should sleep on it and see it with clear eyes in the morn 😪

On the up and up, the gallery is fixed!

https://closeread.netlify.app/

jimjam-slam commented 3 months ago

It looks to me like the stylesshet is loading just fine!

Screenshot 2024-07-28 at 14 46 04

That said, CSS has not traditionally supported nesting (I believe it's just starting to come in), and it definitely doesn't support // line comments (/* ... */ block comments only in CSS), so grid.css may not work without adjustment. It might make more sense to compile the SCSS rules section down manually to CSS. I'll see if that makes a difference.

jimjam-slam commented 3 months ago

In https://github.com/qmd-lab/closeread/pull/53/commits/815efea1a9ac7d89f0ceb3b053ef308b6924e77d I've renamed grid.css back to grid.scss (but left the section comments and preamble out).

Then, in ea2a2b6 I've manually compiled grid.css from grid.scss using SASS (npm install -g sass; sass grid.scss grid.css). That way we can be sure it's valid CSS.

One frustration of CSS is that it tends to silently fail: you don't get console error messages when the browser finds invalid CSS, it just ignores it or stops processing it 😮‍💨

This seems to be working, though!

jimjam-slam commented 3 months ago

I've done the same for nytimes.css and the styles seem to be working, but the poem doesn't seem to be scaling properly (not sure if that's expected as we merge things in different places).

andrewpbray commented 3 months ago

I never knew that about the nesting! I've learned a lot of new css syntax looking at your work and I attributed it just to additions to the css syntax over the years. I didn't realize that was all made possible by the scss complication process. TIL...

When I check it out locally it looks great. When I look at the test site, nytimes looks like its using a valid grid.css but the other two do not.

Screenshot 2024-07-28 at 7 09 11 AM
andrewpbray commented 3 months ago

Interesting: I don't see grid.css anywhere in the nytimes example. It works because I wrote the nytimes.css file (foolishly) as just tweaks to the main grid.css, so it has all of the cr classes that we need for the functionality. So the question is why the extension grid.css isn't propagating...

andrewpbray commented 3 months ago

Uf, as the commit history will attest, this took some serious troubleshooting.

The issue was that grid.css wasn't getting distributed along with the extension and I couldn't figure out why. After trying several alternative methods of propagating that file to the demos, the only one method I could get working was to just link from the doc to a copy of the css that I'd moved into the doc directory.

I set up a minimal example of an extension and a website and wasn't able to replicate the issue. Then I looked at our css file and found this mysterious line at the bottom:

/*# sourceMappingURL=grid.css.map */

I wondered if quarto would do a scan for that comment to indicate that this was a downstream css and not to be distributed with the extension. I stripped it out and ... it seems to be working now?

andrewpbray commented 3 months ago

Well this is strange. The state of this branch at this moment publishes Minard just fine to quarto pub

https://andrew.quarto.pub/closeread2/gallery/demos/minard-zoom/minard.html

but still doesn't work on the netlify build...

https://closeread.netlify.app/gallery/demos/minard-zoom/minard.html

😜

andrewpbray commented 3 months ago

I tried commenting out part of the workflow that copied the extension - leaving that up to the pre-render script - but still no luck...