Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

react-a11y-no-onchange should allow onChange when onBlur is present #836

Closed
huan086 opened this issue Mar 4, 2019 · 0 comments · Fixed by #854
Closed

react-a11y-no-onchange should allow onChange when onBlur is present #836

huan086 opened this issue Mar 4, 2019 · 0 comments · Fixed by #854
Labels
Difficulty: Easy Someone with little to no experience in TSLint should be able to send a pull request for this issue. Good First Issue 🙌 Howdy, neighbor! Status: Accepting PRs Type: Bug
Milestone

Comments

@huan086
Copy link

huan086 commented Mar 4, 2019

Bug Report

  • tslint-microsoft-contrib version: 6.0.0
  • TSLint version: 5.12.1
  • TypeScript version: 3.3.3
  • Running TSLint via: webpack fork-ts-checker-webpack-plugin

TypeScript code being linted

export const RouterRowsPerPage = ({ rowsPerPage, history, query }: RowsPerPageProps): JSX.Element => {
    const queryString = stringify(query);
    const [focusState, setFocusState] = useState(false);
    const [selectState, setSelectState] = useState(rowsPerPage);

    const onFocus = useCallback(
        (e: FocusEvent<HTMLSelectElement>) => {
            setFocusState(true);
        },
        []);

    // For accessibility, only change the URL onBlur.
    const onBlur = useCallback(
        (e: FocusEvent<HTMLSelectElement>) => {
            setFocusState(false);

            const newRowsPerPage = parseInt(e.target.value, 10);
            if (rowsPerPage !== newRowsPerPage) {
                history.push(`?${stringify({ ...query, rowsPerPage: newRowsPerPage })}`);
            }
        },
        [queryString, history, rowsPerPage]);

    const onChange = useCallback(
        (e: FocusEvent<HTMLSelectElement>) => {
            const newRowsPerPage = parseInt(e.target.value, 10);
            setSelectState(newRowsPerPage);
        },
        []);

    const selectValue = focusState ? selectState : rowsPerPage;
    return (
        <select onChange={onChange} onFocus={onFocus} onBlur={onBlur} value={selectValue}>
            {options.map(option => (
                <option key={option} role="option" value={option} aria-selected={rowsPerPage === option}>{option}</option>
            ))}
        </select>
    );
};

with tslint.json configuration:

{
  "extends": [ "tslint:latest", "tslint-react", "tslint-microsoft-contrib", "tslint-config-airbnb", "tslint-sonarts" ],
  "linterOptions": {
    "exclude": [ "**/*.json" ]
  },
  "rules": {
    "ter-indent": [ true, 4 ]
  }
}

Actual behavior

react-a11y-no-onchange: onChange event handler should not be used with the <select>. Please use onBlur instead.

Expected behavior

onChange is required for controlled components. See also eslint, which allows onChange as long as onBlur exists

@JoshuaKGoldberg JoshuaKGoldberg added Type: Bug Status: Accepting PRs Good First Issue 🙌 Howdy, neighbor! Difficulty: Easy Someone with little to no experience in TSLint should be able to send a pull request for this issue. labels Mar 21, 2019
soon added a commit to soon/tslint-microsoft-contrib that referenced this issue Apr 2, 2019
soon added a commit to soon/tslint-microsoft-contrib that referenced this issue Apr 4, 2019
soon added a commit to soon/tslint-microsoft-contrib that referenced this issue Apr 4, 2019
@JoshuaKGoldberg JoshuaKGoldberg added this to the 6.1.1-beta milestone Apr 15, 2019
@IllusionMH IllusionMH modified the milestones: 6.1.1-beta, 6.2.0-beta May 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Difficulty: Easy Someone with little to no experience in TSLint should be able to send a pull request for this issue. Good First Issue 🙌 Howdy, neighbor! Status: Accepting PRs Type: Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants