Merge pull request #9875 from keithjgrant/a11y-fixes

Accessibility fixes

SUMMARY
Fixes numerous accessibility issues, including:

updates CodeEditor so label correctly points at associated textarea
fixes issues with tabs on dashboard and details pages
adds missings ids
adds alt text to logo
removes duplicate ids on some lists

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

UI

Reviewed-by: Kersom <None>
Reviewed-by: Keith Grant <keithjgrant@gmail.com>
Reviewed-by: Jake McDermott <yo@jakemcdermott.me>
Reviewed-by: Tiago Góes <tiago.goes2009@gmail.com>
This commit is contained in:
softwarefactory-project-zuul[bot] 2021-04-15 17:39:07 +00:00 committed by GitHub
commit 32200cd893
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
31 changed files with 174 additions and 140 deletions

View File

@ -60,6 +60,7 @@
"mode",
"aria-labelledby",
"aria-hidden",
"aria-controls",
"sortKey",
"ouiaId",
"credentialTypeNamespace",

View File

@ -138,10 +138,13 @@ function AppContainer({ i18n, navRouteConfig = [], children }) {
}
}, [handleLogout, timeRemaining]);
const brandName = config?.license_info?.product_name;
const alt = brandName ? i18n._(t`${brandName} logo`) : i18n._(t`brand logo`);
const header = (
<PageHeader
showNavToggle
logo={<BrandLogo />}
logo={<BrandLogo alt={alt} />}
logoProps={{ href: '/' }}
headerTools={
<PageHeaderToolbar
@ -156,7 +159,7 @@ function AppContainer({ i18n, navRouteConfig = [], children }) {
const simpleHeader = config.isLoading ? null : (
<PageHeader
logo={<BrandLogo />}
logo={<BrandLogo alt={alt} />}
headerTools={
<PageHeaderTools>
<PageHeaderToolsGroup>

View File

@ -13,6 +13,8 @@ const BrandImg = styled.img`
pointer-events: none;
`;
const BrandLogo = () => <BrandImg src="/static/media/logo-header.svg" />;
const BrandLogo = ({ alt }) => (
<BrandImg src="/static/media/logo-header.svg" alt={alt} />
);
export default BrandLogo;

View File

@ -96,11 +96,15 @@ function CodeEditor({
useEffect(
function removeTextareaTabIndex() {
const editorInput = editor.current.refEditor?.querySelector('textarea');
if (editorInput && !readOnly) {
if (!editorInput) {
return;
}
if (!readOnly) {
editorInput.tabIndex = -1;
}
editorInput.id = id;
},
[readOnly]
[readOnly, id]
);
const listen = useCallback(event => {
@ -144,7 +148,7 @@ function CodeEditor({
value={value}
onFocus={onFocus}
onBlur={onBlur}
name={id || 'code-editor'}
name={`${id}-editor` || 'code-editor'}
editorProps={{ $blockScrolling: true }}
fontSize={16}
width="100%"

View File

@ -73,6 +73,7 @@ function VariablesDetail({
css="grid-column: 1 / -1"
>
<ModeToggle
id={`${dataCy}-preview`}
label={label}
helpText={helpText}
dataCy={dataCy}
@ -90,6 +91,7 @@ function VariablesDetail({
css="grid-column: 1 / -1; margin-top: -20px"
>
<CodeEditor
id={`${dataCy}-preview`}
mode={mode}
value={currentValue}
readOnly
@ -125,6 +127,7 @@ function VariablesDetail({
>
<div className="pf-c-form">
<ModeToggle
id={`${dataCy}-preview-expanded`}
label={label}
helpText={helpText}
dataCy={dataCy}
@ -134,6 +137,7 @@ function VariablesDetail({
i18n={i18n}
/>
<CodeEditor
id={`${dataCy}-preview-expanded`}
mode={mode}
value={currentValue}
readOnly
@ -160,6 +164,7 @@ VariablesDetail.defaultProps = {
};
function ModeToggle({
id,
label,
helpText,
dataCy,
@ -173,7 +178,7 @@ function ModeToggle({
<SplitItem isFilled>
<Split hasGutter css="align-items: baseline">
<SplitItem>
<div className="pf-c-form__label">
<label className="pf-c-form__label" htmlFor={id}>
<span
className="pf-c-form__label-text"
css="font-weight: var(--pf-global--FontWeight--bold)"
@ -183,7 +188,7 @@ function ModeToggle({
{helpText && (
<Popover header={label} content={helpText} id={dataCy} />
)}
</div>
</label>
</SplitItem>
<SplitItem>
<MultiButtonToggle

View File

@ -42,12 +42,12 @@ describe('<VariablesDetail>', () => {
expect(input2.prop('value')).toEqual('---foo: bar');
});
test('should render label and value= --- when there are no values', () => {
test('should render label and value --- when there are no values', () => {
const wrapper = mountWithContexts(
<VariablesDetail value="" label="Variables" />
);
expect(wrapper.find('VariablesDetail___StyledCodeEditor').length).toBe(1);
expect(wrapper.find('div.pf-c-form__label').text()).toBe('Variables');
expect(wrapper.find('.pf-c-form__label').text()).toBe('Variables');
});
test('should update value if prop changes', () => {

View File

@ -203,7 +203,7 @@ function VariablesFieldInternals({
<label htmlFor={id} className="pf-c-form__label">
<span className="pf-c-form__label-text">{label}</span>
</label>
{tooltip && <Popover content={tooltip} id={id} />}
{tooltip && <Popover content={tooltip} id={`${id}-tooltip`} />}
</SplitItem>
<SplitItem>
<MultiButtonToggle
@ -235,6 +235,7 @@ function VariablesFieldInternals({
)}
</FieldHeader>
<CodeEditor
id={id}
mode={mode}
readOnly={readOnly}
{...field}

View File

@ -169,7 +169,7 @@ describe('VariablesField', () => {
)}
</Formik>
);
expect(wrapper.find('Popover[data-cy="the-field"]').length).toBe(1);
expect(wrapper.find('Popover[data-cy="the-field-tooltip"]').length).toBe(1);
});
it('should submit value through Formik', async () => {

View File

@ -39,7 +39,7 @@ function FieldWithPrompt({
</span>
)}
</label>
{tooltip && <Popover content={tooltip} id={fieldId} />}
{tooltip && <Popover content={tooltip} id={`${fieldId}-tooltip`} />}
</div>
<StyledCheckboxField
isDisabled={isDisabled}

View File

@ -61,8 +61,8 @@ describe('FieldWithPrompt', () => {
</Formik>
);
expect(wrapper.find('.pf-c-form__label-required')).toHaveLength(1);
expect(wrapper.find('Popover[data-cy="job-template-limit"]').length).toBe(
1
);
expect(
wrapper.find('Popover[data-cy="job-template-limit-tooltip"]').length
).toBe(1);
});
});

View File

@ -1,5 +1,6 @@
import React, { useState } from 'react';
import { func, string } from 'prop-types';
import { t } from '@lingui/macro';
import { Select, SelectOption, SelectVariant } from '@patternfly/react-core';
import { arrayToString, stringToArray } from '../../util/strings';
@ -51,7 +52,7 @@ function TagMultiSelect({ onChange, value }) {
}}
selections={selections}
isOpen={isExpanded}
aria-labelledby="tag-select"
typeAheadAriaLabel={t`Select tags`}
>
{renderOptions(options)}
</Select>

View File

@ -123,6 +123,9 @@ function PaginatedTable({
}
onSetPage={handleSetPage}
onPerPageSelect={handleSetPageSize}
titles={{
paginationTitle: t`Top Pagination`,
}}
/>
);

View File

@ -3,8 +3,7 @@ import { shape, string, number, arrayOf, node, oneOfType } from 'prop-types';
import { Tab, Tabs, TabTitleText } from '@patternfly/react-core';
import { useHistory, useLocation } from 'react-router-dom';
function RoutedTabs(props) {
const { tabsArray } = props;
function RoutedTabs({ tabsArray }) {
const history = useHistory();
const location = useLocation();
@ -33,12 +32,12 @@ function RoutedTabs(props) {
<Tabs activeKey={getActiveTabId()} onSelect={handleTabSelect}>
{tabsArray.map(tab => (
<Tab
aria-label={typeof tab.name === 'string' ? tab.name : ''}
aria-label={typeof tab.name === 'string' ? tab.name : null}
eventKey={tab.id}
key={tab.id}
link={tab.link}
title={<TabTitleText>{tab.name}</TabTitleText>}
role="tab"
aria-controls=""
/>
))}
</Tabs>

View File

@ -50,13 +50,17 @@ describe('ScheduleListItem', () => {
describe('User has edit permissions', () => {
beforeAll(() => {
wrapper = mountWithContexts(
<ScheduleListItem
isSelected={false}
onSelect={onSelect}
schedule={mockSchedule}
isMissingSurvey={false}
isMissingInventory={false}
/>
<table>
<tbody>
<ScheduleListItem
isSelected={false}
onSelect={onSelect}
schedule={mockSchedule}
isMissingSurvey={false}
isMissingInventory={false}
/>
</tbody>
</table>
);
});
@ -190,22 +194,26 @@ describe('ScheduleListItem', () => {
describe('schedule has missing prompt data', () => {
beforeAll(() => {
wrapper = mountWithContexts(
<ScheduleListItem
isSelected={false}
onSelect={onSelect}
schedule={{
...mockSchedule,
summary_fields: {
...mockSchedule.summary_fields,
user_capabilities: {
edit: false,
delete: false,
},
},
}}
isMissingInventory="Inventory Error"
isMissingSurvey="Survey Error"
/>
<table>
<tbody>
<ScheduleListItem
isSelected={false}
onSelect={onSelect}
schedule={{
...mockSchedule,
summary_fields: {
...mockSchedule.summary_fields,
user_capabilities: {
edit: false,
delete: false,
},
},
}}
isMissingInventory="Inventory Error"
isMissingSurvey="Survey Error"
/>
</tbody>
</table>
);
});

View File

@ -175,6 +175,7 @@ function Search({
variant={SelectVariant.single}
className="simpleKeySelect"
aria-label={i18n._(t`Simple key select`)}
typeAheadAriaLabel={i18n._(t`Simple key select`)}
onToggle={setIsSearchDropdownOpen}
onSelect={handleDropdownSelect}
selections={searchColumnName}
@ -212,6 +213,7 @@ function Search({
<Select
variant={SelectVariant.checkbox}
aria-label={name}
typeAheadAriaLabel={name}
onToggle={setIsFilterDropdownOpen}
onSelect={(event, selection) =>
handleFilterDropdownSelect(key, event, selection)

View File

@ -126,6 +126,7 @@ function ActivityStream({ i18n }) {
maxHeight="480px"
variant={SelectVariant.single}
aria-labelledby="grouped-type-select-id"
typeAheadAriaLabel={t`Select an activity type`}
className="activityTypeSelect"
onToggle={setIsTypeDropdownOpen}
onSelect={(event, selection) => {

View File

@ -138,6 +138,7 @@ function CredentialFormFields({ i18n, initialTypeId, credentialTypes }) {
isDisabled={isCredentialTypeDisabled}
ouiaId="CredentialForm-credential_type"
aria-label={i18n._(t`Credential Type`)}
typeAheadAriaLabel={i18n._(t`Select Credential Type`)}
isOpen={isSelectOpen}
variant={SelectVariant.typeahead}
onToggle={setIsSelectOpen}
@ -203,7 +204,7 @@ function CredentialFormFields({ i18n, initialTypeId, credentialTypes }) {
{isCredentialTypeDisabled ? (
<Tooltip
content={i18n._(
`You cannot change the credential type of a credential,
`You cannot change the credential type of a credential,
as it may break the functionality of the resources using it.`
)}
>

View File

@ -43,6 +43,7 @@ function BecomeMethodField({ fieldOptions, isRequired }) {
>
<Select
ouiaId={`CredentialForm-${fieldOptions.id}`}
typeAheadAriaLabel={fieldOptions.label}
maxHeight={200}
variant={SelectVariant.typeahead}
onToggle={setIsOpen}

View File

@ -170,86 +170,94 @@ function Dashboard({ i18n }) {
aria-label={i18n._(t`Job status graph tab`)}
eventKey={0}
title={<TabTitleText>{i18n._(t`Job status`)}</TabTitleText>}
/>
>
<Fragment>
<GraphCardHeader>
<GraphCardActions>
<Select
variant={SelectVariant.single}
placeholderText={i18n._(t`Select period`)}
aria-label={i18n._(t`Select period`)}
typeAheadAriaLabel={i18n._(t`Select period`)}
className="periodSelect"
onToggle={setIsPeriodDropdownOpen}
onSelect={(event, selection) =>
setPeriodSelection(selection)
}
selections={periodSelection}
isOpen={isPeriodDropdownOpen}
>
<SelectOption key="month" value="month">
{i18n._(t`Past month`)}
</SelectOption>
<SelectOption key="two_weeks" value="two_weeks">
{i18n._(t`Past two weeks`)}
</SelectOption>
<SelectOption key="week" value="week">
{i18n._(t`Past week`)}
</SelectOption>
</Select>
<Select
variant={SelectVariant.single}
placeholderText={i18n._(t`Select job type`)}
aria-label={i18n._(t`Select job type`)}
className="jobTypeSelect"
onToggle={setIsJobTypeDropdownOpen}
onSelect={(event, selection) =>
setJobTypeSelection(selection)
}
selections={jobTypeSelection}
isOpen={isJobTypeDropdownOpen}
>
<SelectOption key="all" value="all">
{i18n._(t`All job types`)}
</SelectOption>
<SelectOption key="inv_sync" value="inv_sync">
{i18n._(t`Inventory sync`)}
</SelectOption>
<SelectOption key="scm_update" value="scm_update">
{i18n._(t`SCM update`)}
</SelectOption>
<SelectOption key="playbook_run" value="playbook_run">
{i18n._(t`Playbook run`)}
</SelectOption>
</Select>
</GraphCardActions>
</GraphCardHeader>
<CardBody>
<LineChart
height={390}
id="d3-line-chart-root"
data={jobGraphData}
/>
</CardBody>
</Fragment>
</Tab>
<Tab
aria-label={i18n._(t`Recent Jobs list tab`)}
eventKey={1}
title={<TabTitleText>{i18n._(t`Recent Jobs`)}</TabTitleText>}
/>
>
<div>
{activeTabId === 1 && (
<JobList defaultParams={{ page_size: 5 }} />
)}
</div>
</Tab>
<Tab
aria-label={i18n._(t`Recent Templates list tab`)}
eventKey={2}
title={
<TabTitleText>{i18n._(t`Recent Templates`)}</TabTitleText>
}
/>
>
<div>
{activeTabId === 2 && (
<TemplateList defaultParams={{ page_size: 5 }} />
)}
</div>
</Tab>
</Tabs>
{activeTabId === 0 && (
<Fragment>
<GraphCardHeader>
<GraphCardActions>
<Select
variant={SelectVariant.single}
placeholderText={i18n._(t`Select period`)}
aria-label={i18n._(t`Select period`)}
className="periodSelect"
onToggle={setIsPeriodDropdownOpen}
onSelect={(event, selection) =>
setPeriodSelection(selection)
}
selections={periodSelection}
isOpen={isPeriodDropdownOpen}
>
<SelectOption key="month" value="month">
{i18n._(t`Past month`)}
</SelectOption>
<SelectOption key="two_weeks" value="two_weeks">
{i18n._(t`Past two weeks`)}
</SelectOption>
<SelectOption key="week" value="week">
{i18n._(t`Past week`)}
</SelectOption>
</Select>
<Select
variant={SelectVariant.single}
placeholderText={i18n._(t`Select job type`)}
aria-label={i18n._(t`Select job type`)}
className="jobTypeSelect"
onToggle={setIsJobTypeDropdownOpen}
onSelect={(event, selection) =>
setJobTypeSelection(selection)
}
selections={jobTypeSelection}
isOpen={isJobTypeDropdownOpen}
>
<SelectOption key="all" value="all">
{i18n._(t`All job types`)}
</SelectOption>
<SelectOption key="inv_sync" value="inv_sync">
{i18n._(t`Inventory sync`)}
</SelectOption>
<SelectOption key="scm_update" value="scm_update">
{i18n._(t`SCM update`)}
</SelectOption>
<SelectOption key="playbook_run" value="playbook_run">
{i18n._(t`Playbook run`)}
</SelectOption>
</Select>
</GraphCardActions>
</GraphCardHeader>
<CardBody>
<LineChart
height={390}
id="d3-line-chart-root"
data={jobGraphData}
/>
</CardBody>
</Fragment>
)}
{activeTabId === 1 && <JobList defaultParams={{ page_size: 5 }} />}
{activeTabId === 2 && (
<TemplateList defaultParams={{ page_size: 5 }} />
)}
</Card>
</div>
</MainPageSection>

View File

@ -248,7 +248,7 @@ function LineChart({ id, data, height, i18n, pageContext }) {
.style('fill', () => colors(0))
.attr('cx', d => x(d.DATE))
.attr('cy', d => y(d.FAIL))
.attr('id', d => `success-dot-${dateFormat(d.DATE)}`)
.attr('id', d => `fail-dot-${dateFormat(d.DATE)}`)
.on('mouseover', handleMouseOver)
.on('mousemove', handleMouseMove)
.on('mouseout', handleMouseOut);

View File

@ -43,8 +43,6 @@ function ExecutionEnvironmentListItem({
setIsDisabled(false);
}, []);
const labelId = `check-action-${executionEnvironment.id}`;
return (
<Tr id={`ee-row-${executionEnvironment.id}`}>
<Td
@ -56,15 +54,13 @@ function ExecutionEnvironmentListItem({
}}
dataLabel={i18n._(t`Selected`)}
/>
<Td id={labelId} dataLabel={i18n._(t`Name`)}>
<Td id={`ee-name-${executionEnvironment.id}`} dataLabel={i18n._(t`Name`)}>
<Link to={`${detailUrl}`}>
<b>{executionEnvironment.name}</b>
</Link>
</Td>
<Td id={labelId} dataLabel={i18n._(t`Image`)}>
{executionEnvironment.image}
</Td>
<Td id={labelId} dataLabel={i18n._(t`Organization`)}>
<Td dataLabel={i18n._(t`Image`)}>{executionEnvironment.image}</Td>
<Td dataLabel={i18n._(t`Organization`)}>
{executionEnvironment.organization ? (
<Link
to={`/organizations/${executionEnvironment?.summary_fields?.organization?.id}/details`}

View File

@ -125,6 +125,7 @@ const SCMSubForm = ({ autoPopulateProject, i18n }) => {
sourcePathHelpers.setValue(value);
}}
aria-label={i18n._(t`Select source path`)}
typeAheadAriaLabel={i18n._(t`Select source path`)}
placeholder={i18n._(t`Select source path`)}
createText={i18n._(t`Set source path to`)}
isCreatable

View File

@ -98,7 +98,7 @@ function ProjectListItem({
/>
<Td id={labelId} dataLabel={i18n._(t`Name`)}>
<span>
<Link id={labelId} to={`${detailUrl}`}>
<Link to={`${detailUrl}`}>
<b>{project.name}</b>
</Link>
</span>

View File

@ -88,6 +88,7 @@ function SurveyPreviewModal({
id={`survey-preview-multipleChoice-${q.variable}`}
isDisabled
aria-label={i18n._(t`Multiple Choice`)}
typeAheadAriaLabel={i18n._(t`Multiple Choice`)}
placeholderText={q.default}
onToggle={() => {}}
/>
@ -109,6 +110,7 @@ function SurveyPreviewModal({
}
onToggle={() => {}}
aria-label={i18n._(t`Multi-Select`)}
typeAheadAriaLabel={i18n._(t`Multi-Select`)}
id={`survey-preview-multiSelect-${q.variable}`}
>
{q.choices.length > 0 &&

View File

@ -239,6 +239,7 @@ function NodeTypeStep({ i18n }) {
setIsConvergenceOpen(false);
}}
aria-label={i18n._(t`Convergence select`)}
typeAheadAriaLabel={i18n._(t`Convergence select`)}
className="convergenceSelect"
ouiaId="convergenceSelect"
>

View File

@ -1,6 +1,7 @@
import React, { useState, useEffect } from 'react';
import { func, arrayOf, number, shape, string, oneOfType } from 'prop-types';
import { Select, SelectOption, SelectVariant } from '@patternfly/react-core';
import { t } from '@lingui/macro';
import { LabelsAPI } from '../../../api';
import { useSyncedSelectValue } from '../../../components/MultiSelect';
@ -84,7 +85,7 @@ function LabelSelect({ value, placeholder, onChange, onError, createText }) {
isDisabled={isLoading}
selections={selections}
isOpen={isExpanded}
aria-labelledby="label-select"
typeAheadAriaLabel={t`Select Labels`}
placeholderText={placeholder}
createText={createText}
>

View File

@ -56,6 +56,7 @@ function PlaybookSelect({
selections={selected}
onToggle={setIsOpen}
placeholderText={i18n._(t`Select a playbook`)}
typeAheadAriaLabel={i18n._(t`Select a playbook`)}
isCreateable={false}
onSelect={(event, value) => {
setIsOpen(false);

View File

@ -159,7 +159,7 @@ function WorkflowJobTemplateForm({
)}
</>
<FieldWithPrompt
fieldId="wjft-limit"
fieldId="wfjt-limit"
label={i18n._(t`Limit`)}
promptId="template-ask-limit-on-launch"
promptName="ask_limit_on_launch"
@ -169,7 +169,7 @@ function WorkflowJobTemplateForm({
documentation for more information and examples on patterns.`)}
>
<TextInput
id="text-wfjt-limit"
id="wfjt-limit"
{...limitField}
validated={
!limitMeta.touched || !limitMeta.error ? 'default' : 'error'
@ -190,7 +190,7 @@ function WorkflowJobTemplateForm({
)}
>
<TextInput
id="text-wfjt-scm-branch"
id="wfjt-scm-branch"
{...scmField}
onChange={value => {
scmHelpers.setValue(value);

View File

@ -226,22 +226,14 @@ describe('<WorkflowJobTemplateForm/>', () => {
test('test changes in FieldWithPrompt', async () => {
await act(async () => {
wrapper.find('TextInputBase#text-wfjt-scm-branch').prop('onChange')(
'main'
);
wrapper.find('TextInputBase#text-wfjt-limit').prop('onChange')(
1234567890
);
wrapper.find('TextInputBase#wfjt-scm-branch').prop('onChange')('main');
wrapper.find('TextInputBase#wfjt-limit').prop('onChange')(1234567890);
});
wrapper.update();
expect(wrapper.find('input#text-wfjt-scm-branch').prop('value')).toEqual(
'main'
);
expect(wrapper.find('input#text-wfjt-limit').prop('value')).toEqual(
1234567890
);
expect(wrapper.find('input#wfjt-scm-branch').prop('value')).toEqual('main');
expect(wrapper.find('input#wfjt-limit').prop('value')).toEqual(1234567890);
});
test('webhooks and enable concurrent jobs functions properly', async () => {

View File

@ -43,7 +43,7 @@ function UserListItem({
}}
/>
<Td id={labelId} dataLabel={i18n._(t`Username`)}>
<Link to={`${detailUrl}`} id={labelId}>
<Link to={`${detailUrl}`}>
<b>{user.username}</b>
</Link>
{ldapUser && (