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

Suggestion: adding an option to render a more accessible anchor link #82

Closed
nhoizey opened this issue Feb 25, 2021 · 20 comments
Closed

Comments

@nhoizey
Copy link

nhoizey commented Feb 25, 2021

I've read this great post about accessible anchor links, and I've managed to write a renderPermalink function for markdown-it-anchor that generates the accessible HTML:
https://nicolas-hoizey.com/articles/2021/02/25/accessible-anchor-links-with-markdown-it-and-eleventy/

Unfortunately, I think this can't be a "simple" PR to change the default renderPermalink function, as it changes the HTML structure, and would probably broke visual rendering of every current user.

I see two options:

  • change the default renderPermalink function and release a major version with breaking change
  • add a new option to use this other renderPermalink function only intentionally

First option would enhance accessibility for everyone, but I'm not sure people really pay attention to the major version of a breaking change, so feedback would probably be pretty negative.

What do you think?

@valeriangalliat
Copy link
Owner

valeriangalliat commented Feb 27, 2021

Edit (2021-03-01): TLDR

Edit 2: you know you fucked up your TLDR when the TLDR is that long.

  1. In the suggested implementation, the header anchors are put after the heading element, with a visually hidden alt text as part of the contents of the link as opposed to aria-label, in order to make it work with translation tools, who seem to ignore aria-label.
  2. This causes the alt text to "leak" into search engine results which I find unideal.
  3. This also causes the alt text to show up when rendered without CSS e.g. in RSS readers, but this can be mitigated by doing a separate Markdown render specifically for RSS that wouldn't include the anchor links at all (which seems like a good practice anyways).
  4. This also causes issues with Firefox "reader mode" (headers completely disappearing at worst, or extra whitespace at best) but this is more of a reader mode issue.
  5. The alt text is locale-dependent so it's something to keep in mind as this plugin is generic and not locale-aware.
  6. I'm questioning if the header anchor feature is actually desirable for screen reader users, I'm torn between making all features accessible, and purposely removing this feature from the screen reader experience in order to have a smoother experience that's more focused on the content, and doesn't have the noise of a feature that I deem not critical and not used the vast majority of the time.

My proposition so far:

Offer an accessibleRenderer function to be used as renderPermalink: anchor.accessiblePermalink(title => `Section titled ${title}`). This would put the anchor after the heading element, with the alt text as aria-label to avoid the caveats of the version using a visually hidden span.

Make the header anchor aria-hidden by default; this plugin would mimic GitHub's behaviour on markdown header anchors. Unlike now where the non-hidden anchors cause a poor UX in screen readers, the header anchors would be purposely omitted from the screen reader experience, on the assumption that they cause more noise than they are useful (from a "plugin default" perspective where I don't know the userbase who's gonna interact with the rendered content).

Solved:

  • I was confused about why repeating the title in the alt text but @nhoizey explained below that screen readers users can commonly request to be read all the links in the page, so that now makes more sense to me since the header anchor won't always be read following the title.

Hey Nicolas! Thanks a lot, this is awesome!

I really want this plugin to be accessible by default. As you pointed out this would be a breaking change, but I'm comfortable changing the DOM structure on a major version.

There's a couple points that I want to discuss before going on with this change;

Rendering without CSS

This "requires" some CSS for an "acceptable" result. To illustrate this, I'll quote David's comment on your article:

@nhoizey pour info, le rendu dans mon lecteur RSS n'est pas terrible (répétition du titre)

I wouldn't have thought about this, but this reminds me that there's all sorts of tools that could consume the HTML and embed parts of it somewhere else without applying CSS.

As you responded, this could be mitigated using inline CSS instead of a class, but some RSS readers or similar software might strip or just ignore inline CSS. This also applies to search engines, which might not be ideal?

Screenshot_2021-02-27  Accessibility check site amberwilson co uk - Google Search

Screenshot_2021-02-27  Were my Anchor Links Accessible - Google Search

Right now the default behaviour is not ideal for screen reader users. Do I prefer it to be not ideal for RSS readers users, and (to some extent) search engine users instead? I would rather not have to answer to that question, ideally nobody should find the default behaviour "pas terrible".

I like the idea of having the title itself be the anchor, but this is probably too uncommon of a practice to be the default, and as pointed out, it comes to the expense of being able to easily select parts of the title, or for what it's worth, being able to include other links in the title.

