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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Markdown generates anchor links in headers #15

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

infogulch
Copy link
Collaborator

@infogulch infogulch commented Apr 29, 2022

I think linkable headers are the best thing since sliced bread and wish people would use them more.

This PR adds rehypeSlug & rehypeAutolinkHeadings packages, part of the markdown parser library system used in the project, to make html headers generated from markdown include link anchors to the header. This is nice for linking to a particular section of a long document, like this: https://percival.ink/#embedding-code <- This PR intends to make links like that valid.

I made it so that it's only visible while hovering a header, and very clear that it's a clickable link with a bit of hover animation This is the "I wish people would use them more" part; maybe if I make the UX really good they'll be more likely to use it 馃樅. You can see a visual demo here: https://codepen.io/infogulch/pen/BaYBZxa?editors=1100 Visually I'm aiming for subtle but obvious, but the style could be tuned.

@infogulch infogulch marked this pull request as ready for review May 2, 2022 18:56
@infogulch
Copy link
Collaborator Author

So it just needed await tick(); and element.scrollIntoView();

Copy link
Owner

@ekzhang ekzhang left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for fixing this! Anchor links are super useful. I think it's a little bit unfortunate that there may be two links to the same ID that conflict if headings in different cells have the same text, but that seems very minor.

Only have a couple minor comments with respect to CSS.

src/components/cell/CellOutput.svelte Outdated Show resolved Hide resolved
background-size: 0.9em;
height: 0.9em;
width: 0.9em;
filter: invert(0.1);
Copy link
Owner

Choose a reason for hiding this comment

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

What is filter: invert(0.1)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a generic way to 'lighten' the icon on hover without duplicating or parameterizing the svg or hard-coding colors. I changed it to 20% to make it a bit more noticeable. I'm mostly just having fun with css here, we can tune or remove it if you like. 馃槃

src/components/cell/CellOutput.svelte Outdated Show resolved Hide resolved
src/components/cell/CellOutput.svelte Outdated Show resolved Hide resolved
@infogulch
Copy link
Collaborator Author

infogulch commented May 6, 2022

Nice callouts on the css, I adjusted it.

two links to the same ID that conflict

I agree it's unfortunate. But this will be very hard to fix automatically without tradeoffs. E.g. say we make the ids unique by adding a numbered suffix, then the user copies a link to the second header, then they change the first header so it's not conflicting anymore: now the link breaks. Also since these are separate instances of the markdown component we'd have to do global analysis of the markdown output and oh my god this is a giant can of worms for the size of this feature just make the user fix it 馃槅.

This way, it's easy enough for the user to see what the problem is, you get nicer ids, and it's a good writing practice to name your headers uniquely anyway (cope).

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