Skip to content

Sidebar level link does not work if header starts with numbers #1088

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

Closed
1 task done
StanleyWei opened this issue Mar 23, 2020 · 16 comments
Closed
1 task done

Sidebar level link does not work if header starts with numbers #1088

StanleyWei opened this issue Mar 23, 2020 · 16 comments
Labels

Comments

@StanleyWei
Copy link

StanleyWei commented Mar 23, 2020

Bug Report

Thanks for this great app and I just got one issue for now, if the header text in the document starts with numbers, the auto links in the sidebar will not work. For example:

Header text in the document as follows:

  1. Introduction
    1.1 Purpose

The auto links will be resolved as like http://localhost:3000/#/mybook?id=1-introduction, it will not redirect to the right section when this link was clicked.

If I add any other characters before that, e.g. Chapter 1 Introduction, then it works very well.

I am wondering whether it could support this kind of scenario, or it does support and I may miss something in configuration, please advise. Thanks very much!

Steps to reproduce

  • Change the header text with number started
  • Enable sidebar and ensure the maxlevels include the corresponding header texts
  • Click the sidebar level link

What is current behaviour

  • Sidebar level link does not work if header text starts with numbers, like 1. Overview

What is the expected behaviour

  • Enable auto links of header text to support leading numbers.

Other relevant information

  • Bug does still occur when all/other plugins are disabled?

  • Your OS: Windows WSL

  • Node.js version: v10.16.3

  • npm/yarn version: 6.14.3

  • Browser version: Firefox 74.0

  • Docsify version: docsify-cli 4.4.0

  • Docsify plugins: default

@anikethsaha
Copy link
Member

can you share a reproducible sandbox for this . we have a demo sandbox in the readme.
Also, is it working on docsify version 4.10.2 ?

@StanleyWei
Copy link
Author

You may try the test link below: https://data42.cn/docsify/docs/#/?id=introduction
You will see if click the menu 01: balabala, it will not work. Thanks very much!

Regards,
Stanley

@Koooooo-7
Copy link
Member

hey, it seems the bug about the element id start with numbers, if u want resolve it right now, u can add ur customer header ids , reference this Customise ID for headings.

@anikethsaha How about add a common id prefix by default to avoid those issues?
it looks like

<h3 id="docsify-xxxxxxx">

@anikethsaha
Copy link
Member

anikethsaha commented Mar 24, 2020

@anikethsaha How about add a common id prefix by default to avoid those issues?
it looks like

@Koooooo-7 this is a good solution but then we need to change the ids of the source as well like the markdown rendering as content.
or,
removing the prefix by listening to onclicks, might make it slow

I am pretty sure it was working before but don't know the exact version

@Koooooo-7
Copy link
Member

@Koooooo-7 this is a good solution but then we need to change the ids of the source as well like the markdown rendering as content.or,removing the prefix by listening to onclicks, might make it slow

I mean I just add the prefix on the origin.heading = renderer.heading function .

// it will only work on the titles.
const slug = "docsify-"+slugify(config.id || str);
const url = router.toURL(router.getCurrentPath(), { id: slug });
nextToc.slug = url;
 _self.toc.push(nextToc);
return `<h${level} id="${slug}"><a href="${url}" data-id="${slug}" class="anchor"><span>${str}</span></a></h${level}>`;

I am pretty sure it was working before but don't know the exact version

I get it, in the old version( ~v4.10.2), it will replace the front digit to _$1 in slugify.js.

  let slug = str
    .trim()
    .replace(/[A-Z]+/g, lower)
    .replace(/<[^>\d]+>/g, '')
    .replace(re, '')
    .replace(/\s/g, '-')
    .replace(/-+/g, '-')
    .replace(/^(\d)/, '_$1')
  let count = cache[slug]

But this merge removed it, Fix #895 (#896) in v4.11.0.

  let slug = str
    .trim()
    .replace(/[A-Z]+/g, lower)
    .replace(/<[^>\d]+>/g, '')
    .replace(re, '')
    .replace(/\s/g, '-')
    .replace(/-+/g, '-');
  let count = cache[slug];

@anikethsaha
Copy link
Member

Please try new version 4.11.3 to check if it still exists.

@Koooooo-7 can you check this and see if we can merge these two cases. Else can you create a separate issue. We can surely discuss about it, it looks good to have

@Koooooo-7
Copy link
Member

Please try new version 4.11.3 to check if it still exists.

the latest v4.11.2 is still exist this problem.

@Koooooo-7 can you check this and see if we can merge these two cases. Else can you create a separate issue. We can surely discuss about it, it looks good to have

I opened a new one #1093 , and I may miss some relative issues, u could add them in.

@anikethsaha
Copy link
Member

the latest v4.11.2 is still exist this problem.

you meant v4.11.3 ?

@Koooooo-7
Copy link
Member

the latest v4.11.2 is still exist this problem.

you meant v4.11.3 ?

I didn't find the v4.11.3 tag to checkout. so I test the latest v4.11.2 only.

@anikethsaha
Copy link
Member

image

i released that yesterday.

@Koooooo-7
Copy link
Member

the latest v4.11.2 is still exist this problem.

you meant v4.11.3 ?

I didn't find the v4.11.3 tag to checkout. so I test the latest v4.11.2 only.

image

i released that yesterday.

……okay, maybe I missed something.:cat: could u test the v4.11.3 . If the slugify.js hasn't been updated, I guess the issue is still exist.

@anikethsaha
Copy link
Member

it has been changed in slugify.js . check 154abf5

@Koooooo-7
Copy link
Member

it has been changed in slugify.js . check 154abf5

sounds good. 👍
BTW: I still can't checkout this tag and I didn't find it at the release and repo right now... idk.

@anikethsaha
Copy link
Member

anikethsaha commented Mar 25, 2020

Yea, I think the publish script didn't work as expected. IDK why. I will check that

I ran yarn publish instead of yarn pub 😅

@Koooooo-7
Copy link
Member

Koooooo-7 commented Mar 25, 2020

😂 that's okay. and I tested on the v4.11.3, it works. 💯

@StanleyWei
Copy link
Author

Thank you both very much, @Koooooo-7 @anikethsaha, just tried the latest version and it works well. I will close this issue.

Regards,
Stanley

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

No branches or pull requests

3 participants