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

Add pre and post fields for overriding ToC heading #8643

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

@IveGotNorto
Copy link
Contributor

@IveGotNorto IveGotNorto commented Jun 15, 2021

Pre and post allow users to add semantically correct headings to ToC
without breaking existing functionality.

See #8338

@CLAassistant
Copy link

@CLAassistant CLAassistant commented Jun 15, 2021

CLA assistant check
All committers have signed the CLA.

@jmooring
Copy link
Member

@jmooring jmooring commented Jun 15, 2021

@IveGotNorto

I pulled this locally for some quick testing. Comments...

  • If pre is not defined in the site configuration, its value should be <nav id="TableOfContents">

  • If post is not defined in the site configuration, its value should be </nav>

  • I should be able to override pre independently of post. For example:

    [markup]
      [markup.tableOfContents]
        pre = "<nav><h1>Navigation</h1>"
  • I should be able to override post independently of pre. For example:

    [markup]
      [markup.tableOfContents]
        post = "</nav><!-- end of nav -->"
  • If pre = "" in the site configuration, the outer <ol> or <ul> should not be preceded by anything (i.e., do not insert <nav id="TableOfContents">).

  • If post = "" in the site configuration, the outer </ol> or </ul> should not be followed by anything (i.e., do not insert </nav>).

@jmooring
Copy link
Member

@jmooring jmooring commented Jun 15, 2021

@IveGotNorto Thank you for working on this!

@jmooring
Copy link
Member

@jmooring jmooring commented Jun 18, 2021

@IveGotNorto

Although this is working as intended, the defaults for pre and post should not be nil.
https://github.com/gohugoio/hugo/pull/8643/files#diff-be327cc7b6df56b7b25c4775d78955598913504707bd422132aec74e1b5ffc39R169-R170

The defaults should be <nav id="TableOfContents"> and </nav>.

Pre and post allow users to add semantically correct
headings to ToC without breaking existing functionality.

See gohugoio#8338
@bep
Copy link
Member

@bep bep commented Jun 19, 2021

See #8670

@IveGotNorto
Copy link
Contributor Author

@IveGotNorto IveGotNorto commented Jun 21, 2021

Not really sure where this PR stands. Should I be making some different changes related to #8670? @bep @jmooring

@jmooring
Copy link
Member

@jmooring jmooring commented Jun 29, 2021

I wanted to revist this today to test your changes, but it no longer builds due to a7e3da2.

I cannot answer your question about where this PR stands in relation to #8670 and #8680. It seems like we're in a holding pattern.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants