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

feat: single page #1035

Merged
merged 26 commits into from Aug 13, 2020
Merged

feat: single page #1035

merged 26 commits into from Aug 13, 2020

Conversation

hugomrdias
Copy link
Contributor

@hugomrdias hugomrdias commented Jul 30, 2020

TODO:

  • fix link render hook to use hash based links
  • remove status badges from Toc
  • fix math-mode
    • fix katex css and font loading
  • fix org-mode
    • can we modify org-mode headers
    • or at least fix the heading numbering?
  • make sure refs, images and shortcodes all work in single layout
    • figures use zoomable.js and a page level scratch to detect if the js has already loaded. This fails in single page mode, zoomable is loaded multiple times. JS init ordering needs to be controlled.
  • ensure stable section numbering
    • wrapping pages in sections to allow in page js to scope to a chunk breaks the assumptions of css based counters where it changes the nesting of heading elements
  • ensure render perf is acceptable
    • single page is chonky boi. dont pass entire body element to katex... toc building is slower than we'd like too.

@vmx
Copy link
Contributor

vmx commented Jul 30, 2020

Will there only be the single page mode or both? I really like the multi-page mode, especially if you are editing maths, that's super useful as it doesn't need to render a huge spec.

Copy link
Collaborator

@olizilla olizilla left a comment

Choose a reason for hiding this comment

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

Notes from our pairing session

*
* @author Tim Scanlin
*/

Copy link
Collaborator

Choose a reason for hiding this comment

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

@hugomrdias pls add a comment here to note that this is a customised version of tocbot, with bullet points of what changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • removed click event
  • made scroll/resize passive events
  • cached tocElem querySelector
  • made scroll handler debounced instead of throttled

@@ -2,6 +2,7 @@
title: Proof-of-Replication
bookCollapseSection: true
weight: 2
bookhidden: true
Copy link
Collaborator

Choose a reason for hiding this comment

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

@hugomrdias mentioned this is hidden as it's org-mode rendering is causing issues and the section is likely to be removed in favour of sdr

go.mod Outdated
@@ -3,6 +3,6 @@ module spec
go 1.14

require (
github.com/alex-shpak/hugo-book v0.0.0-20200715185111-e91fa9024b07 // indirect
github.com/alex-shpak/hugo-book v0.0.0-20200723091818-5eceee52edd0 // indirect
Copy link
Collaborator

Choose a reason for hiding this comment

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

how do we pin go deps?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This came up due to a build failure last week that I think was caused by the same issue I hit today in #1048

{{- if eq .Level 1 -}}
{{- $anchor = printf "%s" (delimit $pathParts "$") -}}
{{- else -}}
{{- $anchor = printf "%s--%s" (delimit $pathParts "$") .Anchor -}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can simplify this and change the delimeter... ideally we'd match the old anchors

Copy link
Collaborator

Choose a reason for hiding this comment

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

@hugomrdias mentioned, this is also the line we need to handle the anchor rewrite

Copy link
Contributor Author

Choose a reason for hiding this comment

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

here we also need to handle the case of a file named index.md using the same behavior as _index.md

hugomrdias and others added 3 commits July 31, 2020 19:30
@olizilla
Copy link
Collaborator

olizilla commented Aug 3, 2020

I've fixed the header rendering hook so that each h1-h6 element is rendered inside it's own header element which can contain other content, such as spec-section-status badges, without those appearing in the content, and brought the branch up to date with beta. It's all looking pretty nice now: https://bafybeif2nshn25cowmv64rchk2h2eyw4cccwd7nfhibpklxranc43ijlt4.ipfs.dweb.link/

The content of the heading elements is used to create the Toc, so including anything other than the heading in them is troublesome, as it will appear in the ToC. Pulling them out of the heading element fixes that, but then breaks the numbering, as that is done via CSS counters, which were assuming the heading elements would all be direct children of the body. That's fixed with a CSS tweek.

Alas now we hit a similar problem with math/katex support... We can't run katex on the entire document; in single page mode, it is way too big. So we need to wrap the sections that use katex in an element so we can reduce the scope, but that again will break section numbering. Hopefully that can be fixed with moar css.

Both the ToC creation and the math mode processing should ideally be pulled to a build time rather than run time step.

@vmx
Copy link
Contributor

vmx commented Aug 3, 2020

I'd like to mention that build-time KaTex is possible with a fork of Hugo (it's really a small patch), the details are outlined in this Blog.

Sadly this patch won't be applied upstream as it's using CGo. So if someone would write a version of goldmark-qjs-katex that is using a JS implementation that runs in pure Go (like e.g. goja, it might be able to be upstreamed into Hugo.

If it would be me, I'd just use the fork mentioned above for now and explore future improvements on it when it turns out it's not a viable solution in the long run.

@olizilla
Copy link
Collaborator

olizilla commented Aug 4, 2020

@vmx i was initially strongly against relying on a forked hugo... i have it working with client side rendering, but the more I think about it, the more I think you are right... I should stop fighting hugo and start forking it instead. We don't need to keep up with changes in hugo, we just need a stable platform that works for the content we have.

Beyond math content, the current ToC situation is particularly egrigious, as hugo has the data on in page headings, but only exposes it as pre-formated html ToC that we can't re-use to build our the all-pages mega ToC. If we fork hugo we could expose the page level headings as data and template the ToC we need at build time.

I shall explore.

ensure all the js that is needed is bundled and initalised in an orderly manner.

previously, shortcodes would include script elements to a remote cdn which can lead to many copies of the same lib being requested in single page mode.

License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
@olizilla
Copy link
Collaborator

olizilla commented Aug 5, 2020

Single page, Links, ToC, and math-mode are all working in this PR, but the heading numbering remains fragile. The current solution uses heading-hooks to catch in-page sub-headings, and update their id, and format them with a status badge etc... alas that heading-hook feature is provided by the goldmark markdown parser, so org-mode docs do not trigger it, and have incorrect headers, and are currently missed in the section numbering scheme... which leads to subsequent section numbers not matching between the ToC and the actual headings... (of note, the same problem exits in the old version of site)

I am looking into solutions.

css counters are scoped to the element that first declares them.
This change declares the counters at the content root, so that heading elements
share them regardless of nesting. This also means that we have to clear out all
the lower order counters for every header as they are shared.

License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
@olizilla
Copy link
Collaborator

olizilla commented Aug 6, 2020

I have

  • fixed the heading numbering issue with some more css counters magic
  • fixed the katex rendering of math content by scoping the sections that katex needs to parse, and fixing the katex css and font loading.

My biggest concern with this branch right now is the perf and UX of the ToC which I will look at today. The "follow on scroll" makes it feel jumpy when clicking between sections, and the initial render is slower than I'd like.

There is also the issue of whether building the ToC client side is an acceptable solution.
Pros
+ No need to manually write out the ToC structure in front matter (this was causing discrepencies between the ToC and page content in the original spec site)
+ No need to fight hugo... There is no obvious way to get the in-page heading structure from each markdown doc and use it to genrate the ToC we need at build time... (though i need to double check what the old site is doing there.)

Cons
- flash of missing ToC when loading the page.
- no ToC if JS is disabled.

@olizilla
Copy link
Collaborator

olizilla commented Aug 6, 2020

Just so we are all on the same page, the original spec site relied on authors to add front-matter at every level in the doc to describe the ToC headings that should be build, duplicating the heading structure in the document. It was a manual process that would get out of sync, and leave the ToC and the content not matching up e.g.

Screenshot 2020-08-06 at 10 23 13

The manual nature of it was pleasingly simple, but there are now several examples across the original site where the ToC and the content do not match.

This branch does not attempt to get hugo to build a ToC for the single page view. In page JS grabs all the headings from the content and builds the ToC on the client, so it's always includes the same items. There is no functionality in hugo to get all the headings from all the pages in a way that lets you build a ToC. Am on the look out for a neat hack or workaround that will let us do it.

However while the section numbering is handled by CSS it does not gurantee that the numbers will always match up. If we jump from an h1 to an h3 in a markdown document, with no intermediate h2, the content will correctly number these sections as

1. Foo bar as a h1
1.0.1  baz is an h3

while the ToC will number them

1. Foobar as an h1
1.1 bas is an h3

arguably, this is an error with the content that should be fixed, but I just want to highlight the fragility of using css to handle the numbering. That said, it is a lot more stable / consistent in this branch.

re-use the portable link template :woo hoo:

License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
@olizilla olizilla added the hint: beta Hint: Issues related to the beta branch label Aug 6, 2020
or it should get its own doc

License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
@olizilla
Copy link
Collaborator

olizilla commented Aug 6, 2020

org-mode sections remain a problem.

In the original site it looks like we were using ox-hugo to transform the org-mode docs into markdown before letting hugo parse them. Right now we are leaning on hugo's built in support, which means fewer deps, which is nice for authors, but it also doesn't work with our heading re-writing, so if we are goinng to add more org-mode docs in the future we need to find another solution, which is probably switching back to using ox-hugo to pre-process them.

License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
@vmx vmx mentioned this pull request Aug 7, 2020
@yiannisbot
Copy link
Collaborator

@olizilla these are really great achievements and I think that the pros outweigh the cons. Having katex rendering scoped is also very reasonable, given a minority of sections need it.

I have found a bug in the numbering. It seems that subsections and subsubsections counters do not reset on section start. So, if section 1 goes up to 1.6.9, then section 2 starts from 2.7.10 - see screenshots below.

section2
section3
section4

@olizilla
Copy link
Collaborator

olizilla commented Aug 7, 2020

@yiannisbot yep that is an known issue, it affects Chrome, but not Firefox. Good spot though, remain vigilant. ✊

This is a loving homage to the original ToC. It brings back all the old behaviours and great perf. TocBot perf is terrible on a large doc, so it is removed, in favour of the old ways.

Fixes up resource loading, notably defering katex till well after the page is loaded, to speed up ToC build and reduce the time to get to a useable document.

License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
numerous content heading fixes

License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
- fix toc and content numbering stability
- remove header element that wraps headings to avoid issue with org-mode docs not getting headers re-written
- fix missing section headers in content that ment the listings and appendix were missing
- fix nesting of glossary headings

License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
@olizilla
Copy link
Collaborator

  • section numbers are fixed, and stable between ToC and content.
  • multiple content errors fixed, bringing back glossary and figures sections
  • header nav fixes for small screens

now i'm tweaking the in page fragment links to match the old ones where possible and tracking down some broken links

Screenshot 2020-08-12 at 11 35 26

@olizilla
Copy link
Collaborator

Broken links are fixed, now verifying and simplifying the logic that maps the headers and links.

- match the old spec for generating heading ids and linking to them
- fix issues with old content missing h1 elements.
- document what is going on in render-heading and render-link so others may suffer less.

License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
@olizilla olizilla marked this pull request as ready for review August 13, 2020 13:45
@olizilla
Copy link
Collaborator

olizilla commented Aug 13, 2020

Just so its on the permenant record: From adopting and working on this PR, I still dont think it is in our best interests to coerce hugo to build out a single page website, but in the sprit of "agree and commit" hugomrdias and I have got it working. It re-introduces some complexity that we don't have in the multi-page site (e.g custom id generation, header re-writing, link re-writing). We are working against the grain of what hugo is designed for.

Long term, we will get better results on this project if we make the specs site be multi-page, or use a tool that is designed building out single page spec sites e.g. remark.js or bikeshed or similar.

Regardless, this PR re-introduces single page mode, and has far fewer inconsistencies than the original spec site.

License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
License: MIT
Signed-off-by: Oli Evans <oli@tableflip.io>
@olizilla
Copy link
Collaborator

@hugomrdias pls review this when you get back. The old CSS needs cleaning up, as do a lot of the old templates. It's now very hard to justify using the book hugo theme, as we have to override in both templates and css. The code would be simpler if we just started from scratch.

On which, the perf of TocBot was terrible, and I couldn't face hacking it better, when we also had the request to re-integrate the toc slider, so I wrote a relatively simple and quick toc-builder and ported the toc collapsing logic from the original spec site, so it now feels much more similiar to the orignal site, and the ToC doesn't grind to a halt while it tries sets up hundreds of click handlers and scroll watchers.

Would be worth reviewing those parts at least.

@olizilla olizilla merged commit 7f6205d into beta Aug 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hint: beta Hint: Issues related to the beta branch
Projects
None yet
4 participants