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

docs: updated accessibility section #4161

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Rajdeepc
Copy link
Contributor

@Rajdeepc Rajdeepc commented Mar 8, 2024

Description

Added Accessibility Testing status section to denote the contracts tested on different simulation before delivery.

Screenshot 2024-03-11 at 4 37 50 PM

Related issue(s)

Motivation and context

How has this been tested?

  • Test case 1
    1. Go here
    2. Do this
  • Test case 2
    1. Go here
    2. Do this

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (minor updates related to the tooling or maintenance of the repository, does not impact compiled assets)

Checklist

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • If my change required a change to the documentation, I have updated the documentation in this pull request.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have reviewed at the Accessibility Practices for this feature, see: Aria Practices

Best practices

This repository uses conventional commit syntax for each commit message; note that the GitHub UI does not use this by default so be cautious when accepting suggested changes. Avoid the "Update branch" button on the pull request and opt instead for rebasing your branch against main.

@Rajdeepc Rajdeepc added a11y Issues related to accessibility Documentation labels Mar 8, 2024
Copy link

github-actions bot commented Mar 8, 2024

Tachometer results

Chrome

accordion permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 415 kB 135.00ms - 137.94ms - unsure 🔍
-1% - +2%
-1.62ms - +2.26ms
branch 406 kB 134.89ms - 137.41ms unsure 🔍
-2% - +1%
-2.26ms - +1.62ms
-
Firefox

accordion permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 415 kB 259.41ms - 272.27ms - unsure 🔍
-4% - +3%
-9.39ms - +8.43ms
branch 406 kB 260.14ms - 272.50ms unsure 🔍
-3% - +4%
-8.43ms - +9.39ms
-

Copy link

github-actions bot commented Mar 8, 2024

Lighthouse scores

Category Latest (report) Main (report) Branch (report)
Performance 0.95 0.97 0.97
Accessibility 1 1 1
Best Practices 1 1 1
SEO 1 0.92 0.92
PWA 1 1 1
What is this?

Lighthouse scores comparing the documentation site built from the PR ("Branch") to that of the production documentation site ("Latest") and the build currently on main ("Main"). Higher scores are better, but note that the SEO scores on Netlify URLs are artifically constrained to 0.92.

Transfer Size

Category Latest Main Branch
Total 242.515 kB 228.849 kB 228.507 kB 🏆
Scripts 61.267 kB 54.57 kB 54.347 kB 🏆
Stylesheet 48.717 kB 42.391 kB 42.275 kB 🏆
Document 5.819 kB 5.176 kB 5.173 kB 🏆
Third Party 126.712 kB 126.712 kB 126.712 kB

Request Count

Category Latest Main Branch
Total 43 43 43
Scripts 35 35 35
Stylesheet 5 5 5
Document 1 1 1
Third Party 2 2 2

@Rajdeepc Rajdeepc requested a review from Westbrook March 8, 2024 09:17
@Westbrook
Copy link
Collaborator

I really like how this looks, but I know sure what specifically it adds to the docs site for it to be the same on every page. The component link almost always links to the top of the page you're already looking at at I'm not sure how the GitHub repo is relavant to an accessibility report. If we're just using this statically would it be better to make a single page with a little more content that outlines that those test are or so? That would also give us a place to link the content here to:

image