→ Even though the aria-label option might have other drawbacks, I think it's overall a more sensible trade-off, as it wouldn't have side effects on RSS / search engines display, and still seems that it would be picked up by screen readers in the majority of cases.

Alt text content

While there's a lot of details about the chosen structure in Amber's article, there's no explanation on why the text "Section titled <TITLE>" was chosen.

I have no experience with using screen readers and I'm more than happy to trust people who are more knowledgeable than me on the subject, that being said I would love to learn what's the reasoning behind this text.

My uneducated guess would have been to name it "Link to this section" because I assume that if I was using a screen reader, I wouldn't want to be read the whole title twice every time? And maybe as a screen reader user I wouldn't even want to hear a header anchor after every title in the page, but that's my next question.

We also have to keep in mind that this plugin is not locale-aware, so I can't hardcode a string like "Section titled" in the source. I can make this an option but then the accessible version can't be the default which somewhat makes me sad.

Are headers anchors important enough to justify interrupting the content flow?

Even though marking focusable elements as aria-hidden is a bad practice (from my understanding, because it removes functionality from screen reader users), it might be a sensible choice for a feature like header anchors (which is possibly why GitHub made that choice in the way they render anchors in Markdown pages?), akin to the way we can decide to remove functionality from a website when rendered on smaller screens.

As a screen reader user, is it more important to have the header anchor text presented after every title to have the option to copy it, or is it more important to keep the content meaningful, where the anchor link could be considered a distraction the majority of the time?

I originally mimicked GitHub's behaviour upon the assumption that it was more important for screen reader users to have the content read uninterrupted by anchor links since they're not useful the vast majority of the time. This was later questioned then removed. But as you show in this issue, this was not enough.

Again I understand why accessibility best practices and tools recommend as a general rule to not hide focusable elements, but I would prefer to have the opinion of regular screen reader users about the particular case of header anchors before taking a decision on this.

Visually hidden side effects

There's other details to be careful about, for example (I'm on Firefox) on Amber's article if I triple click on a title and copy it, my clipboard contains:

What are anchor links exactly?
Section titled What are anchor links exactly?

Or on the one named "Contents", if double click on "Contents" and copy the text, it yields:

Contents
Section

This seems to be related to the fact the <h2> is inline and the anchor is absolute positioned, so not directly relevant as far as markdown-it-anchor is concerned, but I'm not comfortable with a solution that requires visually hidden content, as it can seemingly cause unwanted usability side effects depending on how the user styles and positions the title/anchor and how the browser decides to render it. That's another reason why I would be in favour of using aria-label instead.

Conclusion

I was really excited when I saw that issue and both blog posts, as I imagined a bulletproof way to have accessible header anchors, and my first reaction was "fantastic, let's make this the default".

However while digging into the technical implications of this change and considering other use cases and aspects (RSS, search engines, internationalization), I'm left with the usual conclusion that "there's no silver bullet", and that we need to make choices – which I would rather avoid as I try to keep this plugin as unopinionated as possible (but at that point even keeping the current behaviour would be an opinionated choice).

At the end of the day I'm leaning towards the permalink element being described by an aria-label attribute. As for the content of that attribute, I'm still undecided, but it seems that it would require a mandatory configuration option, and that's a bit too "intense" for me to force that by default, so it could look something more like:

const md = require('markdown-it')()
const anchor = require('markdown-it-anchor')

 md.use(anchor, {
  renderPermalink: anchor.accessiblePermalink(title => `Section titled ${title}`)
})

As for the default behaviour I'm considering, if that makes any sense at all, making the anchors aria-hidden on the assumption that they cause more noise than they are useful (especially with the current behaviour where they are inside the heading element), which is probably the opposite of what you wanted me to do by opening this issue. 😅

The reasoning for that is that it's probably better to have no anchor for screen readers than a bad anchor (like it's the case now), but giving (and encouraging) the option to use the anchor.accessiblePermalink renderer which the user can configure according to their locale and own preferences.

I'll end a long answer by a short question: what do you think?

@nhoizey
Copy link
Author

nhoizey commented Mar 1, 2021

Wow, that's a very detailed answer indeed! 😅

And I must say that everything you say is right, there are multiple caveats I didn't anticipate, so it needs much more thoughts to get it right.

For the CSS use case, on my site I removed the anchor link from the feed, users are much less likely to share a link to a feed item anyway. But I guess we can't simply say Markdown-it(-anchors) to generate two versions depending on the way it's used.

For the SEO, I agree it's not great to have the anchor link text inlined like you show… 🥲

Regarding anchor link text, you can't use just "Link to this section" because screen reader users often ask for the list of links in the page (they also ask for the list of headings), and they would get multiple times the same "Link to this section".

Overall, I think you're conclusion and suggestion is the good one, but I'm not an accessibility expert, so I will ask for advice on Twitter first, before we can continue: https://twitter.com/nhoizey/status/1366476887992729601

@davidbgk
Copy link

davidbgk commented Mar 1, 2021

Visually hidden side effects

Another side-effect, activating the Reader mode on Firefox:

Capture d’écran, le 2021-03-01 à 14 58 26

The extra-space between the title and the content is not ideal and might be confusing.

In the future, Text Fragments might save us a couple of headaches :)

@KittyGiraudel
Copy link

KittyGiraudel commented Mar 1, 2021

Hello there. 👋

While there's a lot of details about the chosen structure in Amber's article, there's no explanation on why the text "Section titled <TITLE>" was chosen.

If I’m not mistaken, this was done following a conversation Amber and I had. I would encourage people not to read too much into it. I don’t think there was such a significant decision behind it. We just thought it was adequate content. I don’t pretend it’s the best solution either. Ultimately, this content could be anything, as long as it’s different from title to title.

As a screen reader user, is it more important to have the header anchor text presented after every title to have the option to copy it, or is it more important to keep the content meaningful, where the anchor link could be considered a distraction the majority of the time?

I am not a screen-reader user, so do not quote me on this, but I would say the content being correctly and semantically marked up is the most important. According to a WebAim survey from 2014, two-thirds of screen-reader users scan headings as the first step of trying to find information on a long web page, so this should stay clean and intact above all.

However while digging into the technical implications of this change and considering other use cases and aspects (RSS, search engines, internationalization), I'm left with the usual conclusion that "there's no silver bullet", and that we need to make choices – which I would rather avoid as I try to keep this plugin as unopinionated as possible (but at that point even keeping the current behaviour would be an opinionated choice).

There is no silver bullet, and you probably won’t even have a consensus from screen-reader users, especially when their experience and habits vary based on their browser and assistive technology of choice. What’s important is to come to a solution that makes it possible for people to link to a specific title of an article, without it being a chore.

At the end of the day I'm leaning towards the permalink element being described by an aria-label attribute.

This might be unwise. aria-label is notoriously bad with content translating services such as Google Translate, which is something that a lot of people use on content-heavy pages such as articles, precisely where one might want/find heading anchors.

For the SEO, I agree it's not great to have the anchor link text inlined like you show… 🥲

I would personally not sweat the SEO topic too much. Ultimately, this is the role of search engine to discern what is relevant content and what is a scam, and anchor links will most likely not being reported as illegitimate content. I think this is a non-issue.


If I had to build that feature from scratch, I would mark up headings like this:

<h2 id="this-is-a-heading">
  This is a heading
</h2>
<a href="#this-is-a-heading">
  <!-- Visual # or any other decorative character/icon of choice -->
  <span aria-hidden="true">#</span>
  <!-- Link accessible name, that is unique to every heading -->
  <span class="sr-only">Link to “This is a heading”</span>
</a>

Listing links or headings through the VoiceOver rotor look nice:

VoiceOver links VoiceOver headings
VoiceOver listing links VoiceOver listing headings

Absence of CSS also looks alright, all things considered:
Example title followed by an anchor to link to it


I think from an implementation standpoint, you could default to “Link to …”. To be on the safe side, you could apply lang="en" to the text node so it is read out it in English. You could let people pass the translation of their choice through a function receive the heading content, in which case you’ll also drop the lang attribute. A few versions:

// English (default)
assistiveText: (heading) => `Link to “${heading)`,
// German
assistiveText: (heading) => `Link zum Titel „${heading)`,
// French
assistiveText: (heading) => `Lien vers le titre « ${heading) »`,
// Dutch
assistiveText: (heading) => `Link naar “${heading)`

Honestly making the entire headings links is also a fine option in my opinion. Then we don’t have to mess around all this stuff at all. What you see is what you get.

@nhoizey
Copy link
Author

nhoizey commented Mar 1, 2021

Another side-effect, activating the Reader mode on Firefox:

That's unfortunate, but not as much as the other issues… can we say it's a Reader mode issue? 😅

In the future, Text Fragments might save us a couple of headaches :)

I did mention Text Fragment in my article lead, indeed! 👍

But they currently don't work on short texts (lack of "context", I think), so often not on headings… 😭

@ambrwlsn
Copy link

ambrwlsn commented Mar 1, 2021

Hey! Thanks for picking up on this 🙂
The way the anchor links are formatted on my site is the same as in Kitty's comment above.

With no CSS, the anchor links do indeed show up as follows, either on a website or in an RSS reader:

Plain HTML on a website:
Screenshot 2021-03-01 at 21 27 31

Inoreader RSS reader:
Screenshot 2021-03-01 at 21 29 24

NetNewsWire RSS reader:
Screenshot 2021-03-01 at 21 27 49

I am not a huge fan of how this looks, however I am happy with how the anchor links are expressed in a screen reader "headings" and "links" list, as depicted by Kitty above.

I can understand that this may not be desirable for everyone, though.

@tunetheweb
Copy link

tunetheweb commented Mar 1, 2021

I still prefer to make the headers links themselves, rather than a separate link for the following reasons:

  • it's a bigger tap area for mobile
  • it avoids those Google SERP, RSS and Reader Mode issues
  • it means you don't need to worry about the label text
  • it allows you to hide the anchor icon (inserted with a ::before { content: icon }) on mobile or only when focus/hovered or remove it completely (though there are discoverability issues with some of those options).
  • I've only really tested it on Voice Over but it reads it out as "This is a heading. Heading Level 2. Visited. In page link." which seems nicer and less distributive to me than an additional separate link, but also allows screen readers to benefit from this functionality.

MDN uses this approach (without an anchor icon): https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#attributes

And it's what we put in the Web Almanac (but with an anchor icon on desktop on hover/focus, but no anchor on mobile where there's less space): https://almanac.httparchive.org/en/2020/accessibility#ease-of-reading

@tunetheweb
Copy link

tunetheweb commented Mar 1, 2021

Downside with that though is that it’s harder to select the heading text if you want to copy just the heading text but I’m thinking that’s less likely then wanting to copy the link itself from the URL bar after clicking on it (or by right clicking on desktop/hard pushing on mobile).

@ambrwlsn
Copy link

ambrwlsn commented Mar 1, 2021

Yep @bazzadp, I remember now that I didn't want the heading text to be hard to copy. But I can't now think of a good reason for that. I think the way MDN and the Web Almanac go about it is a good one. I also like how the heading is underlined and the icon shows up on hover/focus on Web Almanac.

This is a great opportunity to edit my post once again with a link to and explanation of this discussion.

I love that the discussion popped up. I also wonder if there will be a decision about the most sensible way for markdown-it-anchor to handle this!

@thierryk
Copy link
Contributor

thierryk commented Mar 2, 2021

Downside with that though is that it’s harder to select the heading text if you want to copy just the heading text

I came up with a solution that mimics the "behavior" of the heading/anchor in @ambrwlsn's article but based on MDN's markup: https://codepen.io/thierry/full/qBqYmgw

@tunetheweb
Copy link

I came up with a solution that mimics the "behavior" of the heading/anchor in @ambrwlsn's article but based on MDN's markup: https://codepen.io/thierry/full/qBqYmgw

That's very, very clever!

The reason Voice-Over does not announce the hash, is you only have this on hover, this means the # content doesn't show up on focus, which is what Voice Over uses. Now this may be a good thing in this case as you probably don't want it announced, but also means keyboard only users on desktop don't get this symbol and visual indicator that this is an anchor link. This could be fixed by adding a focus-within selector, but then the # starts to be announced:

h2:hover a::before,
h2:focus-within a::before {
  display: block;
}

The latest version of CSS introduces the ability to set alt text (including blank to remove it from the accessibility tree) by adding a / to content: with the text after this being used as the alt text:

h2 a::before {
  content: "#";
  content: "#" / '';

Looks like this currently also works with text like here, and not just images which is a nice added bonus.

Note you need both lines on this as support is pretty poor unfortunately and browsers (like Safari) that don't recognise this / syntax will ignore the line completely and not even add the #! Adding the first line means those browsers will fallback to display the # but those browsers that does recognise this syntax (Chromium-based browsers for now) override the previous definition with the new one ignore the # for AT. Firefox and Safari still read it however 😞 As you say a background-image may be the best option for now.

The downsides of the solution are that I'm not loving the double tab on mobile (though that could be fixed by removing the hover style completely and just displaying this all the time), and the click area is notable smaller (especially a problem on mobile), which is the same issue it was previously, but is something I think the full header-version like MDN uses solves. I'm really not convinced the ability to select header text only is a real issue and in the rare occasion you want to do this you can work around it by selecting either side of the header.

To be honest, I still prefer the full header links. But do appreciate the cleverness of this solution none-the-less!

@thierryk
Copy link
Contributor

thierryk commented Mar 2, 2021

The reason Voice-Over does not announce the hash, is you only have this on hover,

Duh! Of course! How did I miss that? Thank you!

The downsides of the solution are that I'm not loving the double tab on mobile

I'm now wrapping the rules inside @media (hover: hover){} so that should take care of most issues you raised about mobile: https://codepen.io/thierry/pen/qBqYmgw

select header text only is a real issue and in the rare occasion you want to do this you can work around it by selecting either side of the header.

As far as I know this is not an issue on mobile, so there is even more reason to use @media (hover: hover){}

Note that it is possible to select text inside links via option + click/select (on Mac). alt key on Windows?

I'll try using a background image instead of a character and see the advantages of it.

Thanks a lot for the feedback!

@valeriangalliat
Copy link
Owner

So happy to see all that activity and discussions on this issue! Thanks everybody for contributing. 😻

@nhoizey

Regarding anchor link text, you can't use just "Link to this section" because screen reader users often ask for the list of links in the page (they also ask for the list of headings), and they would get multiple times the same "Link to this section".

Thanks for the explanation, that makes a lot of sense now!

For the CSS use case, on my site I removed the anchor link from the feed, users are much less likely to share a link to a feed item anyway.

That's fair, I didn't think about that. That makes the RSS thing a non-issue at that point, and it seems like a good practice to omit the header anchors from RSS feeds anyways.

Thanks a lot for asking for external feedback on Twitter, that brought a lot of value to this issue. 🙏

@KittyGiraudel

At the end of the day I'm leaning towards the permalink element being described by an aria-label attribute.

This might be unwise. aria-label is notoriously bad with content translating services such as Google Translate, which is something that a lot of people use on content-heavy pages such as articles, precisely where one might want/find heading anchors.

Wow I didn't realize the extent of the issue with aria-label ("aria-label is ignored by Google's translator, MS Translate has the same issue"), that article you linked was very insightful. aria-label has been around so long now that I assumed the support would be close to that of the alt attribute, I'm sad to see it's far from being the case.

@ambrwlsn

(screenshots)

I am not a huge fan of how this looks, however I am happy with how the anchor links are expressed in a screen reader "headings" and "links" list, as depicted by Kitty above.

I can understand that this may not be desirable for everyone, though.

Thanks for testing in different contexts! I agree that the rendering without CSS or in RSS feeds may not be ideal for everyone, but the screen reader experience is definitely great in your example.

At the end of the day, the alt text "leaking" in places where we usually wouldn't expect it to be is the "cost" for having a proper translation experience on a screen reader, and it's only a small visual inconvenience (as @KittyGiraudel pointed out, it's not a SEO issue as the content itself is legitimate, it's just a visual redundancy when displayed in SERPs and RSS feeds).

@thierryk

I came up with a solution that mimics the "behavior" of the heading/anchor in @ambrwlsn's article but based on MDN's markup: https://codepen.io/thierry/full/qBqYmgw

That's a solid solution. As @bazzadp pointed out we can use h2:focus-within a::before (or h2 a:focus::before) to enable keyboard navigation, and I would add that we can set h2 { cursor: text } to have the usual text cursor on the headers instead of a pointer.

The ::before hash being announced on VoiceOver could be mitigated by using a aria-hidden span inside the link for the permalink symbol. At that point the markup would be very close to what we already have, and with some CSS, could be made to look and feel like the current default behavior as well, while offering the MDN header anchors experience on screen readers, and wherever else the raw markup would be embeded.

I'm gonna give this idea a bit more thinking but I like where this is going.

@thierryk

Note that it is possible to select text inside links via option + click/select (on Mac). alt key on Windows?

Wow, I didn't know that, just confirmed this works on my Linux setup as well!

Tradeoffs

Pros Cons
@ambrwlsn / @KittyGiraudel solution (anchor following header with visually-hidden alt text) Accessible experience Small visual inconveniences in SERPs, RSS feeds and reader mode, requires configuration of alt text prefix
Above solution but with aria-label instead Less visual inconveniences Header anchor alt text doesn't translate in most cases, requires configuration of alt text prefix
Above solution but with aria-describedby Less visual inconveniences Doesn't read "Link to" before the header name in the permalink
Whole header is the link (MDN / Web Almanac) Accessible experience Partially selecting the header is harder and we can't include links inside the header
@thierryk's version of the MDN markup Accessible experience Can't include links inside the header
Current experience None Garbage is read as part of headers
aria-hidden anchor links Doesn't pollute headers in screen readers unlike current experience Header anchor feature is omitted from screen readers

How markdown-it-anchor should handle this

@ambrwlsn

I love that the discussion popped up. I also wonder if there will be a decision about the most sensible way for markdown-it-anchor to handle this!

I kept reasoning about this issue with the objective to find a default renderPermalink function for some reason, but in hindsight, the absolute default of markdown-it-anchor is to just add the id attribute on headings; there is no need for a renderPermalink default.

At that point, since it appears that no one solution can satisfy every user of markdown-it-anchor, I believe that markdown-it-anchor shouldn't provide a default as far as how permalinks are rendered, which allows me to keep this library unopinionated.

By having no default (instead of changing the markup of the default option), I also make sure that if users upgrade the library to the next major version without changing their code, it will not silently change the way their site renders, it will just fail to compile, which I think can save some trouble.

My suggestion after this discussion is to remove the permalink boolean option. In order to render a permalink, a renderPermalink function would be required, and markdown-it-anchor would provide a number of built-in renderPermalink functions (the ones discussed in this issue to start with). It would then be up to the markdown-it-anchor users to chose which renderPermalink function suits them best.

That being said, with backwards compatibility in mind, I might deprecate the current default instead. This would ensure that new users are provided with the accessible choices in the documentation, but existing users who would upgrade to a new version wouldn't have to change their code right away, they would instead see a deprecation warning if they use the legacy default renderer.

In code, this would look like:

const md = require('markdown-it')()
const anchor = require('markdown-it-anchor')

 md.use(anchor, {
  renderPermalink: anchor.permalink.ohMyGoshWeNeedToNameAllOfThoseNow(permalinkOptions)
})

And for the list of built-in renderers (naming suggestions more than welcome):

// The one originally discussed
anchor.permalink.linkAfterHeader({
  assistiveText: title => `Permalink to ${title}`, // Required except if `style` is `aria-describedby`
  style: 'visually-hidden', // Could also be `aria-label` or `aria-describedby`
  visuallyHiddenClass: 'sr-only' // Required if `style` is `visually-hidden`,
  symbol: '¶' // Currently `permalinkSymbol`
})

// MDN / Web Almanac style
anchor.permalink.headerLink({
  symbol: null, // If set, include a `aria-hidden` `span` inside the link with this symbol for styling purpose
  symbolClass: 'permalink-symbol', // Allows to give a class to the `span`
  space: true, // Currently `permalinkSpace`
  placement: 'after' // Currently `permalingBefore`
})

// Aliases
anchor.permalink.mdn
anchor.permalink.httpArchive
anchor.permalink.webAlmanac

// GitHub style
anchor.permalink.ariaHidden({
  symbol: '¶', // Currently `permalinkSymbol`
  space: true, // Currently `permalinkSpace`
  placement: 'after' // Currently `permalingBefore`
})

// Aliased as
anchor.permalink.github

I'll probably rename permalinkClass, permalinkHref and permalinkAttrs to class, renderHref and renderAttrs and include them in the permalinkOptions to all the above functions, instead of being top-level markdown-it-anchor options to keep everything contained.

If this sounds like a good solution to everybody, I'll go on with implementing and documenting those changes (I might tweak a bit the API as I give this more thinking but it should be close to what I proposed above).

As for the linkAfterHeader renderer, I'll base it on your code @nhoizey, or if you prefer, I can leave this one aside to let you make a separate PR for it once I have the base refactoring/structure down, whatever you're most comfortable with!

Other details

Even if I want to keep this mostly backwards compatible, there's one thing I want to break, which is not requiring permalink: true to be set if we're using a custom renderer. I don't expect many users to set a custom renderer whilst not passing the permalink: true option, or explicitly setting it to false, but this is technically breaking so I won't take the risk and I'll bump a major regardless.

This will slightly improve the new experience as we'll be able to do md.use(anchor, { renderPermalink: anchor.permalink.whatever() }) without having to systematically include the legacy permalink: true option.

And at that point I can also check whether the permalink option is a boolean or a function to allow doing md.use(anchor, { permalink: anchor.permalink.whatever() }).


I'd love to hear your feedback below, and based on it, I can free up some time in the next couple of weeks to make this change happen. ❤️

@thierryk
Copy link
Contributor

thierryk commented Mar 6, 2021

I would add that we can set h2 { cursor: text }

Yep! That's exactly what I have on the new version I am working on. The original Pen was just a "proof of concept" to see if pointer-events:none could help with text selection.

Now I have something that works the way I want based on the following markdown:

## [Introduction](#introduction "Anchor Link")

As far as I know this should be fine regarding localization (title doesn't have the same issue as aria-label).

I think that markup (associated with CSS) addresses many of the issues listed in this thread apart of course the fact that it prevents including links inside the heading. :-(
I also need to check if title and ::after could be both spoken by SR at once (I think it depends mostly on user's setting but I could be wrong).

I'm writing an article about this, that's why I'm not sharing my new Pen here yet.

@nhoizey
Copy link
Author

nhoizey commented Mar 7, 2021

@valeriangalliat that's a great overview of all pros and cons! 👍

Your suggestion to provide multiple functions to chose from is interesting, even if I would love everyone to agree on one single solution. I'm not sure it's useful to have aliases with mdn, github, etc., as long as the docs cite them as examples. Feel free to use my code directly!

And I think we can wait for @thierryk's next iteration, even if I do have some (rare) headings containing links in my contents: https://nicolas-hoizey.com/articles/2004/08/04/les-transferts-de-gros-fichiers-simplifies/#dropload

@thierryk
Copy link
Contributor

thierryk commented Mar 9, 2021

I was inspired by the Demo as JSFiddle and did this online "tool" to help get the proper slug for an easy copy/paste (better experience using keyboard only).

@nhoizey
Copy link
Author

nhoizey commented Mar 10, 2021

@thierryk for safe slugs, I always use https://github.com/sindresorhus/slugify

(But that's not the topic of this issue… 😅)

@leereamsnyder
Copy link

leereamsnyder commented May 10, 2021

Little late here, but when I was implementing the Web Almanac/MDN pattern (whole header is the link) on my own site, I noticed that the headers didn't appear at all in Safari’s Reader View. 😢

I fussed around and figured out that some additional markup around the text content will make them show up again, but it feels sorta gross:

<h2 id="introduction">
  <a href="#introduction">
    <span>
      Introduction
    </span>
  </a>
</h2>

I wrote up my findings here: https://www.leereamsnyder.com/blog/making-headings-with-links-show-up-in-safari-reader

@tunetheweb
Copy link

Thanks for raising that @leereamsnyder . That definitely didn't use to happen when I first implemented that for the Web Almanac (I have screenshots to prove it! 😀). But yeah confirm it does happen now which sucks 😞

BTW one less icky way of getting round it is swapping round the headers and links:

  <a href="#introduction">
    <h2 id="introduction">
      Introduction
    </h2>
  </a>

More test cases here: https://www.tunetheweb.com/experiments/heading-links/

And raised this as a bug with WebKit team: https://bugs.webkit.org/show_bug.cgi?id=225609

@thierryk
Copy link
Contributor

BTW one less icky way of getting round it is swapping round the headers and links

FWIW, I found an issue with that construct: CodePen Home
block-level elements inside anchors and :focus

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

No branches or pull requests

8 participants