Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

add grid.scss to docs sitewide yaml #53

Merged
merged 14 commits into from
Jul 31, 2024
Merged

Conversation

andrewpbray
Copy link
Contributor

No description provided.

@andrewpbray andrewpbray mentioned this pull request Jul 27, 2024
@andrewpbray andrewpbray changed the title add grid.scss add grid.scss to docs sitewide yaml Jul 27, 2024
@andrewpbray
Copy link
Contributor Author

andrewpbray commented Jul 28, 2024

@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
Copy link
Collaborator

jimjam-slam commented Jul 28, 2024

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
Copy link
Collaborator

In 815efea 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!

James Goldie added 2 commits July 28, 2024 15:00
(Note that poem doesn't seem to be scaling)
@jimjam-slam
Copy link
Collaborator

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@andrewpbray andrewpbray merged commit 7ff0bc8 into main Jul 31, 2024
2 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants