Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue/5154 Accessibility compliance #5745

Closed
wants to merge 13 commits into from

Conversation

LukasStordeur
Copy link
Collaborator

Description

Implement Axe-Jest into our current tests. All pages are being checked whether they contain violations to the compliance rules set by the web-standards. The complete list of rules, grouped WCAG level and best practice, can found in doc/rule-descriptions.md.

closes #5154

@LukasStordeur LukasStordeur self-assigned this May 8, 2024
@@ -37,7 +37,7 @@ export const CompileStageReportTable: React.FC<Props> = ({
<Table {...props} variant={TableVariant.compact}>
<Thead>
<Tr>
<Th key="toggle" />
<Th aria-hidden key="toggle"></Th>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<Th aria-hidden key="toggle"></Th>
<Th aria-hidden key="toggle"></Th>
Suggested change
<Th aria-hidden key="toggle"></Th>
<Th aria-hidden key="toggle"/>

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Both are valid, but if you have any arguments about it, I'll gladly change it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Right, I just personally gravitate towards self-closing tags, probably as it most of the time if there are separate closing brackets then it feels like it should be something between them. But that's just me :D

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated

@@ -41,7 +41,7 @@ export const OrderDetailsTable: React.FC<Props> = ({
<Table {...props} variant={TableVariant.compact}>
<Thead>
<Tr>
<Th style={{ width: "15px" }}></Th>
<Th style={{ width: "15px" }} aria-hidden></Th>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<Th style={{ width: "15px" }} aria-hidden></Th>
<Th style={{ width: "15px" }} aria-hidden></Th>
Suggested change
<Th style={{ width: "15px" }} aria-hidden></Th>
<Th style={{ width: "15px" }} aria-hidden />

@@ -66,7 +66,7 @@ export const ResourcesTable: React.FC<Props> = ({
<Table {...props} variant={TableVariant.compact}>
<Thead>
<Tr>
<Th></Th>
<Th aria-hidden></Th>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Now as I take a look, shouldn't the {heads} be inside of the <Th>. that or just change the that component to be self-closing

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Those empty Head tags are on purpose to have a column without title for the collapsible arrows

@@ -62,7 +62,7 @@ export const DiscoveredResourcesTable: React.FC<Props> = ({
<Table {...props} variant={TableVariant.compact}>
<Thead>
<Tr>
<Th></Th>
<Th aria-hidden></Th>
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here, I know that it wasn't changed by you, but I just noticed that so wanted to point it out :)

@@ -40,7 +40,7 @@ export const HistoryTable: React.FC<Props> = ({ service, logs }) => {
<Table>
<Thead>
<Tr>
<Th />
<Th aria-hidden></Th>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<Th aria-hidden></Th>
<Th aria-hidden></Th>
Suggested change
<Th aria-hidden></Th>
<Th aria-hidden />

@@ -78,7 +79,7 @@ export const InventoryTable: React.FC<Props> = ({
<Table {...props}>
<Thead>
<Tr>
<Th />
<Th aria-hidden></Th>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<Th aria-hidden></Th>
<Th aria-hidden></Th>
Suggested change
<Th aria-hidden></Th>
<Th aria-hidden />

@LukasStordeur LukasStordeur added the merge-tool-ready This ticket is ready to be merged in label May 15, 2024
@inmantaci
Copy link
Contributor

Processing this pull request

inmantaci pushed a commit that referenced this pull request May 15, 2024
# Description

Implement Axe-Jest into our current tests. All pages are being checked whether they contain violations to the compliance rules set by the web-standards. The complete list of rules, grouped WCAG level and best practice, can found in [doc/rule-descriptions.md](https://github.com/dequelabs/axe-core/blob/develop/doc/rule-descriptions.md).

closes #5154
@inmantaci
Copy link
Contributor

Merged into branches master, iso7 in 63a0fb7

@inmantaci inmantaci closed this May 15, 2024
@inmantaci inmantaci deleted the issue/5154-compliance-owasp branch May 15, 2024 06:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge-tool-ready This ticket is ready to be merged in
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve Accessibility to match full compliance
3 participants