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

Ensure autoHeader dom result is similar to parsed H1 #811

Merged
merged 1 commit into from
Feb 4, 2020
Merged

Ensure autoHeader dom result is similar to parsed H1 #811

merged 1 commit into from
Feb 4, 2020

Conversation

cyrilf
Copy link
Contributor

@cyrilf cyrilf commented Apr 6, 2019

fix #516

Please makes sure these boxes are checked before submitting your PR, thank you!

  • Make sure you are merging your commits to master branch.
  • Add some descriptions and refer relative issues for you PR.
  • DO NOT include files inside lib directory.

In reference to #516
Ensure the h1 dom element created by the autoHeader feature is similar to the ħ1 parsed from a markdown file.

-<h1>Title</h1>
+<h1 id="title">
+  <a href="#/?id=title" data-id="title" class="anchor">
+    <span>Title</span>
+  </a>
+</h1>

Please review this code carefully as I don't know if exposing the renderer is a good idea.

dom.before(main, h1)
const h1 = this.compiler.header(activeEl.innerText, 1)
const wrapper = dom.create('div', h1)
dom.before(main, wrapper.children[0])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because h1 is returned as a String, I use this trick to convert it into a DOM node

@timaschew
Copy link
Member

Please run these commands to trigger netlify deployment for this Pull Request:

git commit --amend --no-edit
git push --force

@cyrilf
Copy link
Contributor Author

cyrilf commented Apr 13, 2019

Done. ✔️

@timaschew timaschew changed the base branch from master to develop April 14, 2019 15:22
@timaschew
Copy link
Member

@jhildenbiddle what do you think?

@jhildenbiddle
Copy link
Member

Is there a preview for this PR? I don't see a link...

@timaschew
Copy link
Member

timaschew commented Apr 21, 2019

Oh, it seems that it’s visible just to me.
https://deploy-preview-811--docsifyjs.netlify.com/#/

I will try to find out how to make them public. And will also add you as a team member in netlify.

@timaschew
Copy link
Member

I don’t think it’s bad to expose the renderer.

@stale
Copy link

stale bot commented Feb 4, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Feb 4, 2020
@cyrilf
Copy link
Contributor Author

cyrilf commented Feb 4, 2020

Any update on this PR?
Just received the notification from the bot but I still think it's a nice to have for more consistency. (haven't check master recently tho' so I don't know if it's still relevant 🙈 )

@stale stale bot removed the wontfix label Feb 4, 2020
@anikethsaha
Copy link
Member

Please rebase.

src/core/render/compiler.js Outdated Show resolved Hide resolved
@cyrilf
Copy link
Contributor Author

cyrilf commented Feb 4, 2020

@anikethsaha Rebased and updated your requested change ✌️

Copy link
Member

@anikethsaha anikethsaha left a comment

Choose a reason for hiding this comment

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

Nice work 🎉
Sorry for the delay

@anikethsaha anikethsaha merged commit 2928eb7 into docsifyjs:develop Feb 4, 2020
@cyrilf cyrilf deleted the feature/autoheader-h1-link branch February 4, 2020 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Render H1 elements added via autoHeader as anchors
4 participants