Comment on lines 203 to 205
const hasReadmeinPackages = fs.existsSync(
path.resolve(__dirname, '../../../packages', packageName, 'README.md')
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does this state mean to you? Also, when would a README.md not exist?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be simplified now with isComponent value. This was experimental to check inside the package directory. I found isComponent already does this.

@@ -15,6 +15,9 @@ layout: layout.njk
{% for part in collections[componentName] %}
<sp-tab-panel class="section" value="{{ part.data.partType }}">
{{ part.templateContent | safe }}
{% if hasReadmePackagesTemplate %}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will place this content at the bottom of the Examples AND the API Table which is what is causing duplicate IDs in the same DOM tree and causing the Tooltips not to open in the Examples tab.

Copy link
Contributor Author

@Rajdeepc Rajdeepc Mar 11, 2024

Choose a reason for hiding this comment

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

The logic has been changed to add inside the for loop within the tab panel

{% for part in collections[componentName] %}
        <sp-tab-panel class="section" value="{{ part.data.partType }}">
            {{ part.templateContent | safe }}
            {% if isComponentTemplate and part.data.partType === 'examples' %}
              {% include "partials/accessibility.njk" %}
            {% endif %}
        </sp-tab-panel>
      {% endfor %}
      

<sp-table-cell><sp-link href="https://opensource.adobe.com/spectrum-web-components/components/{{componentName}}/#usage">{{componentName}}</sp-link></sp-table-cell>
<sp-table-cell>
<span id="defaulttest">Default state</span>
<sp-overlay trigger="defaulttest@hover" placement="bottom">
Copy link
Collaborator

Choose a reason for hiding this comment

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

We're still working on #3596 but, the accessibility here with a screen reader should be more like https://enhancement-docs-accessibility--spectrum-web-components.netlify.app/components/overlay/#example than it is now. I'm not sure if that's the lack of tab indexes on the triggers, or the duplicate IDs, or type="auto" when it should be type="hint", or something else, but please take a closer look to ensure we've at least prepared this content to the best of the capabilities of the elements we're using.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked and the accessibility is not there for badge component which in turn renders a label. So I have updated each element inside this table to render as a button to leverage the accessibility implementations of a button and tested it in screen reader. Please note this is a screen reader test. The UI has been updated accordingly.

Screenshot 2024-03-11 at 3 58 36 PM

@@ -19,6 +19,9 @@ layout: layout.njk
{% endfor %}
</sp-tabs>
</main>
{% if isComponentTemplate %}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Slightly better positioning, but it still displays on both tabs, is that intended? It's also outside of the <main> element which means it's in a weird part of the document, semantically.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has been changed to a more controlled logic.

Comment on lines 20 to 23
<sp-badge variant="positive" id="trigger-1">
Tested
</sp-badge>
<sp-overlay trigger="trigger-1@hover" placement="bottom">
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm seeing this work with a pointing device, but still not seeing this relationship managed in a screen reader.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True since this is being triggered from an unaccessible component i.e badge which needs to be set an aria-label

@Rajdeepc Rajdeepc requested a review from Westbrook March 11, 2024 11:17
@Rajdeepc Rajdeepc marked this pull request as draft March 11, 2024 12:14
@Westbrook
Copy link
Collaborator

Don't forget to check this out in dark mode:
image

Also, does the content with Tooltips need to be Buttons?

Comment on lines +15 to +17
<sp-table-cell>
<sp-button static="white" style="--mod-bold-font-weight:400">Default state</sp-button>
</sp-table-cell>
Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't use a button for static cell content. Also, static="white" is meant to render a button over a dark colored background, so that the border and focus ring will be white. Using it in this case makes the border and focus ring render as white on white, which makes it impossible to track focus.

Suggested change
<sp-table-cell>
<sp-button static="white" style="--mod-bold-font-weight:400">Default state</sp-button>
</sp-table-cell>
<sp-table-cell>
Default state
</sp-table-cell>

Comment on lines +5 to +84
<sp-table quiet style="--mod-table-row-background-color-hover: none">
<sp-table-head>
<sp-table-head-cell>Component</sp-table-head-cell>
<sp-table-head-cell>Accessibility test</sp-table-head-cell>
<sp-table-head-cell>Accessibility Contract</sp-table-head-cell>
<sp-table-head-cell>Status</sp-table-head-cell>
</sp-table-head>
<sp-table-body>
<sp-table-row value="row1">
<sp-table-cell><sp-link href="https://opensource.adobe.com/spectrum-web-components/components/{{componentName}}/#usage">{{componentName}}</sp-link></sp-table-cell>
<sp-table-cell>
<sp-button static="white" style="--mod-bold-font-weight:400">Default state</sp-button>
</sp-table-cell>
<sp-table-cell>
<sp-button static="white" style="--mod-bold-font-weight:400">Test(s) that ensure the initial render state of a component is accessible.</sp-button></sp-table-cell>
<sp-table-cell>
<sp-button id="trigger-1" variant="secondary" style="--mod-button-background-color-default: #a7f0ba;">Tested</sp-button>
<sp-overlay trigger="trigger-1@hover" placement="bottom">
<sp-tooltip>
Passes all automated tests with no reported accessibility violations.
</sp-tooltip>
</sp-overlay>
</sp-table-cell>
</sp-table-row>
<sp-table-row value="row2">
<sp-table-cell></sp-table-cell>
<sp-table-cell>
<sp-button static="white" style="--mod-bold-font-weight:400">Advanced states</sp-button>
</sp-table-cell>
<sp-table-cell>
<sp-button static="white" style="--mod-bold-font-weight:400">Tests that ensure additional states of the component are accessible.
This could be interactive states of a component or its multiple variants.</sp-button>
</sp-table-cell>
<sp-table-cell>
<sp-button id="trigger-2" variant="secondary" style="--mod-button-background-color-default: #a7f0ba;">Tested</sp-button>
<sp-overlay trigger="trigger-2@hover" placement="bottom">
<sp-tooltip>
Passes all automated tests with no reported accessibility violations.
</sp-tooltip>
</sp-overlay>
</sp-table-cell>
</sp-table-row>
<sp-table-row value="row2">
<sp-table-cell></sp-table-cell>
<sp-table-cell>
<sp-button static="white" style="--mod-bold-font-weight:400">Keyboard navigation</sp-button>
</sp-table-cell>
<sp-table-cell>
<sp-button static="white" style="--mod-bold-font-weight:400">Tests that ensure focus is properly managed, and all interactive functions of a
component have a proper keyboard-accessible equivalent.</sp-button>
</sp-table-cell>
<sp-table-cell>
<sp-button id="trigger-3" variant="secondary" style="--mod-button-background-color-default: #a7f0ba;">Tested</sp-button>
<sp-overlay trigger="trigger-3@hover" placement="bottom">
<sp-tooltip>
Passes all automated tests with no reported accessibility violations.
</sp-tooltip>
</sp-overlay>
</sp-table-cell>
</sp-table-row>
<sp-table-row value="row3">
<sp-table-cell></sp-table-cell>
<sp-table-cell>
<sp-button static="white" style="--mod-bold-font-weight:400">Screen reader</sp-button>
</sp-table-cell>
<sp-table-cell>
<sp-button static="white" style="--mod-bold-font-weight:400">This manual testing ensures that the visual information on the screen
is properly conveyed and read correctly by screen readers such as JAWS, VoiceOver, and NVDA.</sp-button>
</sp-table-cell>
<sp-table-cell>
<sp-button id="trigger-4" variant="secondary" style="--mod-button-background-color-default: #9ef0f0;">Manually Tested</sp-button>
<sp-overlay trigger="trigger-4@hover" placement="bottom">
<sp-tooltip>
A human has manually tested this component, e.g. screen reader testing.
</sp-tooltip>
</sp-overlay>
</sp-table-cell>
</sp-table-row>
</sp-table-body>
</sp-table>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<sp-table quiet style="--mod-table-row-background-color-hover: none">
<sp-table-head>
<sp-table-head-cell>Component</sp-table-head-cell>
<sp-table-head-cell>Accessibility test</sp-table-head-cell>
<sp-table-head-cell>Accessibility Contract</sp-table-head-cell>
<sp-table-head-cell>Status</sp-table-head-cell>
</sp-table-head>
<sp-table-body>
<sp-table-row value="row1">
<sp-table-cell><sp-link href="https://opensource.adobe.com/spectrum-web-components/components/{{componentName}}/#usage">{{componentName}}</sp-link></sp-table-cell>
<sp-table-cell>
<sp-button static="white" style="--mod-bold-font-weight:400">Default state</sp-button>
</sp-table-cell>
<sp-table-cell>
<sp-button static="white" style="--mod-bold-font-weight:400">Test(s) that ensure the initial render state of a component is accessible.</sp-button></sp-table-cell>
<sp-table-cell>
<sp-button id="trigger-1" variant="secondary" style="--mod-button-background-color-default: #a7f0ba;">Tested</sp-button>
<sp-overlay trigger="trigger-1@hover" placement="bottom">
<sp-tooltip>
Passes all automated tests with no reported accessibility violations.
</sp-tooltip>
</sp-overlay>
</sp-table-cell>
</sp-table-row>
<sp-table-row value="row2">
<sp-table-cell></sp-table-cell>
<sp-table-cell>
<sp-button static="white" style="--mod-bold-font-weight:400">Advanced states</sp-button>
</sp-table-cell>
<sp-table-cell>
<sp-button static="white" style="--mod-bold-font-weight:400">Tests that ensure additional states of the component are accessible.
This could be interactive states of a component or its multiple variants.</sp-button>
</sp-table-cell>
<sp-table-cell>
<sp-button id="trigger-2" variant="secondary" style="--mod-button-background-color-default: #a7f0ba;">Tested</sp-button>
<sp-overlay trigger="trigger-2@hover" placement="bottom">
<sp-tooltip>
Passes all automated tests with no reported accessibility violations.
</sp-tooltip>
</sp-overlay>
</sp-table-cell>
</sp-table-row>
<sp-table-row value="row2">
<sp-table-cell></sp-table-cell>
<sp-table-cell>
<sp-button static="white" style="--mod-bold-font-weight:400">Keyboard navigation</sp-button>
</sp-table-cell>
<sp-table-cell>
<sp-button static="white" style="--mod-bold-font-weight:400">Tests that ensure focus is properly managed, and all interactive functions of a
component have a proper keyboard-accessible equivalent.</sp-button>
</sp-table-cell>
<sp-table-cell>
<sp-button id="trigger-3" variant="secondary" style="--mod-button-background-color-default: #a7f0ba;">Tested</sp-button>
<sp-overlay trigger="trigger-3@hover" placement="bottom">
<sp-tooltip>
Passes all automated tests with no reported accessibility violations.
</sp-tooltip>
</sp-overlay>
</sp-table-cell>
</sp-table-row>
<sp-table-row value="row3">
<sp-table-cell></sp-table-cell>
<sp-table-cell>
<sp-button static="white" style="--mod-bold-font-weight:400">Screen reader</sp-button>
</sp-table-cell>
<sp-table-cell>
<sp-button static="white" style="--mod-bold-font-weight:400">This manual testing ensures that the visual information on the screen
is properly conveyed and read correctly by screen readers such as JAWS, VoiceOver, and NVDA.</sp-button>
</sp-table-cell>
<sp-table-cell>
<sp-button id="trigger-4" variant="secondary" style="--mod-button-background-color-default: #9ef0f0;">Manually Tested</sp-button>
<sp-overlay trigger="trigger-4@hover" placement="bottom">
<sp-tooltip>
A human has manually tested this component, e.g. screen reader testing.
</sp-tooltip>
</sp-overlay>
</sp-table-cell>
</sp-table-row>
</sp-table-body>
</sp-table>
<sp-table role="table" quiet style="--mod-table-row-background-color-hover: none">
<sp-table-head>
<sp-table-head-cell>Component</sp-table-head-cell>
<sp-table-head-cell>Accessibility test</sp-table-head-cell>
<sp-table-head-cell>Accessibility Contract</sp-table-head-cell>
<sp-table-head-cell>Status</sp-table-head-cell>
</sp-table-head>
<sp-table-body>
<sp-table-row value="row1">
<sp-table-cell role="cell"><sp-link href="https://opensource.adobe.com/spectrum-web-components/components/{{componentName}}/#usage">{{componentName}}</sp-link></sp-table-cell>
<sp-table-cell role="rowheader">
Default state
</sp-table-cell>
<sp-table-cell role="cell">
Test(s) that ensure the initial render state of a component is accessible.
</sp-table-cell>
<sp-table-cell role="cell">
<sp-button id="trigger-1" variant="secondary" style="--mod-button-background-color-default: #a7f0ba;">Tested</sp-button>
<sp-overlay trigger="trigger-1@hover" placement="bottom">
<sp-tooltip>
Passes all automated tests with no reported accessibility violations.
</sp-tooltip>
</sp-overlay>
</sp-table-cell>
</sp-table-row>
<sp-table-row value="row2">
<sp-table-cell role="cell"></sp-table-cell>
<sp-table-cell role="rowheader">
Advanced states
</sp-table-cell>
<sp-table-cell role="cell">
Tests that ensure additional states of the component are accessible.
This could be interactive states of a component or its multiple variants.
</sp-table-cell>
<sp-table-cell role="cell">
<sp-button id="trigger-2" variant="secondary" style="--mod-button-background-color-default: #a7f0ba;">Tested</sp-button>
<sp-overlay trigger="trigger-2@hover" placement="bottom">
<sp-tooltip>
Passes all automated tests with no reported accessibility violations.
</sp-tooltip>
</sp-overlay>
</sp-table-cell>
</sp-table-row>
<sp-table-row value="row2">
<sp-table-cell role="cell"></sp-table-cell>
<sp-table-cell role="rowheader">
Keyboard navigation
</sp-table-cell>
<sp-table-cell role="cell">
Tests that ensure focus is properly managed, and all interactive functions of a
component have a proper keyboard-accessible equivalent.
</sp-table-cell>
<sp-table-cell role="cell">
<sp-button id="trigger-3" variant="secondary" style="--mod-button-background-color-default: #a7f0ba;">Tested</sp-button>
<sp-overlay trigger="trigger-3@hover" placement="bottom">
<sp-tooltip>
Passes all automated tests with no reported accessibility violations.
</sp-tooltip>
</sp-overlay>
</sp-table-cell>
</sp-table-row>
<sp-table-row value="row3">
<sp-table-cell role="cell"></sp-table-cell>
<sp-table-cell role="rowheader">
Screen reader
</sp-table-cell>
<sp-table-cell role="cell">
This manual testing ensures that the visual information on the screen
is properly conveyed and read correctly by screen readers such as JAWS, VoiceOver, and NVDA.
</sp-table-cell>
<sp-table-cell role="cell">
<sp-button id="trigger-4" variant="secondary" style="--mod-button-background-color-default: #9ef0f0;">Manually Tested</sp-button>
<sp-overlay trigger="trigger-4@hover" placement="bottom">
<sp-tooltip>
A human has manually tested this component, e.g. screen reader testing.
</sp-tooltip>
</sp-overlay>
</sp-table-cell>
</sp-table-row>
</sp-table-body>
</sp-table>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a11y Issues related to accessibility Documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants