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

Expose custom options regardless of permalink styles #101

Closed
kleinfreund opened this issue Jul 10, 2021 · 4 comments
Closed

Expose custom options regardless of permalink styles #101

kleinfreund opened this issue Jul 10, 2021 · 4 comments

Comments

@kleinfreund
Copy link

kleinfreund commented Jul 10, 2021

With the changes introduced in v8.0.0, a couple of preset permalink styles are available (see https://github.com/valeriangalliat/markdown-it-anchor#permalinks). I tried migrating my use of markdown-it-anchor to these styles to remove the deprecation warning from my build output.

Unfortunately, it doesn’t seem possible to reproduce my header anchor markup with any of the options:

<h2 id="honorable-mentions" tabindex="-1">
  Honorable mentions

  <a class="header-anchor" href="#honorable-mentions">
    <span class="header-anchor__icon">
      <span aria-label="Link Symbol" role="img">🔗</span>
      <span class="visually-hidden">Jump to heading</span>
    </span>
  </a>
</h2>

The options on the provided presets only partially fit my needs:

  • Requirement 1: Anchor is within the heading element.

    I cannot use linkAfterHeader because it renders the anchor as a sibling rather than a child elment. That’s fine in principle. After all, that’s what this style is meant to do.

  • Requirement 2: Customize anchor markup or at least set the class name and visually-hidden class name, perhaps customize the anchor text based on the heading text.

    I cannot use headerLink because these customizations aren’t possible. According to the documentation, it accepts a few default options, but I believe that rather means it has asome default options because the type definitions don’t list any parameters.

  • Requirement 3: Anchor is exposed to screen readers.

    I cannot use ariaHidden because it applies aria-hidden="true"

To summarize, the configurability of linkAfterHeader works well for me were it not for the markup ending up as a sibling element of the heading rather than a child.

I wonder why the flexible configurability is not available on all presets. More specifically, I don’t think it’s ultimately very sensible to tie the following two aspects strictly together based on which preset one chooses:

  • the position of the markup (inside or outside the heading)
  • the available options (setting the anchor text based on the heading text, setting a class name, etc.

In my case, the appropriate solution is probably to use a custom render function. That’s probably not a big deal in practice, but I don’t know how to go about this because that feature is not documented as far as I can tell.

I would prefer that the various permalink styles all support the same degree of customization. Especially the position of the anchor is something I’d like to generally be able to customize regardless of style preset. A good analogy to what I thought I’d be able to do is the Element.insertAdjacentHTML API, in particular its position parameter:

position

A DOMString representing the position relative to the element; must be one of the following strings:

  • 'beforebegin': Before the element itself.
  • 'afterbegin': Just inside the element, before its first child.
  • 'beforeend': Just inside the element, after its last child.
  • 'afterend': After the element itself.

There’s a good chance I’m missing a key aspect of the picture here, but right now, I’m just a bit lost as to what option’s best suited for me going forward. For now, I will keep using the following set of options and hope they don’t actually get removed:

const markdownItAnchorOptions = {
  permalink: true,
  permalinkSymbol: `
    <span class="header-anchor__icon">
      <span aria-label="Link Symbol" role="img">🔗</span>
      <span class="visually-hidden">Jump to heading</span>
    </span>
  `.trim(),
  permalinkBefore: false,
};

On a slightly related note, I would appreciate if deprecations are explicitly marked in the changelog. The changelog entry for version v8.0.0 looks very harmless and appears as a mere feature addition and while my current configuration still works, should it get removed (which I assume is the case since it warns me in the log output that the option is deprecated), I would need to migrate to a set of options that I currently don’t understand well (because implementing a custom permalink renderer is undocumented).

@kleinfreund
Copy link
Author

I started to play around with the project a bit to figure out if I’m able to contribute a change that would allow me to cover my use case a bit more gracefully (of course at this point, I don’t even know if what I’ve assembled in words above is even sensible, so this is very much just on my own initiative).

While doing that, I had a look at the tests to familiarize myself with some usage scenarios and I wondered if you would be open to accepting a pull request for migrating the existing test suite to Jest so that they can be structured a bit more neatly using describe and test blocks. I’m happy to submit a PR for that.

@valeriangalliat
Copy link
Owner

valeriangalliat commented Aug 26, 2021

Hey! That's a cool way of rendering permalinks, I didn't consider that in the previous version and it definitely should be possible without a custom renderer.

It's also true that the readme is lacking about how to make custom renderers, I'll update it to cover that.

I'm thinking of allowing the ariaHidden permalink (which is essentially the legacy one with a different way to pass options and the addition of aria-hidden="true") to not add ariaHidden (so I need a different name for that, maybe linkInsideHeader with an ariaHidden option).

That way you could do:

const markdownItAnchorOptions = {
  permalink: anchor.permalink.linkInsideHeader({
    symbol:  `
      <span class="header-anchor__icon">
        <span aria-label="Link Symbol" role="img">🔗</span>
        <span class="visually-hidden">Jump to heading</span>
      </span>
    `.trim(),
    placement: 'before'
  }
};

As for the tests, while bare assert was fine in the early stages of this project, it's true it grew quite a bit and could use describe and test blocks. I think I would keep assert as the assertion library and just use Mocha to provide describe/test, so the task would consist in organizing the existing tests in functions and giving them descriptions. Let me know if you still want to contribute that. :) I took this as an opportunity to try AVA which test.js now uses!

@valeriangalliat
Copy link
Owner

valeriangalliat commented Aug 26, 2021

Okay linkInsideHeader is available with version 8.2.0 that I just released, that should be enough to make your use case work with the markup above!

I think it's worth mentioning that one of the reasons the original permalink was deprecated is because it was challenging to make it accessible. Even with the aria-label/visually-hidden markup, you can notice that by listing the page headings and links in VoiceOver (Caps Lock + U). For headers it reads:

  • Link Symbol Jump to heading Title 1
  • Link Symbol Jump to heading Title 2
  • Link Symbol Jump to heading Title 3

And when requesting the links in the page:

  • Link Symbol Jump to heading
  • Link Symbol Jump to heading
  • Link Symbol Jump to heading

On top of that aria-label might not be reliably picked up by translation services so it's often recommended to avoid it for this reason. The other permalink option were designed to integrate even better with screen readers, and I think they're worth taking a look at.

For example linkAfterHeader will read "Title 1, Title 2, Title 3" for headings and "Permalink to “Title 1”, Permalink to “Title 2”, Permalink to “Title 3”" for links, which is arguably more useful, but that markup do require styling updates so it's not an instant fix.

I hope this help, cheers!

@kleinfreund
Copy link
Author

kleinfreund commented Oct 3, 2021

Hey there,

thanks for the thorough and helpful response. I’ve just updated the site where I’m using the aforementioned style using the follow configuration (i.e. exactly what you suggested):

const markdownItAnchorOptions = {
  permalink: markdownItAnchor.default.permalink.linkInsideHeader({
    symbol: `
      <span class="header-anchor__icon">
        <span aria-label="Link Symbol" role="img">🔗</span>
        <span class="visually-hidden">Jump to heading</span>
      </span>
    `.trim(),
    placement: 'before',
  }),
};

I have read the conversations feeding into the decisions made regarding the deprecation of permalinks and understand the motivation and results, too. I’ll look into improving the markup on my site separately (i.e. separately from the dependency update). I hope translation services like that built into Chrome eventually pick up on the likes of aria-label to make developer choices more flexible.


I’ll close this issue for now because I for one am satisfied with the outcome.

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

2 participants