-
-
Notifications
You must be signed in to change notification settings - Fork 9
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
be able to mimic the new heading structure used by github #20
Comments
I did a small PR #21 of what I'm currently using for my project, please let me know what you think of it and if you would be interested of merging it, I would also be willing to make modifications if needed |
As an alternative I did this: behavior: 'wrap',
properties: {
className: ['section_heading'],
}, Then added the icon with CSS: .section_heading::after {
content: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 16 16' width='18' aria-hidden='true'%3E%3Cpath fill='%23000' d='m7.775 3.275 1.25-1.25a3.5 3.5 0 1 1 4.95 4.95l-2.5 2.5a3.5 3.5 0 0 1-4.95 0 .751.751 0 0 1 .018-1.042.751.751 0 0 1 1.042-.018 1.998 1.998 0 0 0 2.83 0l2.5-2.5a2.002 2.002 0 0 0-2.83-2.83l-1.25 1.25a.751.751 0 0 1-1.042-.018.751.751 0 0 1-.018-1.042Zm-4.69 9.64a1.998 1.998 0 0 0 2.83 0l1.25-1.25a.751.751 0 0 1 1.042.018.751.751 0 0 1 .018 1.042l-1.25 1.25a3.5 3.5 0 1 1-4.95-4.95l2.5-2.5a3.5 3.5 0 0 1 4.95 0 .751.751 0 0 1-.018 1.042.751.751 0 0 1-1.042.018 1.998 1.998 0 0 0-2.83 0l-2.5 2.5a1.998 1.998 0 0 0 0 2.83Z'%3E%3C/path%3E%3C/svg%3E");
width: 18px;
display: inline-block;
margin-left: 8px;
} |
Yes this works great 👍 , I mean if you are ok with having the svg in your css than this solution is valid too, it is just that if it is possible to extend the wrap functionality or add a new "behavior" type, then your solution will still work but you will also be able to do it without css, which would be great to have that choice if you look at the comments in the PR, you will find a suggestion by wooorm which works really well, it does not use a new behavior but instead add the possibility to use content with the "wrap" behavior, which solves this case and even might solve other uses cases someone else might come up with |
Related-to: GH-20. Related-to: GH-21. Co-authored-by: Chris Weber <[email protected]>
This comment has been minimized.
This comment has been minimized.
This should now do the trick: import {s} from 'hastscript'
import {rehype} from 'rehype'
import rehypeAutolinkHeadings from 'rehype-autolink-headings'
import rehypeSlug from 'rehype-slug'
import {read} from 'to-vfile'
const file = await rehype()
.data('settings', {fragment: true})
.use(rehypeSlug)
.use(rehypeAutolinkHeadings, {
behavior: 'wrap',
content: s(
'svg.octicon.octicon-link',
{
viewBox: '0 0 16 16',
version: '1.1',
width: 16,
height: 16,
ariaHidden: 'true'
},
s('path', {
d: 'm7.775 3.275 1.25-1.25a3.5 3.5 0 1 1 4.95 4.95l-2.5 2.5a3.5 3.5 0 0 1-4.95 0 .751.751 0 0 1 .018-1.042.751.751 0 0 1 1.042-.018 1.998 1.998 0 0 0 2.83 0l2.5-2.5a2.002 2.002 0 0 0-2.83-2.83l-1.25 1.25a.751.751 0 0 1-1.042-.018.751.751 0 0 1-.018-1.042Zm-4.69 9.64a1.998 1.998 0 0 0 2.83 0l1.25-1.25a.751.751 0 0 1 1.042.018.751.751 0 0 1 .018 1.042l-1.25 1.25a3.5 3.5 0 1 1-4.95-4.95l2.5-2.5a3.5 3.5 0 0 1 4.95 0 .751.751 0 0 1-.018 1.042.751.751 0 0 1-1.042.018 1.998 1.998 0 0 0-2.83 0l-2.5 2.5a1.998 1.998 0 0 0 0 2.83Z'
})
),
headingProperties: {tabIndex: '-1', dir: 'auto'},
properties: {className: ['heading-link']}
})
.process(await read('example.html'))
console.log(String(file)) |
Initial checklist
Problem
I tried to reproduce the new headings that github is now using, with an anchor inside of the heading element and inside of the anchor the heading text and an icon:
However if I set the behavior to "wrap" it creates a heading with an anchor element inside which solves the first part of the problem but the second part of the problem can't be solved as I can not use content to create the icon when the behavior is "wrap"
So as of today it seems like there is no way of recreating what github does using rehype-autolink-headings, or did I miss something?
Solution
From the 4 alternatives, I like the last suggestion best as it is not a breaking change and it gives the devs full control about what ever they want to put into the heading element and it only introduces one new behavior value
Oh actually there is one more thing I didn't mention yet, rehype-autolink-headings has a "properties" option that allows to change the attributes of the anchor element, when the content was an icon it made sense to have for example a tabIndex set to "-1", but I was wondering if it was still making sense if the text of the heading is inside the anchor too and not just the icon?
This is why a had another look at the github markup I posted above, and that's when I noticed that they added the tabIndex attribute to the heading element and not the anchor element (meaning the actual anchor in the heading is gets focused when pressing the tab key, as it has no tabIndex), I assume that all the changes github did, are to make headings more accessible, but the tabIndex on the heading seems like it is a bug, I mean heading elements have no tabIndex so why would you want to set it to "-1" to disable it? I checked out the MDN tabIndex page and it confirmed my assumption that browsers don't add a tanIndex to headings. The tabIndex would only make sense it the heading element itself got converted into an anchor element that uses the aria heading role to mimic the behavior of a heading element, so I tried out the new github accessibility feature that they call Underline Links in Text Blocks Public Beta to see if it has an impact on headings, if I turn the new feature on, headings are not underlined at all, however if I turn it off (old behavior) headings are underlined on hover, but at no point the heading element is replaced by an anchor element, so still no clue why there is a tabIndex attribute on the heading element.
Putting aside githubs behavior for now, I was wondering if the properties option should get used by the new behavior and be applied to the anchor? The current default value for the properties options is {ariaHidden: 'true', tabIndex: -1}. But if the behavior is set to the new "substitute", then applying those two values to the link would not be wrong. I think the aria-hidden should be an attribute of the icon and not the anchor (as only the icon is decorative, but not the text), the tabIndex="-1" should still get applied to the anchor, because the anchor in the heading should not be focusable (as opposed to what github currently does). My logic is that an anchor has a tabIndex by default because it brings the user to a new part of a page or a completely new page, however the anchor in a heading does not have that purpose (it just changes the URL in the address bar so that it can be shared), from an accessibility point of view the anchors in the toc (table of contents) should be the ones that get used to move to parts of the page. This is why I think that it makes sense to change the default value of the properties to {tabIndex="-1"} when the behavior is set to substitute.
Alternatives
Here is a list of solutions I could think of, maybe there is one you like best:
The text was updated successfully, but these errors were encountered: