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.

Sign up for GitHub

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

Jump to bottom

Bug 1531243 - Fix some a11y issues with buttons and labels #4731

Merged
merged 3 commits into from
Mar 6, 2019
Merged

Bug 1531243 - Fix some a11y issues with buttons and labels #4731

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
504356c
Bug 1531243 - Fix a11y issues with aria-label and button roles
Mar 1, 2019
2486901
Bug 1531243 - Fix ESLint rule jsx-a11y/anchor-is-valid
Mar 1, 2019
a7a18ba
Bug 1531243 - Fix ESLint jsx--a11y/label-has-associated-control
Mar 1, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,7 @@ module.exports = {
'class-methods-use-this': 'off',
'consistent-return': 'off',
'default-case': 'off',
'jsx-a11y/anchor-is-valid': 'off',
'jsx-a11y/click-events-have-key-events': 'off',
'jsx-a11y/label-has-associated-control': 'off',
'jsx-a11y/label-has-for': 'off',
'jsx-a11y/no-noninteractive-element-interactions': 'off',
'jsx-a11y/no-static-element-interactions': 'off',
Expand Down
2 changes: 1 addition & 1 deletion tests/selenium/pages/treeherder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ def open_log_viewer(self, method='pointer'):
class Pinboard(Region):

_root_locator = (By.ID, 'pinboard-panel')
_clear_all_locator = (By.CSS_SELECTOR, '#pinboard-controls .dropdown-menu li:nth-child(3) a')
_clear_all_locator = (By.CSS_SELECTOR, '#pinboard-controls .dropdown-menu li:nth-child(3) button')
_jobs_locator = (By.CLASS_NAME, 'pinned-job')
_save_menu_locator = (By.CSS_SELECTOR, '#pinboard-controls .save-btn-dropdown')

Expand Down
4 changes: 0 additions & 4 deletions ui/css/treeherder-test-view.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,6 @@ main .progress {
color: white;
}

.toggle-count {
cursor: pointer;
}

[disabled]:hover {
cursor: not-allowed;
}
Expand Down
4 changes: 4 additions & 0 deletions ui/css/treeherder.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@
height: 100%;
}

.dropdown-item.disabled {
cursor: not-allowed;
font-style: italic;
}
/*
* Resizer between PushList and DetailsPanel
*/
Expand Down
23 changes: 19 additions & 4 deletions ui/job-view/CustomJobActions.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,14 @@ import jsonSchemaDefaults from 'json-schema-defaults';
// https://github.com/nodeca/js-yaml/pull/462
import jsyaml from 'js-yaml/dist/js-yaml';
import { slugid } from 'taskcluster-client-web';
import { Button, Modal, ModalHeader, ModalBody, ModalFooter } from 'reactstrap';
import {
Button,
Label,
Modal,
ModalHeader,
ModalBody,
ModalFooter,
} from 'reactstrap';
import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
import { faCheckSquare } from '@fortawesome/free-regular-svg-icons';

Expand Down Expand Up @@ -200,12 +207,14 @@ class CustomJobActions extends React.PureComponent {
{!!actions && (
<div>
<div className="form-group">
<label>Action</label>
<Label for="action-select-input">Action</Label>
<Select
Copy link

Choose a reason for hiding this comment

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

AFAIK, react-select is not just a standard HTML <select>, but would instead use custom markup. <label forr=...> is only supported by HTML form elements, so I'm not sure if this will work. If you haven't already, you could double check this with the Accessibility inspector in the Firefox Dev Tools. For example, you could right click the control, select Inspect Accessibility Properties and check the name property for that element.

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 made some modifications to this that may help. Though it looks like react-select just doesn't have good ARIA support. I may need to switch to another select type component to get this fixed.

JedWatson/react-select#2456

Copy link

Choose a reason for hiding this comment

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

Okay. I can't tell whether this is going to work without being able to see it rendered. I guess we can handle this in a follow up if this doesn't work.

Copy link
Contributor

Choose a reason for hiding this comment

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

Though it looks like react-select just doesn't have good ARIA support. I may need to switch to another select type component to get this fixed.

Removing use of react-select would also fix a few other issues (see bug 1507903).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @edmorley ! :) Thanks for the context. Yeah, I think I will replace it in a follow-up PR.

inputId="action-select"
aria-describedby="selectedActionHelp"
value={selectedActionOption}
onChange={this.onChangeAction}
options={actionOptions}
name="Action"
/>
<p id="selectedActionHelp" className="help-block">
{selectedAction.description}
Expand All @@ -223,8 +232,11 @@ class CustomJobActions extends React.PureComponent {
{!!selectedAction.schema && (
<React.Fragment>
<div className="col-s-12 col-md-6 form-group">
<label>Payload</label>
<Label for="payload-textarea" className="w-100">
Payload
</Label>
<textarea
id="payload-textarea"
value={payload}
className="form-control pre"
rows="10"
Expand All @@ -233,8 +245,11 @@ class CustomJobActions extends React.PureComponent {
/>
</div>
<div className="col-s-12 col-md-6 form-group">
<label>Schema</label>
<Label for="schema-textarea" className="w-100">
Schema
</Label>
<textarea
id="schema-textarea"
className="form-control pre"
rows="10"
readOnly
Expand Down
24 changes: 13 additions & 11 deletions ui/job-view/details/BugFiler.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,7 @@ export class BugFilerClass extends React.Component {
))}
</FormGroup>
</div>
<label>Summary:</label>
<Label for="summary">Summary:</Label>
<div className="d-flex">
{!!unhelpfulSummaryReason && (
<div>
Expand Down Expand Up @@ -667,7 +667,7 @@ export class BugFilerClass extends React.Component {
)}
<div className="ml-5 mt-2">
<div>
<label>
<Label>
<Input
type="checkbox"
checked={checkedLogLinks.includes(parsedLog)}
Expand All @@ -680,10 +680,10 @@ export class BugFilerClass extends React.Component {
>
Include Parsed Log Link
</a>
</label>
</Label>
</div>
<div>
<label>
<Label>
<Input
type="checkbox"
checked={checkedLogLinks.includes(fullLog)}
Expand All @@ -692,11 +692,11 @@ export class BugFilerClass extends React.Component {
<a target="_blank" rel="noopener noreferrer" href={fullLog}>
Include Full Log Link
</a>
</label>
</Label>
</div>
{!!reftestUrl && (
<div>
<label>
<Label>
<Input
type="checkbox"
checked={checkedLogLinks.includes(reftestUrl)}
Expand All @@ -709,22 +709,23 @@ export class BugFilerClass extends React.Component {
>
Include Reftest Viewer Link
</a>
</label>
</Label>
</div>
)}
</div>
<div className="d-flex flex-column">
<label>Comment:</label>
<Label for="summary-input">Comment:</Label>
<Input
onChange={evt => this.setState({ comment: evt.target.value })}
type="textarea"
id="summary-input"
className="flex-grow-1"
rows={5}
/>
</div>
<div className="d-inline-flex mt-2 ml-5">
<div className="mt-2">
<label>
<Label>
<Input
onChange={() =>
this.setState({ isIntermittent: !isIntermittent })
Expand All @@ -733,7 +734,7 @@ export class BugFilerClass extends React.Component {
checked={isIntermittent}
/>
This is an intermittent failure
</label>
</Label>
</div>
<div className="d-inline-flex ml-2">
<Input
Expand Down Expand Up @@ -790,9 +791,10 @@ export class BugFilerClass extends React.Component {
</div>
{!!crashSignatures.length && (
<div>
<label>Signature:</label>
<Label for="signature-input">Signature:</Label>
<Input
type="textarea"
id="signature-input"
onChange={evt =>
this.setState({ crashSignatures: evt.target.value })
}
Expand Down
41 changes: 21 additions & 20 deletions ui/job-view/details/PinBoard.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react';
import PropTypes from 'prop-types';
import { FormGroup, Input, FormFeedback } from 'reactstrap';
import { Button, FormGroup, Input, FormFeedback } from 'reactstrap';
import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
import { faPlusSquare, faTimes } from '@fortawesome/free-solid-svg-icons';

Expand Down Expand Up @@ -553,19 +553,19 @@ class PinBoard extends React.Component {
title={this.hasPinnedJobs() ? '' : 'No pinned jobs'}
>
<div className="btn-group save-btn-group dropdown">
<button
<Button
className={`btn btn-light-bordered btn-xs save-btn ${
!isLoggedIn || !this.canSaveClassifications()
? 'disabled'
: ''
}`}
type="button"
outline
title={this.saveUITitle('classification')}
onClick={this.save}
>
save
</button>
<button
</Button>
<Button
className={`btn btn-light-bordered btn-xs dropdown-toggle save-btn-dropdown ${
!this.hasPinnedJobs() && !this.pinboardIsDirty()
? 'disabled'
Expand All @@ -576,43 +576,44 @@ class PinBoard extends React.Component {
? 'No pinned jobs'
: 'Additional pinboard functions'
}
type="button"
outline
data-toggle="dropdown"
>
<span className="caret" />
</button>
</Button>
<ul className="dropdown-menu save-btn-dropdown-menu">
<li
className={!isLoggedIn ? 'disabled' : ''}
title={
!isLoggedIn ? 'Not logged in' : 'Repeat the pinned jobs'
}
>
<a
className="dropdown-item"
<Button
className={`${!isLoggedIn ? 'disabled' : ''} dropdown-item`}
onClick={() => !isLoggedIn || this.retriggerAllPinnedJobs()}
>
Retrigger all
</a>
</Button>
</li>
<li
className={this.canCancelAllPinnedJobs() ? '' : 'disabled'}
title={this.cancelAllPinnedJobsTitle()}
>
<a
className="dropdown-item"
<li title={this.cancelAllPinnedJobsTitle()}>
<Button
className={`${
this.canCancelAllPinnedJobs() ? '' : 'disabled'
} dropdown-item`}
onClick={() =>
this.canCancelAllPinnedJobs() &&
this.cancelAllPinnedJobs()
}
>
Cancel all
</a>
</Button>
</li>
<li>
<a className="dropdown-item" onClick={() => this.unPinAll()}>
<Button
className="dropdown-item"
onClick={() => this.unPinAll()}
>
Clear all
</a>
</Button>
</li>
</ul>
</div>
Expand Down
35 changes: 18 additions & 17 deletions ui/job-view/details/summary/ActionBar.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';
import PropTypes from 'prop-types';
import { Button } from 'reactstrap';
import $ from 'jquery';
import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
import { faChartBar } from '@fortawesome/free-regular-svg-icons';
Expand Down Expand Up @@ -315,17 +316,17 @@ class ActionBar extends React.PureComponent {
logViewerFullUrl={logViewerFullUrl}
/>
<li>
<span
<Button
id="pin-job-btn"
title="Add this job to the pinboard"
className="btn icon-blue"
onClick={() => pinJob(selectedJob)}
>
<FontAwesomeIcon icon={faThumbtack} />
</span>
</Button>
</li>
<li>
<span
<Button
id="retrigger-btn"
title={
user.isLoggedIn
Expand All @@ -337,7 +338,7 @@ class ActionBar extends React.PureComponent {
onClick={() => this.retriggerJob([selectedJob])}
>
<FontAwesomeIcon icon={faRedo} />
</span>
</Button>
</li>
{isReftest(selectedJob) &&
jobLogUrls.map(jobLogUrl => (
Expand All @@ -354,7 +355,7 @@ class ActionBar extends React.PureComponent {
))}
{this.canCancel() && (
<li>
<a
<Button
title={
user.isLoggedIn
? 'Cancel this job'
Expand All @@ -364,22 +365,22 @@ class ActionBar extends React.PureComponent {
onClick={() => this.cancelJob()}
>
<FontAwesomeIcon icon={faTimesCircle} />
</a>
</Button>
</li>
)}
</ul>
<ul className="nav navbar-right">
<li className="dropdown">
<span
<Button
id="actionbar-menu-btn"
title="Other job actions"
aria-haspopup="true"
aria-expanded="false"
className="dropdown-toggle"
className="btn btn-sm dropdown-toggle bg-transparent border-0 pr-2 py-0 m-0"
data-toggle="dropdown"
>
<FontAwesomeIcon icon={faEllipsisH} aria-hidden="true" />
</span>
</Button>
<ul className="dropdown-menu actionbar-menu" role="menu">
<li>
<span
Expand Down Expand Up @@ -408,30 +409,30 @@ class ActionBar extends React.PureComponent {
</a>
</li>
<li>
<a
className="dropdown-item"
<Button
className="dropdown-item py-2"
onClick={this.createInteractiveTask}
>
Create Interactive Task
</a>
</Button>
</li>
{isPerfTest(selectedJob) && (
<li>
<a
className="dropdown-item"
<Button
className="dropdown-item py-2"
onClick={this.createGeckoProfile}
>
Create Gecko Profile
</a>
</Button>
</li>
)}
<li>
<a
<Button
onClick={this.toggleCustomJobActions}
className="dropdown-item"
>
Custom Action...
</a>
</Button>
</li>
</React.Fragment>
)}
Expand Down
Loading