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

Anchor links #143

Closed

Conversation

fabiosantoscode
Copy link
Contributor

Okay, this implements anchor links in headings from markdown text!

I love anchor links, so I really had to do this one.

@fabiosantoscode
Copy link
Contributor Author

(Fixes #52)

@fhemberger
Copy link
Contributor

Great idea, still needs some fine tuning (see the link on the bottom right):

https://nodejs.org/en/about/working-groups/#website
bildschirmfoto 2015-09-14 um 18 40 23

Also a little bit of padding would be nice, so the hash doesn't touch the headline. Maybe also a more subtile hover style for those links without the green background?

@fabiosantoscode
Copy link
Contributor Author

Yes, good idea. The font might be too bold too.

@phillipj
Copy link
Member

Nice, looking forward to this landing 👍

On Monday, September 14, 2015, Fábio Santos notifications@github.com
wrote:

Yes, good idea. The font might be too bold too.


Reply to this email directly or view it on GitHub
#143 (comment)
.

@mrkiffie
Copy link
Contributor

I've forked @fabiosantoscode's branch here, resolved conflicts and extended the renderer function to support markdown headings that contain links. Here is the comparison with master. Any thoughts?

@phillipj
Copy link
Member

Nice refactoring w/tests @mrkiffie 👍 Maybe you could address the styling issues @fhemberger pointed out aswell while you're at it?

Also a little bit of padding would be nice, so the hash doesn't touch the headline. Maybe also a more subtile hover style for those links without the green background?

@mrkiffie
Copy link
Contributor

Ok, I'll have a look at those style tweaks

@mrkiffie
Copy link
Contributor

@phillipj, I've updated the styles to be similar to those of the attachment in #52.
I also simplified the markup that was being generated as it doesn't appear to serve a purpose.

@phillipj
Copy link
Member

Very nice @mrkiffie! I would say your branch looks mergeable after squashing your commits into one and opening a PR from your fork.

@phillipj
Copy link
Member

Fixed with 294af4f, thanks to both of you 👍

@phillipj phillipj closed this Sep 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants