diff --git a/.eslintrc.json b/.eslintrc.json index 23083d6186a..b0be5232a9f 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -2,7 +2,8 @@ "extends": [ "plugin:github/es6", "plugin:github/react", - "plugin:jsx-a11y/recommended" + "plugin:jsx-a11y/recommended", + "plugin:react-hooks/recommended" ], "rules": { "import/no-namespace": 0, @@ -11,6 +12,7 @@ "ignoreRestSiblings": true }], "eslint-comments/no-use": 0, + "react-hooks/exhaustive-deps": "error", "jsx-a11y/label-has-for": [ 2, { diff --git a/package.json b/package.json index ca792cd6589..2e8ad9d162b 100644 --- a/package.json +++ b/package.json @@ -41,8 +41,8 @@ "@styled-system/prop-types": "5.1.2", "@styled-system/props": "5.1.4", "@styled-system/theme-get": "5.1.2", - "@types/styled-components": "^4.4.0", "@testing-library/react": "9.4.0", + "@types/styled-components": "^4.4.0", "@types/styled-system": "5.1.2", "babel-plugin-macros": "2.6.1", "babel-polyfill": "6.26.0", @@ -69,6 +69,7 @@ "eslint-plugin-github": "3.4.0", "eslint-plugin-jest": "22.20.0", "eslint-plugin-jsx-a11y": "6.2.3", + "eslint-plugin-react-hooks": "3.0.0", "jest": "25.0.0", "jest-styled-components": "6.3.3", "jscodeshift": "0.6.4", diff --git a/src/Pagination/Pagination.js b/src/Pagination/Pagination.js index ff9b56a5b7f..200f6788464 100644 --- a/src/Pagination/Pagination.js +++ b/src/Pagination/Pagination.js @@ -93,7 +93,7 @@ function usePaginationPages({ ) }) - }, [model, hrefBuilder, pageChange]) + }, [model, hrefBuilder, pageChange, theme]) return children } diff --git a/src/SelectMenu/SelectMenuTab.js b/src/SelectMenu/SelectMenuTab.js index 160df1806da..f1421db24d1 100644 --- a/src/SelectMenu/SelectMenuTab.js +++ b/src/SelectMenu/SelectMenuTab.js @@ -63,7 +63,7 @@ const SelectMenuTab = ({tabName, index, className, onClick, ...rest}) => { if (!menuContext.selectedTab && index === 0) { menuContext.setSelectedTab(tabName) } - }, []) + }, [index, menuContext, tabName]) const isSelected = menuContext.selectedTab === tabName diff --git a/src/SelectMenu/hooks/useKeyboardNav.js b/src/SelectMenu/hooks/useKeyboardNav.js index 1c5dae1fb67..4675d13d157 100644 --- a/src/SelectMenu/hooks/useKeyboardNav.js +++ b/src/SelectMenu/hooks/useKeyboardNav.js @@ -1,82 +1,89 @@ -import {useEffect} from 'react' +import {useEffect, useCallback} from 'react' // adapted from details-menu web component https://github.com/github/details-menu-element function useKeyboardNav(details, open, setOpen) { - const handleKeyDown = event => { - const closeDetails = () => { - setOpen(false) - const summary = details.current.querySelector('summary') - if (summary) summary.focus() - } - const openDetails = () => { - setOpen(true) - } - const focusItem = next => { - const options = Array.from( - details.current.querySelectorAll('[role^="menuitem"]:not([hidden]):not([disabled]):not([aria-disabled="true"])') - ) - const selected = document.activeElement - const index = options.indexOf(selected) - const found = next ? options[index + 1] : options[index - 1] - const def = next ? options[0] : options[options.length - 1] - return found || def - } + const handleKeyDown = useCallback( + event => { + const closeDetails = () => { + setOpen(false) + const summary = details.current.querySelector('summary') + if (summary) summary.focus() + } + const openDetails = () => { + setOpen(true) + } + const focusItem = next => { + const options = Array.from( + details.current.querySelectorAll( + '[role^="menuitem"]:not([hidden]):not([disabled]):not([aria-disabled="true"])' + ) + ) + const selected = document.activeElement + const index = options.indexOf(selected) + const found = next ? options[index + 1] : options[index - 1] + const def = next ? options[0] : options[options.length - 1] + return found || def + } - const isMenuItem = el => { - const role = el.getAttribute('role') - return role === 'menuitem' || role === 'menuitemcheckbox' || role === 'menuitemradio' - } - if (!(event instanceof KeyboardEvent)) return - const isSummaryFocused = event.target instanceof Element && event.target.tagName === 'SUMMARY' - switch (event.key) { - case 'Escape': - if (open) { - closeDetails(details) - event.preventDefault() - event.stopPropagation() - } - break - case 'ArrowDown': - { - if (isSummaryFocused && !open) { - openDetails(details) + const isMenuItem = el => { + const role = el.getAttribute('role') + return role === 'menuitem' || role === 'menuitemcheckbox' || role === 'menuitemradio' + } + if (!(event instanceof KeyboardEvent)) return + const isSummaryFocused = event.target instanceof Element && event.target.tagName === 'SUMMARY' + switch (event.key) { + case 'Escape': + if (open) { + closeDetails(details) + event.preventDefault() + event.stopPropagation() } - const target = focusItem(true) - if (target) target.focus() - event.preventDefault() - } - break - case 'ArrowUp': - { - if (isSummaryFocused && !open) { - openDetails() + break + case 'ArrowDown': + { + if (isSummaryFocused && !open) { + openDetails(details) + } + const target = focusItem(true) + if (target) target.focus() + event.preventDefault() } - const target = focusItem(false) - if (target) target.focus() - event.preventDefault() - } - break - case ' ': - case 'Enter': - { - const selected = document.activeElement - if (selected && isMenuItem(selected) && selected.closest('details') === details) { + break + case 'ArrowUp': + { + if (isSummaryFocused && !open) { + openDetails() + } + const target = focusItem(false) + if (target) target.focus() event.preventDefault() - event.stopPropagation() - selected.click() } - } - break - } - } + break + case ' ': + case 'Enter': + { + const selected = document.activeElement + if (selected && isMenuItem(selected) && selected.closest('details') === details) { + event.preventDefault() + event.stopPropagation() + selected.click() + } + } + break + } + }, + [details, open, setOpen] + ) + useEffect(() => { - if (!details.current) return + const current = details.current + if (!current) return - details.current.addEventListener('keydown', handleKeyDown) + current.addEventListener('keydown', handleKeyDown) return () => { - details.current.removeEventListener('keydown', handleKeyDown) + current.removeEventListener('keydown', handleKeyDown) } - }, [details.current]) + }, [details, handleKeyDown]) } export default useKeyboardNav diff --git a/yarn.lock b/yarn.lock index 08ac3cf2613..cec80e1527a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4837,6 +4837,11 @@ eslint-plugin-prettier@>=2.6.0: dependencies: prettier-linter-helpers "^1.0.0" +eslint-plugin-react-hooks@3.0.0: + version "3.0.0" + resolved "https://registry.yarnpkg.com/eslint-plugin-react-hooks/-/eslint-plugin-react-hooks-3.0.0.tgz#9e80c71846eb68dd29c3b21d832728aa66e5bd35" + integrity sha512-EjxTHxjLKIBWFgDJdhKKzLh5q+vjTFrqNZX36uIxWS4OfyXe5DawqPj3U5qeJ1ngLwatjzQnmR0Lz0J0YH3kxw== + eslint-plugin-react@>=7.7.0: version "7.17.0" resolved "https://registry.yarnpkg.com/eslint-plugin-react/-/eslint-plugin-react-7.17.0.tgz#a31b3e134b76046abe3cd278e7482bd35a1d12d7"