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

Refactor for accessible permalinks (fixes #82) #89

Merged
merged 6 commits into from
Jun 14, 2021

Conversation

valeriangalliat
Copy link
Owner

While this PR is backwards compatible, I deem the changes significant enough to be published as a major version.

This PR is the result of the discussion in Suggestion: adding an option to render a more accessible anchor link (#82).

Browse the readme from this PR.

Overview

Because the default behaviour of permalink: true was inaccessible, and no single solution could satisfy all use cases, I chose to remove the default behaviour and require explicit configuration of a renderPermalink function (which I renamed to just permalink as the boolean option is now obsolete).

For convenience I included a number of different renderers that can be used out of the box, e.g. permalink: anchor.permalink.headerLink() including one with the same markup as the current default anchor.permalink.ariaHidden.

Backwards compatibility

While this will be a major version, I still maintained complete backwards compatibility with the old API. Using the boolean permalink option will still behave like it does today, and if the legacy default renderPermalink function is used (by not explicitly configuring a renderPermalink function), it will use process.emitWarning when first called to inform the user of the new way of handling permalinks.

Ping!

@nhoizey, I'd love to have your input on that implementation, it's very close to what I proposed a couple of months ago in the issue. I know this doesn't bring a consensus on a single implementation like you would have ideally wanted, but I hope you still find this to be a reasonable improvement over what we had before, at least in the way it raises awareness and doesn't encourage an inaccessible markup by default anymore.

@nagaozen, you're a key contributor of markdown-it-anchor, so I'd like to make you aware of those changes before I integrate them. If you have the time to take a quick look to this PR, let me know if you have any concerns or suggestions. :)

I thought an `<a>` element always needed to be inside a `<p>` to be
valid HTML, but this seems not true anymore.
@nhoizey
Copy link

nhoizey commented Jun 9, 2021

@valeriangalliat this looks like a really nice improvement, with multiple options and great documentation to help decide which one to use! 👍

@valeriangalliat valeriangalliat merged commit 70548dd into master Jun 14, 2021
@valeriangalliat valeriangalliat deleted the accessibility branch June 14, 2021 14:53
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.

2 participants