Bug 1551613 - Enable jsx-a11y eslint for TopSites component and fix errors (#5035)

This commit is contained in:
Emily McMinn 2019-05-17 17:46:42 -04:00 коммит произвёл Ed Lee
Родитель 753802549d
Коммит 2d7ccd5039
14 изменённых файлов: 101 добавлений и 36 удалений

Просмотреть файл

@ -0,0 +1,14 @@
import React from "react";
export function A11yLinkButton(props) {
// function for merging classes, if necessary
let className = "a11y-link-button";
if (props.className) {
className += ` ${props.className}`;
}
return (
<button type="button" {...props} className={className}>
{props.children}
</button>
);
}

Просмотреть файл

@ -0,0 +1,13 @@
.a11y-link-button {
border: 0;
padding: 0;
cursor: pointer;
text-align: unset;
color: var(--newtab-link-primary-color);
&:hover,
&:focus {
text-decoration: underline;
}
}

Просмотреть файл

@ -472,7 +472,8 @@ export class ASRouterAdminInner extends React.PureComponent {
if (!this.state.providers) {
return null;
}
return (<p>Show messages from <select value={this.state.messageFilter} onBlur={this.onChangeMessageFilter}>
// eslint-disable-next-line jsx-a11y/no-onchange
return (<p>Show messages from <select value={this.state.messageFilter} onChange={this.onChangeMessageFilter}>
<option value="all">all providers</option>
{this.state.providers.map(provider => (<option key={provider.id} value={provider.id}>{provider.id}</option>))}
</select></p>);

Просмотреть файл

@ -183,7 +183,9 @@ export class TopSiteLink extends React.PureComponent {
}
return (<li className={topSiteOuterClassName} onDrop={this.onDragEvent} onDragOver={this.onDragEvent} onDragEnter={this.onDragEvent} onDragLeave={this.onDragEvent} {...draggableProps}>
<div className="top-site-inner">
<a href={link.searchTopSite ? undefined : link.url} tabIndex="0" onKeyPress={this.onKeyPress} onClick={onClick} draggable={true}>
{/* We don't yet support an accessible drag-and-drop implementation, see Bug 1552005 */}
{/* eslint-disable-next-line jsx-a11y/anchor-is-valid */}
<a className="top-site-button" href={link.searchTopSite ? undefined : link.url} tabIndex="0" onKeyPress={this.onKeyPress} onClick={onClick} draggable={true}>
<div className="tile" aria-hidden={true} data-fallback={letterFallback}>
<div className={imageClassName} style={imageStyle} />
{link.searchTopSite && <div className="top-site-icon search-topsite" />}

Просмотреть файл

@ -1,4 +1,5 @@
import {actionCreators as ac, actionTypes as at} from "common/Actions.jsm";
import {A11yLinkButton} from "content-src/components/A11yLinkButton/A11yLinkButton";
import {FormattedMessage} from "react-intl";
import React from "react";
import {TOP_SITES_SOURCE} from "./TopSitesConstants";
@ -163,9 +164,9 @@ export class TopSiteForm extends React.PureComponent {
customScreenshotUrl && this.props.previewUrl === this.cleanUrl(customScreenshotUrl);
if (!this.state.showCustomScreenshotForm) {
return (<a className="enable-custom-image-input" onClick={this.onEnableScreenshotUrlForm}>
<FormattedMessage id="topsites_form_use_image_link" />
</a>);
return (<A11yLinkButton onClick={this.onEnableScreenshotUrlForm} className="enable-custom-image-input">
<FormattedMessage id="topsites_form_use_image_link" />
</A11yLinkButton>);
}
return (<div className="custom-image-input-container">
<TopSiteFormInput
@ -198,8 +199,10 @@ export class TopSiteForm extends React.PureComponent {
previewLink.screenshot = this.props.previewResponse;
previewLink.customScreenshotURL = this.props.previewUrl;
}
// Handles the form submit so an enter press performs the correct action
const onSubmit = previewMode ? this.onPreviewButtonClick : this.onDoneButtonClick;
return (
<form className="topsite-form">
<form className="topsite-form" onSubmit={onSubmit}>
<div className="form-input-container">
<h3 className="section-title">
<FormattedMessage id={showAsAdd ? "topsites_form_add_header" : "topsites_form_edit_header"} />
@ -229,14 +232,14 @@ export class TopSiteForm extends React.PureComponent {
</div>
</div>
<section className="actions">
<button className="cancel" type="button" onClick={this.onCancelButtonClick}>
<button className="cancel" type="button" onClick={this.onCancelButtonClick} >
<FormattedMessage id="topsites_form_cancel_button" />
</button>
{previewMode ?
<button className="done preview" type="submit" onClick={this.onPreviewButtonClick}>
<button className="done preview" type="submit" >
<FormattedMessage id="topsites_form_preview_button" />
</button> :
<button className="done" type="submit" onClick={this.onDoneButtonClick}>
<button className="done" type="submit" >
<FormattedMessage id={showAsAdd ? "topsites_form_add_button" : "topsites_form_save_button"} />
</button>}
</section>

Просмотреть файл

@ -7,6 +7,7 @@ export class TopSiteFormInput extends React.PureComponent {
this.state = {validationError: this.props.validationError};
this.onChange = this.onChange.bind(this);
this.onMount = this.onMount.bind(this);
this.onClearIconPress = this.onClearIconPress.bind(this);
}
componentWillReceiveProps(nextProps) {
@ -22,6 +23,15 @@ export class TopSiteFormInput extends React.PureComponent {
}
}
onClearIconPress(event) {
// If there is input in the URL or custom image URL fields,
// and we hit 'enter' while tabbed over the clear icon,
// we should execute the function to clear the field.
if (event.key === "Enter") {
this.props.onClear();
}
}
onChange(ev) {
if (this.state.validationError) {
this.setState({validationError: false});
@ -33,23 +43,35 @@ export class TopSiteFormInput extends React.PureComponent {
this.input = input;
}
render() {
renderLoadingOrCloseButton() {
const showClearButton = this.props.value && this.props.onClear;
if (this.props.loading) {
return (<div className="loading-container"><div className="loading-animation" /></div>);
} else if (showClearButton) {
return (<button type="button" className="icon icon-clear-input icon-button-style"
onClick={this.props.onClear}
onKeyPress={this.onClearIconPress} />);
}
return null;
}
render() {
const {typeUrl} = this.props;
const {validationError} = this.state;
return (<label><FormattedMessage id={this.props.titleId} />
<div className={`field ${typeUrl ? "url" : ""}${validationError ? " invalid" : ""}`}>
{this.props.loading ?
<div className="loading-container"><div className="loading-animation" /></div> :
showClearButton && <div className="icon icon-clear-input" onClick={this.props.onClear} />}
<input type="text"
value={this.props.value}
ref={this.onMount}
onChange={this.onChange}
placeholder={this.props.intl.formatMessage({id: this.props.placeholderId})}
// Set focus on error if the url field is valid or when the input is first rendered and is empty
// eslint-disable-next-line jsx-a11y/no-autofocus
autoFocus={this.props.shouldFocus}
disabled={this.props.loading} />
{this.renderLoadingOrCloseButton()}
{validationError &&
<aside className="error-tooltip">
<FormattedMessage id={this.props.errorMessageId} />

Просмотреть файл

@ -139,7 +139,7 @@ export class _TopSites extends React.PureComponent {
<div className="edit-topsites-wrapper">
{editForm &&
<div className="edit-topsites">
<div className="modal-overlay" onClick={this.onEditFormClose} />
<div className="modal-overlay" onClick={this.onEditFormClose} role="presentation" />
<div className="modal">
<TopSiteForm
site={props.TopSites.rows[editForm.index]}
@ -152,7 +152,7 @@ export class _TopSites extends React.PureComponent {
}
{showSearchShortcutsForm &&
<div className="edit-search-shortcuts">
<div className="modal-overlay" onClick={this.onSearchShortcutsFormClose} />
<div className="modal-overlay" onClick={this.onSearchShortcutsFormClose} role="presentation" />
<div className="modal">
<SearchShortcutsForm
TopSites={props.TopSites}

Просмотреть файл

@ -406,10 +406,6 @@ $hover-transition-duration: 150ms;
font-size: 13px;
margin-top: 4px;
cursor: pointer;
&:hover {
text-decoration: underline;
}
}
.custom-image-input-container {

Просмотреть файл

@ -103,7 +103,8 @@ a {
padding: 10px 30px;
white-space: nowrap;
&:hover:not(.dismiss) {
&:hover:not(.dismiss),
&:focus:not(.dismiss) {
box-shadow: $shadow-primary;
transition: box-shadow 150ms;
}
@ -141,6 +142,7 @@ input {
}
// Components
@import '../components/A11yLinkButton/A11yLinkButton';
@import '../components/Base/Base';
@import '../components/ErrorBoundary/ErrorBoundary';
@import '../components/TopSites/TopSites';

Просмотреть файл

@ -9,6 +9,7 @@
vertical-align: middle;
width: $icon-size;
// helper classes
&.icon-spacer {
margin-inline-end: 8px;
}
@ -17,6 +18,17 @@
margin-inline-end: 6px;
}
&.icon-button-style {
fill: var(--newtab-icon-secondary-color);
border: 0;
&:focus,
&:hover {
fill: var(--newtab-text-primary-color);
}
}
// icon images
&.icon-bookmark-added {
background-image: url('chrome://browser/skin/bookmark.svg');
}
@ -26,7 +38,6 @@
}
&.icon-clear-input {
fill: var(--newtab-icon-secondary-color);
background-image: url('#{$image-path}glyph-cancel-16.svg');
}

Просмотреть файл

@ -132,7 +132,7 @@
"debugcoverage": "open logs/coverage/index.html",
"lint": "npm-run-all lint:*",
"lint:eslint": "esw --ext=.js,.jsm,.json,.jsx .",
"lint:jsx-a11y": "esw --config=.eslintrc.jsx-a11y.js --ext=.jsx content-src/asrouter content-src/components/ASRouterAdmin",
"lint:jsx-a11y": "esw --config=.eslintrc.jsx-a11y.js --ext=.jsx content-src/asrouter content-src/components/ASRouterAdmin content-src/components/TopSites",
"lint:sasslint": "sass-lint -v -q",
"strings-import": "node ./bin/strings-import.js",
"test": "npm run testmc",

Просмотреть файл

@ -172,7 +172,7 @@ describe("ASRouterAdmin", () => {
assert.lengthOf(wrapper.find(".message-id"), 1);
wrapper.find("select").simulate("blur", {target: {value: "bar"}});
wrapper.find("select").simulate("change", {target: {value: "bar"}});
assert.lengthOf(wrapper.find(".message-id"), 0);
});

Просмотреть файл

@ -3,6 +3,7 @@ import {GlobalOverrider, mountWithIntl, shallowWithIntl} from "test/unit/utils";
import {MIN_CORNER_FAVICON_SIZE, MIN_RICH_FAVICON_SIZE} from "content-src/components/TopSites/TopSitesConstants";
import {TOP_SITES_DEFAULT_ROWS, TOP_SITES_MAX_SITES_PER_ROW} from "common/Reducers.jsm";
import {TopSite, TopSiteLink, _TopSiteList as TopSiteList, TopSitePlaceholder} from "content-src/components/TopSites/TopSite";
import {A11yLinkButton} from "content-src/components/A11yLinkButton/A11yLinkButton";
import {FormattedMessage} from "react-intl";
import {LinkMenu} from "content-src/components/LinkMenu/LinkMenu";
import React from "react";
@ -753,7 +754,7 @@ describe("<TopSiteForm>", () => {
it("should dispatch a PREVIEW_REQUEST", () => {
wrapper.setState({customScreenshotUrl: "screenshot"});
wrapper.find(".preview").simulate("click");
wrapper.find(".preview").simulate("submit");
assert.calledTwice(wrapper.props().dispatch);
assert.calledWith(wrapper.props().dispatch, ac.AlsoToMain({
@ -822,18 +823,18 @@ describe("<TopSiteForm>", () => {
});
it("should set validationError if url is empty", () => {
assert.equal(wrapper.state().validationError, false);
wrapper.find(".done").simulate("click");
wrapper.find(".done").simulate("submit");
assert.equal(wrapper.state().validationError, true);
});
it("should set validationError if url is invalid", () => {
wrapper.setState({"url": "not valid"});
assert.equal(wrapper.state().validationError, false);
wrapper.find(".done").simulate("click");
wrapper.find(".done").simulate("submit");
assert.equal(wrapper.state().validationError, true);
});
it("should call onClose and dispatch with right args if URL is valid", () => {
wrapper.setState({"url": "valid.com", "label": "a label"});
wrapper.find(".done").simulate("click");
wrapper.find(".done").simulate("submit");
assert.calledOnce(wrapper.instance().props.onClose);
assert.calledWith(
wrapper.instance().props.dispatch,
@ -854,7 +855,7 @@ describe("<TopSiteForm>", () => {
});
it("should not pass empty string label in dispatch data", () => {
wrapper.setState({"url": "valid.com", "label": ""});
wrapper.find(".done").simulate("click");
wrapper.find(".done").simulate("submit");
assert.calledWith(
wrapper.instance().props.dispatch,
{
@ -867,7 +868,7 @@ describe("<TopSiteForm>", () => {
it("should open the custom screenshot input", () => {
assert.isFalse(wrapper.state().showCustomScreenshotForm);
wrapper.find(".enable-custom-image-input").simulate("click");
wrapper.find(A11yLinkButton).simulate("click");
assert.isTrue(wrapper.state().showCustomScreenshotForm);
});
@ -893,7 +894,7 @@ describe("<TopSiteForm>", () => {
it("should show error and not call onClose or dispatch if URL is empty", () => {
wrapper.setState({"url": ""});
assert.equal(wrapper.state().validationError, false);
wrapper.find(".done").simulate("click");
wrapper.find(".done").simulate("submit");
assert.equal(wrapper.state().validationError, true);
assert.notCalled(wrapper.instance().props.onClose);
assert.notCalled(wrapper.instance().props.dispatch);
@ -901,13 +902,13 @@ describe("<TopSiteForm>", () => {
it("should show error and not call onClose or dispatch if URL is invalid", () => {
wrapper.setState({"url": "not valid"});
assert.equal(wrapper.state().validationError, false);
wrapper.find(".done").simulate("click");
wrapper.find(".done").simulate("submit");
assert.equal(wrapper.state().validationError, true);
assert.notCalled(wrapper.instance().props.onClose);
assert.notCalled(wrapper.instance().props.dispatch);
});
it("should call onClose and dispatch with right args if URL is valid", () => {
wrapper.find(".done").simulate("click");
wrapper.find(".done").simulate("submit");
assert.calledOnce(wrapper.instance().props.onClose);
assert.calledTwice(wrapper.instance().props.dispatch);
assert.calledWith(
@ -930,7 +931,7 @@ describe("<TopSiteForm>", () => {
it("should set customScreenshotURL to null if it was removed", () => {
wrapper.setState({customScreenshotUrl: ""});
wrapper.find(".done").simulate("click");
wrapper.find(".done").simulate("submit");
assert.calledWith(
wrapper.instance().props.dispatch,
@ -943,7 +944,7 @@ describe("<TopSiteForm>", () => {
});
it("should call onClose and dispatch with right args if URL is valid (negative index)", () => {
wrapper.setProps({index: -1});
wrapper.find(".done").simulate("click");
wrapper.find(".done").simulate("submit");
assert.calledOnce(wrapper.instance().props.onClose);
assert.calledTwice(wrapper.instance().props.dispatch);
assert.calledWith(
@ -957,7 +958,7 @@ describe("<TopSiteForm>", () => {
});
it("should not pass empty string label in dispatch data", () => {
wrapper.setState({"label": ""});
wrapper.find(".done").simulate("click");
wrapper.find(".done").simulate("submit");
assert.calledWith(
wrapper.instance().props.dispatch,
{

Просмотреть файл

@ -64,7 +64,7 @@ scripts:
# lint: Run eslint and sass-lint
lint:
eslint: esw --ext=.js,.jsm,.json,.jsx .
jsx-a11y: esw --config=.eslintrc.jsx-a11y.js --ext=.jsx content-src/asrouter content-src/components/ASRouterAdmin
jsx-a11y: esw --config=.eslintrc.jsx-a11y.js --ext=.jsx content-src/asrouter content-src/components/ASRouterAdmin content-src/components/TopSites
sasslint: sass-lint -v -q
# strings-import: Replace local strings with those from l10n-central