Skip to content

Commit cc07ea1

Browse files
authored
Adds links to screenshots when include_screenshots is true (#137)
Adds links to screenshots (off by default). This can be configured and turned on if `include_screenshots` is set to `true`. GH Staff only - [testing results](github/accessibility#10186 (comment)) <img width="932" height="878" alt="Screen shot of an issue with a new 'View screenshot' link" src="https://github.com/user-attachments/assets/854b2007-0ff7-4889-ac3c-c1c2913de59f" />
2 parents 95a0df4 + 0b1f891 commit cc07ea1

20 files changed

+345
-22
lines changed

.github/actions/file/action.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ inputs:
1414
cached_filings:
1515
description: "Cached filings from previous runs, as stringified JSON. Without this, duplicate issues may be filed."
1616
required: false
17+
screenshot_repository:
18+
description: "Repository (with owner) where screenshots are stored on the gh-cache branch. Defaults to the 'repository' input if not set. Required if issues are open in a different repo to construct proper screenshot URLs."
19+
required: false
1720

1821
outputs:
1922
filings:

.github/actions/file/src/generateIssueBody.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type {Finding} from './types.d.js'
22

3-
export function generateIssueBody(finding: Finding): string {
3+
export function generateIssueBody(finding: Finding, screenshotRepo: string): string {
44
const solutionLong = finding.solutionLong
55
?.split('\n')
66
.map((line: string) =>
@@ -9,15 +9,26 @@ export function generateIssueBody(finding: Finding): string {
99
: line,
1010
)
1111
.join('\n')
12+
13+
let screenshotSection
14+
if (finding.screenshotId) {
15+
const screenshotUrl = `https://github.com/${screenshotRepo}/blob/gh-cache/.screenshots/${finding.screenshotId}.png`
16+
screenshotSection = `
17+
[View screenshot](${screenshotUrl})
18+
`
19+
}
20+
1221
const acceptanceCriteria = `## Acceptance Criteria
1322
- [ ] The specific axe violation reported in this issue is no longer reproducible.
1423
- [ ] The fix MUST meet WCAG 2.1 guidelines OR the accessibility standards specified by the repository or organization.
1524
- [ ] A test SHOULD be added to ensure this specific axe violation does not regress.
1625
- [ ] This PR MUST NOT introduce any new accessibility issues or regressions.
1726
`
27+
1828
const body = `## What
1929
An accessibility scan flagged the element \`${finding.html}\` on ${finding.url} because ${finding.problemShort}. Learn more about why this was flagged by visiting ${finding.problemUrl}.
2030
31+
${screenshotSection ?? ''}
2132
To fix this, ${finding.solutionShort}.
2233
${solutionLong ? `\nSpecifically:\n\n${solutionLong}` : ''}
2334

.github/actions/file/src/index.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,13 @@ export default async function () {
1818
const findings: Finding[] = JSON.parse(core.getInput('findings', {required: true}))
1919
const repoWithOwner = core.getInput('repository', {required: true})
2020
const token = core.getInput('token', {required: true})
21+
const screenshotRepo = core.getInput('screenshot_repository', {required: false}) || repoWithOwner
2122
const cachedFilings: (ResolvedFiling | RepeatedFiling)[] = JSON.parse(
2223
core.getInput('cached_filings', {required: false}) || '[]',
2324
)
2425
core.debug(`Input: 'findings: ${JSON.stringify(findings)}'`)
2526
core.debug(`Input: 'repository: ${repoWithOwner}'`)
27+
core.debug(`Input: 'screenshot_repository: ${screenshotRepo}'`)
2628
core.debug(`Input: 'cached_filings: ${JSON.stringify(cachedFilings)}'`)
2729

2830
const octokit = new OctokitWithThrottling({
@@ -55,12 +57,18 @@ export default async function () {
5557
filing.issue.state = 'closed'
5658
} else if (isNewFiling(filing)) {
5759
// Open a new issue for the filing
58-
response = await openIssue(octokit, repoWithOwner, filing.findings[0])
60+
response = await openIssue(octokit, repoWithOwner, filing.findings[0], screenshotRepo)
5961
// eslint-disable-next-line @typescript-eslint/no-explicit-any
6062
;(filing as any).issue = {state: 'open'} as Issue
6163
} else if (isRepeatedFiling(filing)) {
62-
// Reopen the filing’s issue (if necessary)
63-
response = await reopenIssue(octokit, new Issue(filing.issue))
64+
// Reopen the filing's issue (if necessary) and update the body with the latest finding
65+
response = await reopenIssue(
66+
octokit,
67+
new Issue(filing.issue),
68+
filing.findings[0],
69+
repoWithOwner,
70+
screenshotRepo,
71+
)
6472
filing.issue.state = 'reopened'
6573
}
6674
if (response?.data && filing.issue) {

.github/actions/file/src/openIssue.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ function truncateWithEllipsis(text: string, maxLength: number): string {
1717
return text.length > maxLength ? text.slice(0, maxLength - 1) + '…' : text
1818
}
1919

20-
export async function openIssue(octokit: Octokit, repoWithOwner: string, finding: Finding) {
20+
export async function openIssue(octokit: Octokit, repoWithOwner: string, finding: Finding, screenshotRepo?: string) {
2121
const owner = repoWithOwner.split('/')[0]
2222
const repo = repoWithOwner.split('/')[1]
2323

@@ -27,7 +27,7 @@ export async function openIssue(octokit: Octokit, repoWithOwner: string, finding
2727
GITHUB_ISSUE_TITLE_MAX_LENGTH,
2828
)
2929

30-
const body = generateIssueBody(finding)
30+
const body = generateIssueBody(finding, screenshotRepo ?? repoWithOwner)
3131

3232
return octokit.request(`POST /repos/${owner}/${repo}/issues`, {
3333
owner,
Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,25 @@
11
import type {Octokit} from '@octokit/core'
22
import type {Issue} from './Issue.js'
3+
import type {Finding} from './types.d.js'
4+
import {generateIssueBody} from './generateIssueBody.js'
5+
6+
export async function reopenIssue(
7+
octokit: Octokit,
8+
{owner, repository, issueNumber}: Issue,
9+
finding?: Finding,
10+
repoWithOwner?: string,
11+
screenshotRepo?: string,
12+
) {
13+
let body: string | undefined
14+
if (finding && repoWithOwner) {
15+
body = generateIssueBody(finding, screenshotRepo ?? repoWithOwner)
16+
}
317

4-
export async function reopenIssue(octokit: Octokit, {owner, repository, issueNumber}: Issue) {
518
return octokit.request(`PATCH /repos/${owner}/${repository}/issues/${issueNumber}`, {
619
owner,
720
repository,
821
issue_number: issueNumber,
922
state: 'open',
23+
body,
1024
})
1125
}

.github/actions/file/src/types.d.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ export type Finding = {
77
problemUrl: string
88
solutionShort: string
99
solutionLong?: string
10+
screenshotId?: string
1011
}
1112

1213
export type Issue = {

.github/actions/file/tests/generateIssueBody.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,4 +47,18 @@ describe('generateIssueBody', () => {
4747
expect(body).not.toContain('- Fix any of the following:')
4848
expect(body).not.toContain('- Fix all of the following:')
4949
})
50+
51+
it('uses the screenshotRepo for the screenshot URL, not the filing repo', () => {
52+
const body = generateIssueBody({...baseFinding, screenshotId: 'abc123'}, 'github/my-workflow-repo')
53+
54+
expect(body).toContain('github/my-workflow-repo/blob/gh-cache/.screenshots/abc123.png')
55+
expect(body).not.toContain('github/accessibility-scanner')
56+
})
57+
58+
it('omits screenshot section when screenshotId is not present', () => {
59+
const body = generateIssueBody(baseFinding, 'github/accessibility-scanner')
60+
61+
expect(body).not.toContain('View screenshot')
62+
expect(body).not.toContain('.screenshots')
63+
})
5064
})
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
import {describe, it, expect, vi} from 'vitest'
2+
3+
// Mock generateIssueBody so we can inspect what screenshotRepo is passed
4+
vi.mock('../src/generateIssueBody.js', () => ({
5+
generateIssueBody: vi.fn((_finding, screenshotRepo: string) => `body with screenshotRepo=${screenshotRepo}`),
6+
}))
7+
8+
import {openIssue} from '../src/openIssue.ts'
9+
import {generateIssueBody} from '../src/generateIssueBody.ts'
10+
11+
const baseFinding = {
12+
scannerType: 'axe',
13+
ruleId: 'color-contrast',
14+
url: 'https://example.com/page',
15+
html: '<span>Low contrast</span>',
16+
problemShort: 'elements must meet minimum color contrast ratio thresholds',
17+
problemUrl: 'https://dequeuniversity.com/rules/axe/4.10/color-contrast?application=playwright',
18+
solutionShort: 'ensure the contrast between foreground and background colors meets WCAG thresholds',
19+
}
20+
21+
function mockOctokit() {
22+
return {
23+
request: vi.fn().mockResolvedValue({data: {id: 1, html_url: 'https://github.com/org/repo/issues/1'}}),
24+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
25+
} as any
26+
}
27+
28+
describe('openIssue', () => {
29+
it('passes screenshotRepo to generateIssueBody when provided', async () => {
30+
const octokit = mockOctokit()
31+
await openIssue(octokit, 'org/filing-repo', baseFinding, 'org/workflow-repo')
32+
33+
expect(generateIssueBody).toHaveBeenCalledWith(baseFinding, 'org/workflow-repo')
34+
})
35+
36+
it('falls back to repoWithOwner when screenshotRepo is not provided', async () => {
37+
const octokit = mockOctokit()
38+
await openIssue(octokit, 'org/filing-repo', baseFinding)
39+
40+
expect(generateIssueBody).toHaveBeenCalledWith(baseFinding, 'org/filing-repo')
41+
})
42+
43+
it('posts to the correct filing repo, not the screenshot repo', async () => {
44+
const octokit = mockOctokit()
45+
await openIssue(octokit, 'org/filing-repo', baseFinding, 'org/workflow-repo')
46+
47+
expect(octokit.request).toHaveBeenCalledWith(
48+
'POST /repos/org/filing-repo/issues',
49+
expect.objectContaining({
50+
owner: 'org',
51+
repo: 'filing-repo',
52+
}),
53+
)
54+
})
55+
56+
it('includes the correct labels based on the finding', async () => {
57+
const octokit = mockOctokit()
58+
await openIssue(octokit, 'org/repo', baseFinding)
59+
60+
expect(octokit.request).toHaveBeenCalledWith(
61+
expect.any(String),
62+
expect.objectContaining({
63+
labels: ['axe rule: color-contrast', 'axe-scanning-issue'],
64+
}),
65+
)
66+
})
67+
68+
it('truncates long titles with ellipsis', async () => {
69+
const octokit = mockOctokit()
70+
const longFinding = {
71+
...baseFinding,
72+
problemShort: 'a'.repeat(300),
73+
}
74+
await openIssue(octokit, 'org/repo', longFinding)
75+
76+
const callArgs = octokit.request.mock.calls[0][1]
77+
expect(callArgs.title.length).toBeLessThanOrEqual(256)
78+
expect(callArgs.title).toMatch(/$/)
79+
})
80+
})
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
import {describe, it, expect, vi, beforeEach} from 'vitest'
2+
3+
// Mock generateIssueBody so we can inspect what screenshotRepo is passed
4+
vi.mock('../src/generateIssueBody.js', () => ({
5+
generateIssueBody: vi.fn((_finding, screenshotRepo: string) => `body with screenshotRepo=${screenshotRepo}`),
6+
}))
7+
8+
import {reopenIssue} from '../src/reopenIssue.ts'
9+
import {generateIssueBody} from '../src/generateIssueBody.ts'
10+
import {Issue} from '../src/Issue.ts'
11+
12+
const baseFinding = {
13+
scannerType: 'axe',
14+
ruleId: 'color-contrast',
15+
url: 'https://example.com/page',
16+
html: '<span>Low contrast</span>',
17+
problemShort: 'elements must meet minimum color contrast ratio thresholds',
18+
problemUrl: 'https://dequeuniversity.com/rules/axe/4.10/color-contrast?application=playwright',
19+
solutionShort: 'ensure the contrast between foreground and background colors meets WCAG thresholds',
20+
}
21+
22+
const testIssue = new Issue({
23+
id: 42,
24+
nodeId: 'MDU6SXNzdWU0Mg==',
25+
url: 'https://github.com/org/filing-repo/issues/7',
26+
title: 'Accessibility issue: test',
27+
state: 'closed',
28+
})
29+
30+
function mockOctokit() {
31+
return {
32+
request: vi.fn().mockResolvedValue({data: {id: 42, html_url: 'https://github.com/org/filing-repo/issues/7'}}),
33+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
34+
} as any
35+
}
36+
37+
describe('reopenIssue', () => {
38+
beforeEach(() => {
39+
vi.clearAllMocks()
40+
})
41+
42+
it('passes screenshotRepo to generateIssueBody when provided', async () => {
43+
const octokit = mockOctokit()
44+
await reopenIssue(octokit, testIssue, baseFinding, 'org/filing-repo', 'org/workflow-repo')
45+
46+
expect(generateIssueBody).toHaveBeenCalledWith(baseFinding, 'org/workflow-repo')
47+
})
48+
49+
it('falls back to repoWithOwner when screenshotRepo is not provided', async () => {
50+
const octokit = mockOctokit()
51+
await reopenIssue(octokit, testIssue, baseFinding, 'org/filing-repo')
52+
53+
expect(generateIssueBody).toHaveBeenCalledWith(baseFinding, 'org/filing-repo')
54+
})
55+
56+
it('does not generate a body when finding is not provided', async () => {
57+
const octokit = mockOctokit()
58+
await reopenIssue(octokit, testIssue)
59+
60+
expect(generateIssueBody).not.toHaveBeenCalled()
61+
expect(octokit.request).toHaveBeenCalledWith(
62+
expect.any(String),
63+
expect.not.objectContaining({body: expect.anything()}),
64+
)
65+
})
66+
67+
it('does not generate a body when repoWithOwner is not provided', async () => {
68+
const octokit = mockOctokit()
69+
await reopenIssue(octokit, testIssue, baseFinding)
70+
71+
expect(generateIssueBody).not.toHaveBeenCalled()
72+
})
73+
74+
it('sends PATCH to the correct issue URL with state open', async () => {
75+
const octokit = mockOctokit()
76+
await reopenIssue(octokit, testIssue, baseFinding, 'org/filing-repo', 'org/workflow-repo')
77+
78+
expect(octokit.request).toHaveBeenCalledWith(
79+
'PATCH /repos/org/filing-repo/issues/7',
80+
expect.objectContaining({
81+
state: 'open',
82+
issue_number: 7,
83+
}),
84+
)
85+
})
86+
87+
it('includes generated body when finding and repoWithOwner are provided', async () => {
88+
const octokit = mockOctokit()
89+
await reopenIssue(octokit, testIssue, baseFinding, 'org/filing-repo', 'org/workflow-repo')
90+
91+
expect(octokit.request).toHaveBeenCalledWith(
92+
expect.any(String),
93+
expect.objectContaining({
94+
body: 'body with screenshotRepo=org/workflow-repo',
95+
}),
96+
)
97+
})
98+
})

.github/actions/find/action.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ inputs:
99
auth_context:
1010
description: "Stringified JSON object containing 'username', 'password', 'cookies', and/or 'localStorage' from an authenticated session"
1111
required: false
12+
include_screenshots:
13+
description: "Whether to capture screenshots of scanned pages and include links to them in the issue"
14+
required: false
15+
default: "false"
1216

1317
outputs:
1418
findings:

0 commit comments

Comments
 (0)