Skip to content
This repository has been archived by the owner on Sep 30, 2024. It is now read-only.

[SG-33093] Enable accessibilityAudit on client/web/src/integration/batches.test.ts #33332

Merged
merged 11 commits into from
Apr 5, 2022
Merged

[SG-33093] Enable accessibilityAudit on client/web/src/integration/batches.test.ts #33332

merged 11 commits into from
Apr 5, 2022

Conversation

gitstart-sourcegraph
Copy link
Collaborator

@gitstart-sourcegraph gitstart-sourcegraph commented Apr 2, 2022 •
edited by sourcegraph-bot
Loading

Description

  • Enabling accessibility audit on batches page in its integration test

Refs

SG Issue
Gitstart Ticket

Test plan

Run the integration test yarn test-integration:base client/web/src/integration/batches.test.ts

Success criteria

  • All integration tests with percy snapshots have accessibilityAudit enabled

App preview:

@cla-bot cla-bot bot added the cla-signed label Apr 2, 2022
@sourcegraph-bot
Copy link
Contributor

sourcegraph-bot commented Apr 2, 2022 •
edited
Loading

Codenotify: Notifying subscribers in CODENOTIFY files for diff 3de688c...a51a195.

Notify File(s)
@courier-new client/web/src/enterprise/batches/list/GettingStarted.tsx
client/web/src/integration/batches.test.ts
@eseliger client/web/src/enterprise/batches/list/GettingStarted.tsx
client/web/src/integration/batches.test.ts

@sourcegraph-bot
Copy link
Contributor

sourcegraph-bot commented Apr 2, 2022 •
edited
Loading

Codenotify: Notifying subscribers in OWNERS files for diff 3de688c...a51a195.

Notify File(s)
@sourcegraph/frontend-platform-devs client/wildcard/src/components/Menu/Menu.tsx
client/wildcard/src/components/Menu/MenuButton.tsx
client/wildcard/src/components/NavMenu/snapshots/NavMenu.test.tsx.snap
client/wildcard/src/components/PageHeader/PageHeader.tsx
client/wildcard/src/components/PageHeader/snapshots/PageHeader.test.tsx.snap

@gitstart-sourcegraph gitstart-sourcegraph self-assigned this Apr 3, 2022
@gitstart-sourcegraph gitstart-sourcegraph changed the title [SG-33093.1] Enable accessibilityAudit on client/web/src/integration/batches.test.ts [SG-33093] Enable accessibilityAudit on client/web/src/integration/batches.test.ts Apr 3, 2022
@gitstart-sourcegraph gitstart-sourcegraph added the gitstart Contract partner label Apr 3, 2022
@gitstart-sourcegraph gitstart-sourcegraph linked an issue Apr 3, 2022 that may be closed by this pull request
Accessibility: Enable accessibilityAudit on different pages in our integration tests #33093
Closed
@gitstart-sourcegraph
Copy link
Collaborator Author

gitstart-sourcegraph commented Apr 3, 2022 •
edited
Loading

@umpox @valerybugakov could you review the first PR for accessibilityAudit? We need your suggestions on some a11y-ignore usages (already left comments for issues).

BTW it seems that we know another issue of current wildcard/Menu with Reach Menu + Popover, it's about Popover removes MenuList from DOM, it causes aria-controls attribute in MenuButton invalid

@umpox
Copy link
Contributor

umpox commented Apr 4, 2022

@gitstart-sourcegraph The aria-controls issue is interesting. We could work around this by using isExpanded from Menu to only add aria-controls when visible. I think this is OK though, it seems from reading this issue: w3c/aria#995 that aria-controls has relatively low support. We should be able to make these components accessible without it

umpox
umpox suggested changes Apr 4, 2022
View reviewed changes
Copy link
Contributor

@umpox umpox left a comment

Choose a reason for hiding this comment

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

Thanks! Left some some comments

client/web/src/components/AppRouterContainer/AppRouterContainer.tsx Outdated Show resolved Hide resolved
client/web/src/enterprise/batches/list/GettingStarted.tsx Outdated Show resolved Hide resolved
client/web/src/nav/StatusMessagesNavItem.tsx Outdated Show resolved Hide resolved
@gitstart-sourcegraph
Copy link
Collaborator Author

@umpox could you help to check percy snapshots reports? It seems that all snapshots from this PR is new

umpox
umpox approved these changes Apr 5, 2022
View reviewed changes
Copy link
Contributor

@umpox umpox left a comment

Choose a reason for hiding this comment

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

Two small comments, rest looks good! I think the Percy changes aren't related to this PR, but because we're currently having some issues with Percy builds on main. Will look into it and have re-ran the build to check if that fixes

client/web/src/components/AppRouterContainer/AppRouterContainer.tsx Outdated Show resolved Hide resolved
client/wildcard/src/components/PageHeader/PageHeader.tsx Outdated Show resolved Hide resolved
valerybugakov
valerybugakov approved these changes Apr 5, 2022
View reviewed changes
Copy link
Member

@valerybugakov valerybugakov left a comment

Choose a reason for hiding this comment

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

Some jest tests need to be fixed to make Buidkite green again. Otherwise LGTM.

@gitstart-sourcegraph
Copy link
Collaborator Author

Thanks @valerybugakov @umpox all checks are good except percy check. Should we merge this PR or wait for the fixes?

umpox
umpox approved these changes Apr 5, 2022
View reviewed changes
Copy link
Contributor

@umpox umpox left a comment

Choose a reason for hiding this comment

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

@gitstart-sourcegraph I think the Percy issue should be fixed now, can you update with latest main and see if CI passes?

@gitstart-sourcegraph gitstart-sourcegraph merged commit 1021018 into main Apr 5, 2022
@gitstart-sourcegraph gitstart-sourcegraph deleted the contractors/SG-33093.1.1 branch April 5, 2022 16:56
thiswayman pushed a commit that referenced this pull request Apr 7, 2022
@gitstart-sourcegraph @gitstart @thiswayman
[SG-33093] Enable accessibilityAudit on `client/web/src/integration/b…
c7c86c1 
…atches.test.ts` (#33332)

* test: enable and fix accessibilityAudit test issues in batches.test.ts

Co-authored-by: gitstart-sourcegraph <gitstart@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@umpox umpox umpox approved these changes

@valerybugakov valerybugakov valerybugakov approved these changes

Assignees

@gitstart-sourcegraph gitstart-sourcegraph

Labels
cla-signed gitstart Contract partner
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Accessibility: Enable accessibilityAudit on different pages in our integration tests
5 participants
@gitstart-sourcegraph @sourcegraph-bot @umpox @valerybugakov @gitstart