Skip to content
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/ten-needles-sell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@astrojs/starlight': minor
---

Improves the accessibility of asides and tabs by removing some unnecessary HTML landmarks.
10 changes: 2 additions & 8 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,9 @@ jobs:
- name: Type check packages
run: pnpm typecheck

pa11y:
a11y:
name: Check for accessibility issues
runs-on: ubuntu-22.04
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v4
Expand All @@ -107,12 +107,6 @@ jobs:
- name: Install Dependencies
run: pnpm i

- name: Build docs site
working-directory: ./docs
run: pnpm build
env:
NO_GRADIENTS: true

- name: Run accessibility audit
working-directory: ./docs
run: pnpm t
Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ To add a language, you will need its BCP-47 tag and a label. See [“Adding a ne
- Add your language to the `locales` config in `docs/astro.config.mjs`
- Add your language to the `locales` config in `docs/lunaria.config.json`
- Add your language’s subtag to the i18n label config in `.github/labeler.yml`
- Add your language to the `pa11y` script’s `--sitemap-exclude` flag in `package.json`
- Add your language to the `config.sitemap.exclude` option in `docs/__a11y__/test-utils.ts`
- Create the first translated page for your language.
This must be the Starlight landing page: `docs/src/content/docs/{language}/index.mdx`.
- Open a pull request on GitHub to add your changes to Starlight!
Expand Down
22 changes: 22 additions & 0 deletions docs/__a11y__/docs.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { expect, test } from './test-utils';

test('does not report accessibility violations on the docs site', async ({ docsSite }) => {
let violationsCount = 0;

const urls = await docsSite.getAllUrls();

for (const url of urls) {
const violations = await docsSite.testPage(url);

if (violations.length > 0) {
violationsCount += violations.length;
}

await docsSite.reportPageViolations(violations);
}

expect(
violationsCount,
`Found ${violationsCount} accessibility violations. Check the errors above for more details.`
).toBe(0);
});
155 changes: 155 additions & 0 deletions docs/__a11y__/test-utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
import { test as baseTest, type Page } from '@playwright/test';
import {
DefaultTerminalReporter,
getViolations,
injectAxe,
reportViolations,
} from 'axe-playwright';
import Sitemapper from 'sitemapper';

export { expect, type Locator } from '@playwright/test';

const config: Config = {
axe: {
// https://www.deque.com/axe/core-documentation/api-documentation/#axecore-tags
runOnly: {
type: 'tag',
values: ['wcag2a', 'wcag21a', 'wcag2aa', 'wcag21aa', 'wcag22aa', 'best-practice'],
},
},
// A list of violation to ignore.
ignore: [{ id: 'landmark-unique', nodeMatcher: landmarkUniqueNodeMatcher }],
sitemap: {
url: 'http://localhost:4321/sitemap-index.xml',
exclude: {
// A pattern to exclude URLs from the sitemap.
pattern: /\/(de|zh-cn|fr|es|pt-br|pt-pt|it|id|ko|ru|tr|hi|da|uk)\/.*/,
// A list of slugs to exclude from the sitemap after processing the pattern.
slugs: [
'components/using-components',
'getting-started',
'guides/customization',
'guides/i18n',
'guides/overriding-components',
'guides/pages',
'guides/project-structure',
'guides/site-search',
'manual-setup',
'reference/frontmatter',
'reference/overrides',
'reference/plugins',
],
},
replace: {
query: 'https://starlight.astro.build',
value: 'http://localhost:4321',
},
},
};

process.env.ASTRO_TELEMETRY_DISABLED = 'true';
process.env.ASTRO_DISABLE_UPDATE_CHECK = 'true';

export const test = baseTest.extend<{
docsSite: DocsSite;
}>({
docsSite: async ({ page }, use) => use(new DocsSite(page)),
});

// A Playwright test fixture accessible from within all tests.
class DocsSite {
constructor(private readonly page: Page) {}

async getAllUrls() {
const sitemap = new Sitemapper({ url: config.sitemap.url });
const { sites } = await sitemap.fetch();

if (sites.length === 0) {
throw new Error('No URLs found in sitemap.');
}

const urls: string[] = [];

for (const site of sites) {
const url = site.replace(config.sitemap.replace.query, config.sitemap.replace.value);
if (config.sitemap.exclude.pattern.test(url)) continue;
if (config.sitemap.exclude.slugs.some((slug) => url.endsWith(`/${slug}/`))) continue;
urls.push(url);
}

return urls;
}

async testPage(url: string) {
await this.page.goto(url);
await injectAxe(this.page);
await this.page.waitForLoadState('networkidle');
const violations = await getViolations(this.page, undefined, config.axe);
return this.#filterViolations(violations);
}

async reportPageViolations(violations: Awaited<ReturnType<typeof this.testPage>>) {
const url = this.page.url().replace(config.sitemap.replace.value, '');

if (violations.length > 0) {
console.error(`> Found ${violations.length} violations on ${url}\n`);
await reportViolations(violations, new DefaultTerminalReporter(true, true, false));
console.error('\n');
} else {
console.log(`> Found no violations on ${url}`);
}
}

#filterViolations(violations: Awaited<ReturnType<typeof getViolations>>) {
return violations.filter((violation) => {
return !config.ignore.some((ignore) => {
if (typeof ignore === 'string') return violation.id === ignore;
if (violation.id !== ignore.id) return false;
if (!ignore.nodeMatcher) return true;
return !violation.nodes.some(ignore.nodeMatcher);
});
});
}
}

