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

feat(markdown): update default markdown-it-anchor permalink function (close #1363) #1452

Merged
merged 5 commits into from
Dec 12, 2023

Conversation

nruffing
Copy link
Contributor

@nruffing nruffing commented Dec 9, 2023

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Provide a description in this PR that addresses what the PR is solving. If this PR is going to solve an existing issue, please reference the issue (e.g. close #123).

What is the purpose of this pull request?

  • Bug fix
  • New feature
  • Documentation update
  • Other

Description

The heading anchors have aria-hidden attributes but are still focusable via keyboard navigation. There was a bit of discussion around this already in the markdown-it-anchor repo and you can address this by just using the headerLink render function instead of the ariaHidden render function since it wraps the entire header in an anchor.

This fixes #1363

Screenshots

We will also need to make some changes to theme-default's base styles to accompany these changes. I will put that PR up shortly and include screenshots there.

Before

After

@meteorlxy
Copy link
Member

cc @Mister-Hope any thoughts about this? It should be kind of breaking change for community themes.

@Mister-Hope
Copy link
Member

Mister-Hope commented Dec 12, 2023

I actually do not care about these kind of changes, my theme is designed to be aligning an exact version of VuePress so it won't bother me.

This fix the a11y issue, so I am positive towards it.

@meteorlxy meteorlxy changed the title fix: use headerLink from markdown-it-anchor for better accessibility feat(markdown): update default markdown-it-anchor permalink function Dec 12, 2023
@meteorlxy meteorlxy changed the title feat(markdown): update default markdown-it-anchor permalink function feat(markdown): update default markdown-it-anchor permalink function (close #1363) Dec 12, 2023
@meteorlxy meteorlxy merged commit f7d6dde into vuepress:main Dec 12, 2023
22 checks passed
@nruffing nruffing deleted the 1363-accessible-anchors branch December 26, 2023 13:22
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.

[Bug report] The header anchor should stay outside of the heading tag
3 participants