This repository has been archived by the owner on Sep 30, 2024. It is now read-only.
Enable accessibilityAudit on batches.test.ts
#33824
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Codenotify: Notifying subscribers in CODENOTIFY files for diff 061438d...d6a7579.
|
|
Codenotify: Notifying subscribers in OWNERS files for diff 061438d...d6a7579. No notifications. |
gitstart-sourcegraph
changed the title
batches.test.ts
umpox
reviewed
Apr 13, 2022
| Rule: "heading-order" (Heading levels should only increase by one) | ||
| To support accessibility rule here, we would need to use h3 tag, since PageHeader(on upper scope) renders h1 and h2 subsequently | ||
| */} | ||
| <h5 className="a11y-ignore p-2 d-none d-md-block text-uppercase text-center text-nowrap">Status</h5> |
There was a problem hiding this comment.
Can we use a h3 tag?
It should be possible to do this and preserve styles using the WIldcard components
<H5 as={H3}>{label}</H5>
| {(licenseAndUsageInfo?.allBatchChanges.totalCount || 0) > 0 && ( | ||
| <h3 className="align-self-end flex-1">{lastTotalCount} batch changes</h3> | ||
| <h3 className="a11y-ignore align-self-end flex-1">{lastTotalCount} batch changes</h3> |
There was a problem hiding this comment.
We should be able to use H2 here too :)
| Rule: "heading-order" (Heading levels should only increase by one) | ||
| To support accessibility rule here, we would need to use h3 tag, since PageHeader(on upper scope) renders h1 and h2 subsequently | ||
| */} | ||
| <h5 className="a11y-ignore p-2 d-none d-sm-block text-uppercase text-center">Current state</h5> |
| Rule: "heading-order" (Heading levels should only increase by one) | ||
| To support accessibility rule here, we would need to use h4 tag and have h2 somewhere in the page, since PageHeader(on upper scope) renders h1 tag and there is an h3 tag as well. | ||
| */} | ||
| <h5 className="a11y-ignore"> |
gitstart-sourcegraph
force-pushed
the
contractors/SG-33093.1.2
branch
from
617221a to
52b5248
Compare
gitstart-sourcegraph
force-pushed
the
contractors/SG-33093.1.2
branch
from
52b5248 to
d6a7579
Compare
umpox
approved these changes
Apr 19, 2022
|
@gitstart-sourcegraph Looks like there are some visual regressions on the batch changes heading, please can we fix? |
Comment on lines
+331
to
333
| // To fix accessibility rule: "heading-order" | ||
| description: '## Very cool batch change', | ||
| creator: { |
There was a problem hiding this comment.
@gitstart-sourcegraph Looks like there are some visual regressions on the batch changes heading, please can we fix?
Hi @umpox, the UI diff as displayed in the percySnapshot occurs with respect to this fix we made here, which involved updating the test parameter from rendering an h3 (fails accessibility check: "heading-order") to instead render h2 (as header description in markdown with ##).
BTW this would not have an effect on actual rendering on web app, since description is generated with user input.
WDYT?
There was a problem hiding this comment.
Ah I see, I missed that - sorry!
LGTM :)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
batchespage in its integration testRefs
SG Issue
Gitstart Ticket
Test plan
Run the integration test
yarn test-integration:base client/web/src/integration/batches.test.tsSuccess criteria
accessibilityAuditenabledApp preview:
Check out the client app preview documentation to learn more.