-
Notifications
You must be signed in to change notification settings - Fork 193
[tests-only] [full-ci] test: [OCISDEV-417] add a11y playwright tests #13268
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
base: master
Are you sure you want to change the base?
Conversation
e5f3541 to
1aae2c5
Compare
Setup new Playwright a11y tests and add a11y reporter. Accessibility checks are added into existing E2E tests. Docs are added to explain the new accessibility tests.
1aae2c5 to
252e568
Compare
|
|
@mzner please review the vue changes. |
| v-bind="componentProps" | ||
| :class="[action.class, 'action-menu-item', 'oc-py-s', 'oc-px-m', 'oc-width-1-1']" | ||
| :aria-label="componentProps.disabled ? action.disabledTooltip?.(actionOptions) : ''" | ||
| :aria-label="componentProps.disabled ? action.disabledTooltip?.(actionOptions) : null" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we be using a label instead of null?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need the label to be gone if the button is not disabled and with '' it is unfortunately persisted on the element (even though with no value). Using null makes sure that the label is gone.
| <template> | ||
| <li v-oc-tooltip="componentProps.disabled ? action.disabledTooltip?.(actionOptions) : ''"> | ||
| <oc-button | ||
| v-oc-tooltip="showTooltip || action.hideLabel ? action.label(actionOptions) : ''" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a difference between this and :title?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tooltip is not available to screen readers.
| this.outputFile = options.outputFile || 'a11y-report.json' | ||
| } | ||
|
|
||
| onBegin(_config: FullConfig, _suite: Suite): void { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not that important, but what do you think about using onStart instead of onBegin?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We cannot use arbitrary name here because these methods are overwriting the ones from the reporter class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.



Description
Setup new Playwright a11y tests and add a11y reporter. Accessibility checks are added into existing E2E tests. Docs are added to explain the new accessibility tests.
Motivation and Context
Migrating tests from cucumber to playwright.
How Has This Been Tested?
Types of changes