function landmarkUniqueNodeMatcher(node: ViolationNode) {
/**
* Ignore the `landmark-unique` violation only if the node HTML is an aside.
*
* The best action to fix this violation would be to remove the landmark altogether as it's not
* necessary in this case and switch to the `note` role. Although, this is not possible at the
* moment due to an issue with NVDA not announcing it and also skipping the associated label for
* a role not supported.
*
* @see https://github.com/nvaccess/nvda/issues/10439
* @see https://github.com/withastro/starlight/pull/2503
*/
return !/^<aside[^>]* class="starlight-aside[^>]*>$/.test(node.html);
}

interface Config {
axe: Parameters<typeof getViolations>[2];
ignore: Array<
| string
| {
id: string;
// A function called for each node to evaluate if it should be ignored or not.
// Return `true` if the node should be considered for the violation, `false` otherwise.
nodeMatcher?: (node: ViolationNode) => boolean;
}
>;
sitemap: {
url: string;
exclude: {
pattern: RegExp;
slugs: string[];
};
replace: {
query: string;
value: string;
};
};
}

type Violations = Awaited<ReturnType<typeof getViolations>>;
type ViolationNode = Violations[number]['nodes'][number];
2 changes: 1 addition & 1 deletion docs/astro.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export default defineConfig({
attrs: { property: 'og:image:alt', content: ogImageAlt },
},
],
customCss: process.env.NO_GRADIENTS ? [] : ['./src/assets/landing.css'],
customCss: ['./src/assets/landing.css'],
locales,
sidebar: [
{
Expand Down
10 changes: 5 additions & 5 deletions docs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@
"type": "module",
"version": "0.0.1",
"scripts": {
"test": "start-server-and-test 'pnpm preview' http://localhost:4321 'pnpm pa11y'",
"pa11y": "pa11y-ci --sitemap 'http://localhost:4321/sitemap-0.xml' --sitemap-find 'https://starlight.astro.build' --sitemap-replace 'http://localhost:4321' --sitemap-exclude '/(de|zh-cn|fr|es|pt-br|pt-pt|it|id|ko|ru|tr|hi|da|uk)/.*'",
"test": "playwright install --with-deps chromium && playwright test",
"dev": "astro dev",
"start": "astro dev",
"build": "astro build",
Expand All @@ -25,8 +24,9 @@
"sharp": "^0.32.5"
},
"devDependencies": {
"pa11y-ci": "^3.0.1",
"starlight-links-validator": "^0.12.1",
"start-server-and-test": "^2.0.4"
"@playwright/test": "^1.45.0",
"axe-playwright": "^2.0.3",
"sitemapper": "^3.2.12",
"starlight-links-validator": "^0.12.1"
}
}
28 changes: 28 additions & 0 deletions docs/playwright.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { defineConfig, devices } from '@playwright/test';

export default defineConfig({
forbidOnly: !!process.env['CI'],
projects: [
{
name: 'Chrome Stable',
use: {
...devices['Desktop Chrome'],
headless: true,
},
},
],
testMatch: '__a11y__/*.test.ts',
// The timeout for the accessibility tests only.
timeout: 180 * 1_000,
webServer: [
{
command: 'pnpm run build && pnpm run preview',
reuseExistingServer: !process.env['CI'],
stdout: 'pipe',
// The timeout of the single build step ran before the accessibility tests.
timeout: 120 * 1_000,
url: 'http://localhost:4321',
},
],
workers: 1,
});
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
"peerDependencyRules": {
"ignoreMissing": [
"@algolia/client-search",
"playwright",
Copy link
Member Author

Choose a reason for hiding this comment

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

This is needed as axe-playwright has a peer dependency of playwright which we explicitly don't use in favor of @playwright/test. Both official packages are different in the sense that playwright automatically installs browser binaries while @playwright/test does not. Running pnpm test:a11y in the packages/starlight/ directory will take care of installing the necessary dependencies in our case only when needed.

"search-insights"
]
}
Expand Down
4 changes: 3 additions & 1 deletion packages/starlight/__e2e__/tabs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,9 @@ test('syncs and restores nested tabs', async ({ page, getProdServer }) => {

async function expectSelectedTab(tabs: Locator, label: string, panel?: string) {
expect(
(await tabs.locator(':scope > div [role=tab][aria-selected=true]').textContent())?.trim()
(
await tabs.locator(':scope > div:first-child [role=tab][aria-selected=true]').textContent()
)?.trim()
).toBe(label);

if (panel) {
Expand Down
2 changes: 1 addition & 1 deletion packages/starlight/__tests__/remark-rehype/asides.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const processor = await createMarkdownProcessor({
],
});

test('generates <aside>', async () => {
test('generates aside', async () => {
const res = await processor.render(`
:::note
Some text
Expand Down
Loading
Loading