Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

full-page templates #253

Closed
wants to merge 10 commits into from
Closed

full-page templates #253

wants to merge 10 commits into from

Conversation

Fil
Copy link
Contributor

@Fil Fil commented Nov 28, 2023

Full-page templates as a custom user function. The API is based on simple strings (no dependency on Html). The default page template computes the sidebar navigation and the pager.

(This will also close #60 if we provide a dashboard.css.)

@Fil Fil mentioned this pull request Nov 28, 2023
src/render.ts Show resolved Hide resolved
Copy link
Member

@mbostock mbostock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about if we just did it in JavaScript? The config.js could expose a template function, and the default could be the one we have here.

@Fil

This comment was marked as resolved.

@Fil Fil marked this pull request as ready for review November 30, 2023 14:48
@Fil
Copy link
Contributor Author

Fil commented Nov 30, 2023

PTAL

@@ -0,0 +1,150 @@
import {parseHTML} from "linkedom";
Copy link
Contributor Author

@Fil Fil Nov 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit of churn is due to moving the default rendering function and its utilities into a separate page.ts. I was (pleasantly) surprised that parseHtml is only needed for the toc.

* for pages in a subdirectory, etc.
*/
root?: string;
},
Copy link
Member

@mbostock mbostock Nov 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we instead passed a declarative representation of the page state — rather than partially rendered HTML fragments — so that the template has complete flexibility in turning the declarative representation of the page into HTML (or anything else, e.g., TeX)? Though that requires us to design an intermediate representation as part of the public API. Worth the effort? 🤔

I also think we should encourage people to use the Html helper here because any template implementation will be prone to escaping bugs without it. (The Html helper is not Hypertext Literal (htl) — htl runs in the browser.)

I suspect we will also need to have an API for the CLI, declare a main or exports in package.json, and probably run tsc to convert TypeScript to JavaScript so that people can import helpers to implement their template function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My approach here is very bare-bones, "stdout"-like. It treats observable markdown as a function that returns HTML fragments that you can just paste together and send as one big string to the browser for a severely unstyled but functional experience:

return title + preloads + module + main

All these fragments are made of safe HTML (unless I made a mistake). Users can assemble them in their own "page templates" (or placeholders) with zero difficulty. Pareto is happy.

A (crazy, or "pure") idea would be to return all of this as a single html fragment, and let the default template collect the elements with a querySelector (it already does it to build the TOC, so it wouldn't be much more costly). But probably too crazy.

Some elements are not HTML (path, root and the front-matter data); maybe we should put them in a separate argument?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mbostock could you explain your declarative representation of the page state idea?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes… I need more time to think about this one and then come up with a more actionable proposal.

options: Omit<Config, "template">
) => string;

export const page: Template = ({path, data, preloads, module, main, title, base, root}, options) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like we want to eventually add other parameters for overriding other HTML fragments - like footer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, no problem!

${title}${base}<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
<link rel="stylesheet" type="text/css" href="https://fonts.googleapis.com/css2?family=Source+Serif+Pro:ital,wght@0,400;0,600;0,700;1,400;1,600;1,700&display=swap">
<link rel="stylesheet" type="text/css" href="${root}_observablehq/style.css">${
data?.template ? `\n<link rel="stylesheet" type="text/css" href="${root}_observablehq/${data.template}.css">` : ""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

data in this case comes directly from the front matter but what if the user wants to apply the same styling across all pages? maybe I am missing something...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree we'll want to allow customization of the global CSS (adding a stylesheet, or modifying the base stylesheet), but it's not in this PR's scope.

@Fil Fil marked this pull request as draft December 1, 2023 08:06
@Fil Fil mentioned this pull request Dec 1, 2023
@cinxmo cinxmo mentioned this pull request Dec 1, 2023
@Fil Fil closed this Jan 29, 2024
@Fil Fil deleted the fil/template branch February 2, 2024 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Named themes
3 participants