-
-
Notifications
You must be signed in to change notification settings - Fork 214
Table of contents layout #552
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!! I've left a few lil notes
src/website/layout.gleam
Outdated
pub fn table_of_contents_from_djot(document: jot.Document) -> List(ContentLink) { | ||
document.content | ||
|> list.fold(from: [], over: _, with: fn(accum, block) { | ||
case block { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Flatten these nested case expressions and the list.first
into a single case expression please.
src/website/layout.gleam
Outdated
let href = | ||
"#" | ||
<> dict.get(attributes, "id") | ||
|> result.unwrap("") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Skip if id is missing instead of rendering an invalid link
src/website/layout.gleam
Outdated
]) | ||
} | ||
|
||
pub fn with_toc( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No abbreviations in Gleam code please!
src/website/layout.gleam
Outdated
@@ -0,0 +1,292 @@ | |||
import gleam/dict |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merge this module back into the page module please.
The fact the PagaMeta
data type no longer has an appropriate to live without creating dependency cycles shows that this is one module and not two.
src/website/layout.gleam
Outdated
ContentLink(title: String, href: String, children: List(ContentLink)) | ||
} | ||
|
||
pub fn table_of_contents_from_djot(document: jot.Document) -> List(ContentLink) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this used anywhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not currently, but it will be as soon as you use djot to write a guide like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ooo smart
src/website/site.gleam
Outdated
/// Social media share preview image name | ||
preload_images: List(String), | ||
) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This belongs in the page module!
Includes moving the layout module back into the page module, making the djot table of contents much less ugly, and improving code style (toc -> table_of_contents)
Thanks, I've implemented much of this feedback now. |
foolish that I did not include this in the last commit but alas
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!!
G'day!
Today I come bearing a new page layout, the table of contents layout! I've had this one in the pipelines for a bit now, but I've properly factored it as a separate unit so it can be used now.
This includes a small refactor, adding a new layout.gleam file with our basic layout functions, giving some clearer distinction between page content and layout things. It also includes a function that parses Djot document content to produce a List of ContentLinks, used to build the table of contents in the layout.
For now, since I want to start small(er), I have just set it up manually on the two deployment pages.