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

Remove undefined heading anchors #463

Closed
Chng-Zhi-Xuan opened this issue Dec 12, 2018 · 10 comments · Fixed by #880
Closed

Remove undefined heading anchors #463

Chng-Zhi-Xuan opened this issue Dec 12, 2018 · 10 comments · Fixed by #880
Assignees

Comments

@Chng-Zhi-Xuan
Copy link
Contributor

Chng-Zhi-Xuan commented Dec 12, 2018

  • MarkBind Version: v1.14.2
Update
Undefined headings are caused by using HTML heading elements (<h1>, <h2>, etc). While Markdown syntax headings (## Heading 2) will have ids automatically created by MarkBind.

There still needs to be a check for creating anchors for headings without IDs.
Bug Image
undefined-heading-bug

What did you expect to happen?
All anchors should have a valid href to jump to.

What actually happened? Please include the actual, raw output.
Panel contents with headings have anchors generated but undefined href.

@damithc
Copy link
Contributor

damithc commented Feb 3, 2019

The very first heading in the landing page is affected by this problem.
image

@Xenonym Xenonym self-assigned this Feb 21, 2019
@Xenonym
Copy link
Contributor

Xenonym commented Feb 22, 2019

Currently, anchor IDs for Markdown headings are generated by markdown-it-anchor. If we manually generate anchor IDs for HTML headings, we may as well generate anchor IDs for all headings ourselves, since they are all HTML by the time we add anchors in Page.js#addAnchors(), and remove markdown-it-anchor.

Does markdown-it-anchor perform any other function other than to generate heading anchor IDs in markdown?

@acjh
Copy link
Contributor

acjh commented Feb 22, 2019

markdown-it-anchor generates anchors, i.e. <a> tags. I believe the heading IDs are generated by markdown-it.

@Xenonym
Copy link
Contributor

Xenonym commented Feb 22, 2019

markdown-it-anchor generates anchors, i.e. <a> tags. I believe the heading IDs are generated by markdown-it.

@acjh If you remove markdown-it-anchor from markdown-it/index.js, none of the headers will have IDs generated. The markdown-it-anchor documentation confirms that it is the one generating the IDs:

All headers above level will then have an id attribute with a slug of their content.

While markdown-it-anchor does have the ability to generate anchor permalinks, we don't use it and generate them ourselves in Page.js#addAnchors().

@yamgent
Copy link
Member

yamgent commented Feb 22, 2019

While markdown-it-anchor does have the ability to generate anchor permalinks, we don't use it and generate them ourselves in Page.js#addAnchors().

I wasn't aware that markdown-it-anchor has such a functionality. Maybe it is worth giving it a try as a solution for this problem?

@Xenonym
Copy link
Contributor

Xenonym commented Feb 22, 2019

I wasn't aware that markdown-it-anchor has such a functionality. Maybe it is worth giving it a try as a solution for this problem?

@yamgent markdown-it only parses Markdown, so HTML headings won't be processed by markdown-it-anchor. We would still have to handle HTML headings manually if we use markdown-it-anchor's permalinks.

@yamgent
Copy link
Member

yamgent commented Feb 22, 2019

I see, I guess if we want to handle HTML headings also, then we might as well generate everything ourselves.

@Chng-Zhi-Xuan
Copy link
Contributor Author

If we were to handle anchor ID generation ourselves, then it will be related to #633.

Since anchors are added on a Page level, using <include> might cause duplicate anchors (with the same ID) on the same page.

@Xenonym
Copy link
Contributor

Xenonym commented Feb 23, 2019

Since anchors are added on a Page level, using <include> might cause duplicate anchors (with the same ID) on the same page.

@Chng-Zhi-Xuan If we generate anchor IDs ourselves, we can actually also dedepulicate them by adding a numerical suffix (eg. same-heading, same-heading-1, same-heading-2 and so on). Doing the anchor generation at the addAnchors stage means that this deduplication can be done even for statically included files via <include>, which will also fix #633.

The only exception then are dynamic includes via <panel>, but I am not sure this can ever be fixed given that MarkBind wouldn't know what the contents of the panel are during page generation.

@Xenonym
Copy link
Contributor

Xenonym commented Mar 7, 2019

After discussion, the behaviour of IDs and anchor links should be as follows:

  • Elements with IDs and anchors: all headings except those from dynamically included panels
  • Elements without IDs and anchors: all panels

@Chng-Zhi-Xuan Chng-Zhi-Xuan changed the title Undefined Heading Anchors Remove undefined Heading Anchors Jun 3, 2019
@Chng-Zhi-Xuan Chng-Zhi-Xuan changed the title Remove undefined Heading Anchors Remove undefined heading anchors Jun 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants