-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Use header's slug as it is if possible #119
Conversation
LGTM |
@@ -59,10 +59,11 @@ exports.extractHeaders = (content, include = [], md) => { | |||
tokens.forEach((t, i) => { | |||
if (t.type === 'heading_open' && include.includes(t.tag)) { | |||
const title = tokens[i + 1].content | |||
const slug = t.attrs.find(([name]) => name === 'id')[1] |
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.
Avoid using Array.prototype.find
since Buble doesn't polyfill for it and it won't work at IE11.
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.
lib/util only runs in Node so it's ok
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.
Oh, excuse me, I forget some important thing.
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.
Run this at local and looks good to me, not check details
Thanks! |
Co-authored-by: meteorlxy <[email protected]>
This fixes an issue concerning missing slug link of sidebar.
Reproduction
The issue can even be reproduced in the official VuePress guide.
Why Not ...?
link in sidebarWe can see that the guide page doesn't scroll to the
Why Not ...?
header.Reason
The issue is caused by markdown-it's
typographer: true
option, which is for beautification. By the option, three dots (...
) are replaced with a single unicode character…
. They are handled differently by slugify().When logging the tokens,
tokens[i + 1].content
is nottypographer
-ed, even its actual slug is.Fix
The token already has an attribute
id
which is a correct and final result of the parser. Also, the slug is used for sidebar links later, so I guess it would be more proper to useid
as it is.Please let me know if there is anything unclear.
Thanks!