Refactor for accessible permalinks (fixes #82) #89
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
valeriangalliat
force-pushed
the
accessibility
branch
from
7a89418
to
1b87b6e
Compare
I thought an `<a>` element always needed to be inside a `<p>` to be valid HTML, but this seems not true anymore.
|
@valeriangalliat this looks like a really nice improvement, with multiple options and great documentation to help decide which one to use! 👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: truewas inaccessible, and no single solution could satisfy all use cases, I chose to remove the default behaviour and require explicit configuration of arenderPermalinkfunction (which I renamed to justpermalinkas 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 defaultanchor.permalink.ariaHidden.Backwards compatibility
While this will be a major version, I still maintained complete backwards compatibility with the old API. Using the boolean
permalinkoption will still behave like it does today, and if the legacy defaultrenderPermalinkfunction is used (by not explicitly configuring arenderPermalinkfunction), it will useprocess.emitWarningwhen 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. :)