Skip to content

Commit 752ce26

Browse files
authored
Auto-generate aria-labels for octicons and remove linter rule (#58349)
1 parent f677421 commit 752ce26

File tree

9 files changed

+126
-223
lines changed

9 files changed

+126
-223
lines changed

data/reusables/contributing/content-linter-rules.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@
5252
| GHD041 | third-party-action-pinning | Code examples that use third-party actions must always pin to a full length commit SHA | error | feature, actions |
5353
| GHD042 | liquid-tag-whitespace | Liquid tags should start and end with one whitespace. Liquid tag arguments should be separated by only one whitespace. | error | liquid, format |
5454
| GHD043 | link-quotation | Internal link titles must not be surrounded by quotations | error | links, url |
55-
| GHD044 | octicon-aria-labels | Octicons should always have an aria-label attribute even if aria-hidden. | warning | accessibility, octicons |
5655
| GHD045 | code-annotation-comment-spacing | Code comments in annotation blocks must have exactly one space after the comment character(s) | warning | code, comments, annotate, spacing |
5756
| GHD046 | outdated-release-phase-terminology | Outdated release phase terminology should be replaced with current GitHub terminology | warning | terminology, consistency, release-phases |
5857
| GHD047 | table-column-integrity | Tables must have consistent column counts across all rows | warning | tables, accessibility, formatting |
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import { beforeAll, describe, expect, test } from 'vitest'
2+
3+
import { get } from '@/tests/helpers/e2etest'
4+
5+
const makeURL = (pathname: string): string =>
6+
`/api/article/body?${new URLSearchParams({ pathname })}`
7+
8+
describe('article body api', () => {
9+
beforeAll(() => {
10+
// If you didn't set the `ROOT` variable, the tests will fail rather
11+
// cryptically. So as a warning for engineers running these tests,
12+
// alert in case it was accidentally forgotten.
13+
if (!process.env.ROOT) {
14+
console.warn(
15+
'WARNING: The article body tests require the ROOT environment variable to be set to the fixture root',
16+
)
17+
}
18+
})
19+
20+
test('happy path', async () => {
21+
const res = await get(makeURL('/en/get-started/start-your-journey/hello-world'))
22+
expect(res.statusCode).toBe(200)
23+
expect(res.body).toContain('## Introduction')
24+
expect(res.body).toContain('This is just a test.')
25+
expect(res.headers['content-type']).toContain('text/markdown')
26+
})
27+
28+
test('octicons auto-generate aria-labels', async () => {
29+
const res = await get(makeURL('/en/get-started/start-your-journey/hello-world'))
30+
expect(res.statusCode).toBe(200)
31+
32+
// Check that octicons without aria-label get auto-generated ones
33+
expect(res.body).toContain('aria-label="check icon"')
34+
expect(res.body).toContain('aria-label="git branch icon"')
35+
})
36+
37+
test('octicons with custom aria-labels use the custom value', async () => {
38+
const res = await get(makeURL('/en/get-started/start-your-journey/hello-world'))
39+
expect(res.statusCode).toBe(200)
40+
41+
// Check that custom aria-labels are preserved
42+
expect(res.body).toContain('aria-label="Supported"')
43+
expect(res.body).toContain('aria-label="Not supported"')
44+
})
45+
46+
test('octicons with other attributes still get auto-generated aria-labels', async () => {
47+
const res = await get(makeURL('/en/get-started/start-your-journey/hello-world'))
48+
expect(res.statusCode).toBe(200)
49+
50+
// Check that octicons with width attribute still get aria-labels
51+
expect(res.body).toContain('aria-label="rocket icon"')
52+
expect(res.body).toContain('width="32"')
53+
})
54+
55+
test('a pathname that does not exist', async () => {
56+
const res = await get(makeURL('/en/never/heard/of'))
57+
expect(res.statusCode).toBe(404)
58+
const { error } = JSON.parse(res.body)
59+
expect(error).toBe("No page found for '/en/never/heard/of'")
60+
})
61+
62+
test('non-article pages return error', async () => {
63+
// Index pages are not articles and should not be renderable
64+
const res = await get(makeURL('/en/get-started'))
65+
expect(res.statusCode).toBe(403)
66+
const { error } = JSON.parse(res.body)
67+
expect(error).toContain("isn't yet available in markdown")
68+
})
69+
})

src/content-linter/lib/linting-rules/index.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ import { tableColumnIntegrity } from '@/content-linter/lib/linting-rules/table-c
4242
import { thirdPartyActionPinning } from '@/content-linter/lib/linting-rules/third-party-action-pinning'
4343
import { liquidTagWhitespace } from '@/content-linter/lib/linting-rules/liquid-tag-whitespace'
4444
import { linkQuotation } from '@/content-linter/lib/linting-rules/link-quotation'
45-
import { octiconAriaLabels } from '@/content-linter/lib/linting-rules/octicon-aria-labels'
4645
import { liquidIfversionVersions } from '@/content-linter/lib/linting-rules/liquid-ifversion-versions'
4746
import { outdatedReleasePhaseTerminology } from '@/content-linter/lib/linting-rules/outdated-release-phase-terminology'
4847
import { frontmatterVersionsWhitespace } from '@/content-linter/lib/linting-rules/frontmatter-versions-whitespace'
@@ -105,7 +104,7 @@ export const gitHubDocsMarkdownlint = {
105104
thirdPartyActionPinning, // GHD041
106105
liquidTagWhitespace, // GHD042
107106
linkQuotation, // GHD043
108-
octiconAriaLabels, // GHD044
107+
// GHD044 removed - octicon aria-labels are now auto-generated
109108
codeAnnotationCommentSpacing, // GHD045
110109
outdatedReleasePhaseTerminology, // GHD046
111110
tableColumnIntegrity, // GHD047

src/content-linter/lib/linting-rules/octicon-aria-labels.ts

Lines changed: 0 additions & 58 deletions
This file was deleted.

src/content-linter/style/github-docs.ts

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -162,12 +162,7 @@ const githubDocsConfig = {
162162
'partial-markdown-files': true,
163163
'yml-files': true,
164164
},
165-
'octicon-aria-labels': {
166-
// GHD044
167-
severity: 'warning',
168-
'partial-markdown-files': true,
169-
'yml-files': true,
170-
},
165+
// GHD044 removed - octicon aria-labels are now auto-generated
171166
'code-annotation-comment-spacing': {
172167
// GHD045
173168
severity: 'warning',

src/content-linter/tests/unit/octicon-aria-labels.ts

Lines changed: 0 additions & 155 deletions
This file was deleted.

src/content-render/liquid/octicon.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,10 @@ const SyntaxHelp = 'Syntax Error in tag \'octicon\' - Valid syntax: octicon "<na
2424
* Uses the octicons library to render the chosen icon. Also
2525
* supports passing attributes like `width="64"`.
2626
*
27-
* {% octicon "check" %}
27+
* If no aria-label is provided, a default one will be auto-generated
28+
* based on the icon name (e.g., "check icon", "git-branch icon").
29+
*
30+
* {% octicon "check" %} <!-- auto-generates aria-label="check icon" -->
2831
* {% octicon "check" width="64" aria-label="Example label" %}
2932
*/
3033
const Octicon: LiquidTag = {
@@ -70,6 +73,13 @@ const Octicon: LiquidTag = {
7073
throw new Error(`Octicon ${this.icon} does not exist`)
7174
}
7275

76+
// Auto-generate aria-label if not provided
77+
// Replace non-alphanumeric characters with spaces and append " icon"
78+
if (!this.options['aria-label']) {
79+
const defaultLabel = `${this.icon.toLowerCase().replace(/[^a-z0-9]+/gi, ' ')} icon`
80+
this.options['aria-label'] = defaultLabel
81+
}
82+
7383
const result: string = octicons[this.icon].toSVG(this.options)
7484
return result
7585
},

src/content-render/tests/octicon.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,4 +42,34 @@ describe('octicon tag', () => {
4242
'Octicon pizza-patrol does not exist',
4343
)
4444
})
45+
46+
test('auto-generates aria-label when not provided', async () => {
47+
const actual = await renderContent('{% octicon "check" %}')
48+
expect(actual).toContain('<svg ')
49+
expect(actual).toContain('class="octicon octicon-check"')
50+
expect(actual).toContain('aria-label="check icon"')
51+
})
52+
53+
test('auto-generates aria-label with spaces for hyphenated icon names', async () => {
54+
const actual = await renderContent('{% octicon "git-branch" %}')
55+
expect(actual).toContain('<svg ')
56+
expect(actual).toContain('class="octicon octicon-git-branch"')
57+
expect(actual).toContain('aria-label="git branch icon"')
58+
})
59+
60+
test('uses custom aria-label instead of auto-generated one when provided', async () => {
61+
const actual = await renderContent('{% octicon "check" aria-label="Supported" %}')
62+
expect(actual).toContain('<svg ')
63+
expect(actual).toContain('class="octicon octicon-check"')
64+
expect(actual).toContain('aria-label="Supported"')
65+
expect(actual).not.toContain('aria-label="check icon"')
66+
})
67+
68+
test('auto-generates aria-label even when other attributes are provided', async () => {
69+
const actual = await renderContent('{% octicon "filter" width="32" %}')
70+
expect(actual).toContain('<svg ')
71+
expect(actual).toContain('class="octicon octicon-filter"')
72+
expect(actual).toContain('aria-label="filter icon"')
73+
expect(actual).toContain('width="32"')
74+
})
4575
})

0 commit comments

Comments
 (0)