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

move anchor-link off of heading text #3357

Merged
merged 3 commits into from May 30, 2013
Merged

Conversation

minrk
Copy link
Member

@minrk minrk commented May 24, 2013

avoids confusion about where to click, matching Sphinx-style output with a clickable , only drawn on hover.

Also moves the relevant style to textcell.less, since it only affects heading cells.

avoids confusion about where to click, matching Sphinx-style output.

Moves the relevant style to textcell.less
@minrk
Copy link
Member Author

minrk commented May 29, 2013

merging soon if no objection

@Carreau
Copy link
Member

Carreau commented May 30, 2013

No objection. Could the same thing be done purely with css ?

heading-anchor{display:none}
h1:hover.heading-anchor{display:visible}

instead of binding one more js event?

@minrk
Copy link
Member Author

minrk commented May 30, 2013

Could the same thing be done purely with css?

might be, I will give it a try.

@minrk
Copy link
Member Author

minrk commented May 30, 2013

yup, much better. Had to use visibility instead of display for some weird reason, because toggling display wouldn't draw it on the same line.

$('<a/>')
.addClass('anchor-link')
.attr('href', '#' + link)
.html('¶')
Copy link
Member

Choose a reason for hiding this comment

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

which is faster .text(...) or .html(...) I guess here you use html() to escape to the html sequence ...

Copy link
Member Author

Choose a reason for hiding this comment

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

might as well use text, I guess

@Carreau
Copy link
Member

Carreau commented May 30, 2013

yup, much better. Had to use visibility instead of display for some weird reason, because toggling display wouldn't draw it on the same line.

Ah, because display can be block or inline... maybe.

Good to merge for me.

@Carreau
Copy link
Member

Carreau commented May 30, 2013

Ok, merging.

Carreau added a commit that referenced this pull request May 30, 2013
move anchor-link off of heading text
@Carreau Carreau merged commit 79df8f6 into ipython:master May 30, 2013
@minrk minrk deleted the heading-links branch June 2, 2013 18:27
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
move anchor-link off of heading text
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.

None yet

2 participants