8000 fix(a11y): Fix ARIA attributes and switch from `div` to `nav` for the explorer by MathieuDR · Pull Request #1972 · jackyzha0/quartz · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix(a11y): Fix ARIA attributes and switch from div to nav for the explorer #1972

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: v4
Choose a base branch
from
Open

fix(a11y): Fix ARIA attributes and switch from div to nav for the explorer #1972

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9691ef3
fix a11y for explorer
MathieuDR May 10, 2025
f23ee32
🎨 use project formatter
MathieuDR May 10, 2025
c0f80b6
♿️ TOC & overflow list accessibility
MathieuDR May 10, 2025
ac1c0a2
🎨 formatting and fixing error
MathieuDR May 10, 2025
File filter

Filter by extension

8000
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions quartz/components/Explorer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export default ((userOpts?: Partial<Options>) => {

const Explorer: QuartzComponent = ({ cfg, displayClass }: QuartzComponentProps) => {
return (
<div
<nav
class={classNames(displayClass, "explorer")}
data-behavior={opts.folderClickBehavior}
data-collapsed={opts.folderDefaultState}
Expand All @@ -78,6 +78,7 @@ export default ((userOpts?: Partial<Options>) => {
class="explorer-toggle mobile-explorer hide-until-loaded"
data-mobile={true}
aria-controls="explorer-content"
aria-label="Explorer"
>
<svg
xmlns="http://www.w3.org/2000/svg"
Expand Down Expand Up @@ -116,7 +117,7 @@ export default ((userOpts?: Partial<Options>) => {
<polyline points="6 9 12 15 18 9"></polyline>
</svg>
</button>
<div class="explorer-content" aria-expanded={false}>
<div id="explorer-content">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A while back all components were changed to use classes over id's to allow non-atomic usage.

Copy link
Author
@MathieuDR MathieuDR May 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you mean, but the issue with this is that aria-attributes actually requires id's and don't work with classes.
One way to deal with this would be to generate an id for these components, just like the Overflow list did.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except they don't. They require a unique identifier field per tag. This does not have to be the id field.

Copy link
Author
@MathieuDR MathieuDR May 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do think that aria-controls requires the id tag of the element. Here are the specifications

aria-controls specification
Id reference list value type

This element in particular requires it because the button has aria-controls="explorer-content".

HTML element concept, where if you scroll down you see what the unique identifier is according to the dom.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, but that still doesn't address the issue I raised.

Your proposed changes remove the classes and add id's. Removing the classes will break component functionality.

In addition, id's must be unique for aria to function properly, per your own sources. Currently, components are non-atomic. If you want to add id's for the purpose of accessibility, please ensure uniqueness in case of multiple components of the same type. (Ideally consistent as well, e.g. no random generated appended text.)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, Like I said I could generate them if you want to support multiple of the same component.
It would also require the inline script to change a bit so it targets the correct one, but I think we can manage that.

<OverflowList class="explorer-ul" />
</div>
<template id="template-file">
Expand Down Expand Up @@ -152,7 +153,7 @@ export default ((userOpts?: Partial<Options>) => {
</div>
</li>
</template>
</div>
</nav>
)
}

Expand Down
13 changes: 5 additions & 8 deletions quartz/components/OverflowList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
import { JSX } from "preact"

const OverflowList = ({
children,
...props
}: JSX.HTMLAttributes<HTMLUListElement> & { id: string }) => {
const OverflowList = ({ children, ...props }: JSX.HTMLAttributes<HTMLUListElement>) => {
return (
<ul {...props} class={[props.class, "overflow"].filter(Boolean).join(" ")} id={props.id}>
<ul {...props} class={[props.class, "overflow"].filter(Boolean).join(" ")}>
{children}
<li class="overflow-end" />
</ul>
Expand All @@ -14,11 +11,11 @@ const OverflowList = ({

let numExplorers = 0
export default () => {
const id = `list-${numExplorers++}`
const dataId = `list-${numExplorers++}`

return {
OverflowList: (props: JSX.HTMLAttributes<HTMLUListElement>) => (
<OverflowList {...props} id={id} />
<OverflowList {...props} data-list-id={dataId} />
),
overflowListAfterDOMLoaded: `
document.addEventListener("nav", (e) => {
Expand All @@ -34,7 +31,7 @@ document.addEventListener("nav", (e) => {
}
})

const ul = document.getElementById("${id}")
const ul = document.querySelector("ul[data-list-id='${dataId}']")
if (!ul) return

const end = ul.querySelector(".overflow-end")
Expand Down
5 changes: 3 additions & 2 deletions quartz/components/TableOfContents.tsx
View file
Open in desktop
< 6D40 /tr>
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,11 @@ export default ((opts?: Partial<Options>) => {
<button
type="button"
class={fileData.collapseToc ? "collapsed toc-header" : "toc-header"}
aria-labelledby="toc-heading"
aria-controls="toc-content"
aria-expanded={!fileData.collapseToc}
>
<h3>{i18n(cfg.locale).components.tableOfContents.title}</h3>
<h3 id="toc-heading">{i18n(cfg.locale).components.tableOfContents.title}</h3>
<svg
xmlns="http://www.w3.org/2000/svg"
width="24"
Expand All @@ -53,7 +54,7 @@ export default ((opts?: Partial<Options>) => {
<polyline points="6 9 12 15 18 9"></polyline>
</svg>
</button>
<OverflowList class={fileData.collapseToc ? "collapsed toc-content" : "toc-content"}>
<OverflowList id="toc-content" class={fileData.collapseToc ? "collapsed" : ""}>
{fileData.toc.map((tocEntry) => (
<li key={tocEntry.slug} class={`depth-${tocEntry.depth}`}>
<a href={`#${tocEntry.slug}`} data-for={tocEntry.slug}>
Expand Down
2 changes: 1 addition & 1 deletion quartz/components/scripts/explorer.inline.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ function createFolderNode(
}

async function setupExplorer(currentSlug: FullSlug) {
const allExplorers = document.querySelectorAll("div.explorer") as NodeListOf<HTMLElement>
const allExplorers = document.querySelectorAll("nav.explorer") as NodeListOf<HTMLElement>

for (const explorer of allExplorers) {
const dataFns = JSON.parse(explorer.dataset.dataFns || "{}")
Expand Down
2 changes: 1 addition & 1 deletion quartz/components/scripts/toc.inline.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ function toggleToc(this: HTMLElement) {
function setupToc() {
for (const toc of document.getElementsByClassName("toc")) {
const button = toc.querySelector(".toc-header")
const content = toc.querySelector(".toc-content")
const content = toc.querySelector("#toc-content")
if (!button || !content) return
button.addEventListener("click", toggleToc)
window.addCleanup(() => button.removeEventListener("click", toggleToc))
Expand Down
10 changes: 5 additions & 5 deletions quartz/components/styles/explorer.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
margin: 0;
}

.hide-until-loaded ~ .explorer-content {
.hide-until-loaded ~ #explorer-content {
display: none;
}
}
Expand Down Expand Up @@ -108,7 +108,7 @@ button.desktop-explorer {
}
}

.explorer-content {
#explorer-content {
list-style: none;
overflow: hidden;
overflow-y: auto;
Expand Down Expand Up @@ -215,7 +215,7 @@ li:has(> .folder-outer:not(.open)) > .folder-container > svg {
&.collapsed {
flex: 0 0 34px;

& > .explorer-content {
& > #explorer-content {
transform: translateX(-100vw);
visibility: hidden;
}
Expand All @@ -224,13 +224,13 @@ li:has(> .folder-outer:not(.open)) > .folder-container > svg {
&:not(.collapsed) {
flex: 0 0 34px;

& > .explorer-content {
& > #explorer-content {
transform: translateX(0);
visibility: visible;
}
}

.explorer-content {
#explorer-content {
box-sizing: border-box;
z-index: 100;
position: absolute;
Expand Down
2 changes: 1 addition & 1 deletion quartz/components/styles/toc.scss
View file
Open in desktop
4C7F
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ button.toc-header {
}
}

ul.toc-content.overflow {
ul#toc-content.overflow {
list-style: none;
position: relative;
margin: 0.5rem 0;
Expand Down
0