render empty toc if needed #331
Conversation
| @@ -69,6 +69,10 @@ | |||
| </ol> | |||
| </nav> | |||
| <script>{/* redacted init script */}</script> | |||
| <aside id="observablehq-toc" data-selector="#observablehq-main h1:not(:first-of-type), #observablehq-main h2:not(h1 + h2)"> | |||
There was a problem hiding this comment.
maybe we could set toc: false on all these tests? not so much for this PR but so that all these tests don't depend on the way the toc is rendered in the future?
There was a problem hiding this comment.
Do you want me to add a config file for all the tests? Kinda related to #312. Maybe I can hack the test somehow to make it the default at least for tests. 🤔
There was a problem hiding this comment.
Some tests need it though. We could enumerate them here:
export async function normalizeConfig(spec: any, root: string): Promise<Config> {
- let {title, pages = await readPages(root), pager = true, toc = true} = spec;
+ let {title, pages = await readPages(root), pager = true, toc = !root.startsWith("test/input/") || root === "test/input/build/config" || root === "test/input/build/subtitle" || … } = spec;but maybe it's better to do nothing.
There was a problem hiding this comment.
I wouldn’t want to bake test-specific logic into non-test code, too.
There was a problem hiding this comment.
In general our method of snapshot testing means widescale changes when the generated HTML changes. I opted out of that for the client init script since it’s minified and impossible to review, but I don’t think we actually need to suppress the TOC output in these test snapshots — it’s truthful to what we want to test here.
There was a problem hiding this comment.
I don't think it's worth the effort to create test-specific configs, but it's good to know that it is a possibility if this gets annoying.
There was a problem hiding this comment.
(For example, if we change whether the TOC is visible by default, or have some more advanced heuristic to decide whether to show it, it’s nice to see that reflected in the snapshots to reaffirm what is changing.)
Fixes #330.