diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 09f04ca1907..8b3aa239b51 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- `Tabs` and `Menu` arrow key navigation persists the last focused item - `Breakpoint` component - `Tree / TreeItem` - disabled and selected states diff --git a/packages/components/snapshots/MenuGroup/No Label-snap.png b/packages/components/snapshots/MenuGroup/No Label-snap.png new file mode 100644 index 00000000000..68e1fc413d2 Binary files /dev/null and b/packages/components/snapshots/MenuGroup/No Label-snap.png differ diff --git a/packages/components/src/Menu/Menu.story.tsx b/packages/components/src/Menu/Menu.story.tsx index a283e50383b..ae88f3747ee 100644 --- a/packages/components/src/Menu/Menu.story.tsx +++ b/packages/components/src/Menu/Menu.story.tsx @@ -39,7 +39,8 @@ import { Divider } from '../Divider' import { FieldToggleSwitch } from '../Form' import { Icon } from '../Icon' import { Space, SpaceVertical } from '../Layout' -import { Text, Paragraph } from '../Text' +import { Tab, TabList, TabPanel, TabPanels, Tabs } from '../Tabs' +import { Heading, Text, Paragraph } from '../Text' import { Tooltip } from '../Tooltip' import { useToggle } from '../utils' import { Menu } from './Menu' @@ -564,3 +565,33 @@ export const WithTooltip = () => { WithTooltip.parameters = { storyshots: { disable: true }, } + +export const ArrowKeyNavigation = () => ( + + + Menu + + 1 + 2 + 3 + + Tabs + + + 1 + 2 + 3 + + + One + Two + Three + + + + +) + +ArrowKeyNavigation.parameters = { + storyshots: { disable: true }, +} diff --git a/packages/components/src/Menu/MenuGroup.story.tsx b/packages/components/src/Menu/MenuGroup.story.tsx index f933c3517dd..47139a7bbea 100644 --- a/packages/components/src/Menu/MenuGroup.story.tsx +++ b/packages/components/src/Menu/MenuGroup.story.tsx @@ -96,6 +96,12 @@ NoIcons.args = { icons: false, } +export const NoLabel = Template.bind({}) +NoLabel.args = { + ...Basic.args, + label: undefined, +} + export default { component: MenuGroup, title: 'MenuGroup', diff --git a/packages/components/src/Menu/MenuGroup.test.tsx b/packages/components/src/Menu/MenuGroup.test.tsx index 6dd49ac4ef1..37e8a3dfa77 100644 --- a/packages/components/src/Menu/MenuGroup.test.tsx +++ b/packages/components/src/Menu/MenuGroup.test.tsx @@ -26,74 +26,47 @@ import 'jest-styled-components' import React from 'react' -import { assertSnapshot, renderWithTheme } from '@looker/components-test-utils' +import { renderWithTheme } from '@looker/components-test-utils' -import { Box } from '../Layout' import { MenuGroup } from './MenuGroup' import { MenuItem } from './MenuItem' import { MenuList } from './MenuList' -test('MenuGroup', () => { - assertSnapshot( - - who! - - ) -}) - -test('MenuGroup - label', () => { - assertSnapshot( - - what? - who? - where? - - ) -}) - -test('MenuGroup - JSX label', () => { - assertSnapshot( - Questions}> - what? - who? - where? - - ) -}) - -test('MenuGroup - indents MenuItem with no icon when a sibling within the same group has an icon', () => { - const { getByTestId } = renderWithTheme( - - Gouda - Cheddar - - ) - - getByTestId('menu-item-cheddar-icon-placeholder') -}) - -test('MenuGroup - indents MenuItem with no icon when a sibling outside of the group has an icon', () => { - const { getByTestId } = renderWithTheme( - - Gouda +describe('MenuGroup', () => { + test('indents MenuItem with no icon when a sibling within the same group has an icon', () => { + const { getByTestId } = renderWithTheme( + Gouda Cheddar - - ) - - getByTestId('menu-item-cheddar-icon-placeholder') -}) - -test('MenuGroup - does not indent MenuItems with no icon if all siblings do not have icons', () => { - const { queryByTestId } = renderWithTheme( - - Gouda - Cheddar - - ) + ) + + getByTestId('menu-item-cheddar-icon-placeholder') + }) + + test('indents MenuItem with no icon when a sibling outside of the group has an icon', () => { + const { getByTestId } = renderWithTheme( + + Gouda + + Cheddar + + + ) + + getByTestId('menu-item-cheddar-icon-placeholder') + }) + + test('does not indent MenuItems with no icon if all siblings do not have icons', () => { + const { queryByTestId } = renderWithTheme( + + Gouda + Cheddar + + ) - expect( - queryByTestId('menu-item-cheddar-icon-placeholder') - ).not.toBeInTheDocument() + expect( + queryByTestId('menu-item-cheddar-icon-placeholder') + ).not.toBeInTheDocument() + }) }) diff --git a/packages/components/src/Menu/MenuItem.tsx b/packages/components/src/Menu/MenuItem.tsx index 197763252bb..245948f2ce2 100644 --- a/packages/components/src/Menu/MenuItem.tsx +++ b/packages/components/src/Menu/MenuItem.tsx @@ -86,7 +86,11 @@ const MenuItemInternal: FC = (props) => { } = props const [isFocusVisible, setFocusVisible] = useState(false) - const { compact: contextCompact } = useContext(MenuItemContext) + const { + compact: contextCompact, + renderIconPlaceholder, + setRenderIconPlaceholder, + } = useContext(MenuItemContext) const compact = propCompact === undefined ? contextCompact : propCompact const handleOnBlur = (event: React.FocusEvent) => { @@ -95,12 +99,6 @@ const MenuItemInternal: FC = (props) => { } const { closeModal } = useContext(DialogContext) - const { - renderIconPlaceholder, - setRenderIconPlaceholder, - handleArrowDown, - handleArrowUp, - } = useContext(MenuItemContext) const handleOnClick = (event: React.MouseEvent) => { setFocusVisible(false) @@ -111,17 +109,6 @@ const MenuItemInternal: FC = (props) => { } } - const handleOnKeyDown = (event: React.KeyboardEvent) => { - switch (event.key) { - case 'ArrowUp': - handleArrowUp && handleArrowUp(event) - break - case 'ArrowDown': - handleArrowDown && handleArrowDown(event) - break - } - } - const handleOnKeyUp = (event: React.KeyboardEvent) => { onKeyUp && onKeyUp(event) setFocusVisible(true) @@ -174,7 +161,13 @@ const MenuItemInternal: FC = (props) => { : props.rel const menuItemContent = ( - + {renderedIcon} {children} @@ -197,7 +190,6 @@ const MenuItemInternal: FC = (props) => { onBlur={handleOnBlur} onClick={disabled ? undefined : handleOnClick} onKeyUp={handleOnKeyUp} - onKeyDown={handleOnKeyDown} className={className} > {tooltip ? ( diff --git a/packages/components/src/Menu/MenuItemContext.ts b/packages/components/src/Menu/MenuItemContext.ts index ba9d22c3f94..b32eef7374e 100644 --- a/packages/components/src/Menu/MenuItemContext.ts +++ b/packages/components/src/Menu/MenuItemContext.ts @@ -24,14 +24,12 @@ */ -import { createContext, KeyboardEvent } from 'react' +import { createContext } from 'react' export interface MenuItemContextProps { compact?: boolean renderIconPlaceholder?: boolean setRenderIconPlaceholder?: (state: boolean) => void - handleArrowUp?: (e: KeyboardEvent) => void - handleArrowDown?: (e: KeyboardEvent) => void } const menuItemContext: MenuItemContextProps = {} diff --git a/packages/components/src/Menu/MenuList.tsx b/packages/components/src/Menu/MenuList.tsx index d78aacb4fe7..cca7a85e6d2 100644 --- a/packages/components/src/Menu/MenuList.tsx +++ b/packages/components/src/Menu/MenuList.tsx @@ -30,7 +30,6 @@ import React, { Children, forwardRef, isValidElement, - KeyboardEvent, ReactChild, Ref, useEffect, @@ -57,7 +56,7 @@ import { reset, omitStyledProps, } from '@looker/design-tokens' -import { moveFocus, useForkedRef, useWindow } from '../utils' +import { useArrowKeyNav, useWindow } from '../utils' import { MenuItemContext } from './MenuItemContext' import { MenuGroup } from './MenuGroup' @@ -137,6 +136,10 @@ export const MenuListInternal = forwardRef( pin, placement, windowing, + + onBlur, + onFocus, + onKeyDown, ...props }: MenuListProps, forwardedRef: Ref @@ -179,30 +182,23 @@ export const MenuListInternal = forwardRef( return (child: ReactChild) => getMenuGroupHeight(child, compact) }, [windowing, childArray, compact]) - const { content, containerElement, ref } = useWindow({ + const { content, ref } = useWindow({ childHeight: childHeight, children: children as JSX.Element | JSX.Element[], enabled: windowing !== 'none', + ref: forwardedRef, spacerTag: 'li', }) - const forkedRef = useForkedRef(forwardedRef, ref) - function handleArrowKey(direction: number, initial: number) { - moveFocus(direction, initial, containerElement) - } + const navProps = useArrowKeyNav({ + onBlur, + onFocus, + onKeyDown, + ref, + }) const context = { compact, - handleArrowDown: (e: KeyboardEvent) => { - e.preventDefault() - handleArrowKey(1, 0) - return false - }, - handleArrowUp: (e: KeyboardEvent) => { - e.preventDefault() - handleArrowKey(-1, -1) - return false - }, renderIconPlaceholder, setRenderIconPlaceholder, } @@ -210,10 +206,9 @@ export const MenuListInternal = forwardRef( return (
    {content}
diff --git a/packages/components/src/Menu/__snapshots__/MenuGroup.test.tsx.snap b/packages/components/src/Menu/__snapshots__/MenuGroup.test.tsx.snap deleted file mode 100644 index f4b71d82110..00000000000 --- a/packages/components/src/Menu/__snapshots__/MenuGroup.test.tsx.snap +++ /dev/null @@ -1,502 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`MenuGroup - JSX label 1`] = ` -.c4 { - font-family: 'Roboto','Noto Sans JP','Noto Sans CJK KR','Noto Sans Arabic UI','Noto Sans Devanagari UI','Noto Sans Hebrew','Noto Sans Thai UI','Helvetica','Arial',sans-serif; -} - -.c3 { - font-family: Roboto,'Noto Sans JP','Noto Sans CJK KR','Noto Sans Arabic UI','Noto Sans Devanagari UI','Noto Sans Hebrew','Noto Sans Thai UI','Helvetica','Arial',sans-serif; - font-size: 0.875rem; - font-weight: 600; - line-height: 1.25rem; - padding-left: 1rem; - color: #262D33; -} - -.c1 { - background: #FFFFFF; - box-shadow: none; - margin-bottom: 0.25rem; - position: -webkit-sticky; - position: sticky; - top: -1px; -} - -.c1 .c2 { - color: #343C42; -} - -.c0 { - font-family: 'Roboto','Noto Sans JP','Noto Sans CJK KR','Noto Sans Arabic UI','Noto Sans Devanagari UI','Noto Sans Hebrew','Noto Sans Thai UI','Helvetica','Arial',sans-serif; - list-style-type: none; - padding: 0.5rem 0; -} - -.c5 { - -webkit-align-items: center; - -webkit-box-align: center; - -ms-flex-align: center; - align-items: center; - color: #262D33; - font-size: 0.875rem; - font-weight: 400; - list-style-type: none; - outline: none; - -webkit-text-decoration: none; - text-decoration: none; - -webkit-transition: background 150ms cubic-bezier(0.86,0,0.07,1), color 150ms cubic-bezier(0.86,0,0.07,1); - transition: background 150ms cubic-bezier(0.86,0,0.07,1), color 150ms cubic-bezier(0.86,0,0.07,1); -} - -.c5 button, -.c5 a { - font-family: 'Roboto','Noto Sans JP','Noto Sans CJK KR','Noto Sans Arabic UI','Noto Sans Devanagari UI','Noto Sans Hebrew','Noto Sans Thai UI','Helvetica','Arial',sans-serif; - -webkit-align-items: center; - -webkit-box-align: center; - -ms-flex-align: center; - align-items: center; - background: transparent; - border: none; - color: inherit; - cursor: pointer; - display: -webkit-box; - display: -webkit-flex; - display: -ms-flexbox; - display: flex; - -webkit-flex: 1; - -ms-flex: 1; - flex: 1; - font-size: inherit; - font-weight: inherit; - min-height: 40px; - outline: none; - padding: 0.5rem 1rem; - text-align: left; - -webkit-text-decoration: none; - text-decoration: none; - width: 100%; -} - -.c5 button:hover, -.c5 a:hover, -.c5 button:focus, -.c5 a:focus { - color: inherit; - position: relative; - -webkit-text-decoration: none; - text-decoration: none; -} - -.c5 .Icon-sc-7y0t4i-0 { - -webkit-transition: color 150ms cubic-bezier(0.86,0,0.07,1); - transition: color 150ms cubic-bezier(0.86,0,0.07,1); -} - -.c5:hover { - background: #F5F6F7; -} - -.c5[aria-current='true'] { - background: #DEE1E5; - font-weight: 600; -} - -.c5[disabled] { - color: #939BA5; -} - -.c5[disabled] button, -.c5[disabled] a { - cursor: not-allowed; -} - -.c5[disabled]:hover { - background: #FFFFFF; - color: #939BA5; -} - -.c6 .Icon-sc-7y0t4i-0 { - -webkit-align-self: center; - -ms-flex-item-align: center; - align-self: center; -} - -
  • -
    -
    -

    -
    - Questions -
    -

    -
    -
      -
    • - -
    • -
    • - -
    • -
    • - -
    • -
    -
  • -`; - -exports[`MenuGroup - label 1`] = ` -.c3 { - font-family: Roboto,'Noto Sans JP','Noto Sans CJK KR','Noto Sans Arabic UI','Noto Sans Devanagari UI','Noto Sans Hebrew','Noto Sans Thai UI','Helvetica','Arial',sans-serif; - font-size: 0.875rem; - font-weight: 600; - line-height: 1.25rem; - padding-left: 1rem; - color: #262D33; -} - -.c1 { - background: #FFFFFF; - box-shadow: none; - margin-bottom: 0.25rem; - position: -webkit-sticky; - position: sticky; - top: -1px; -} - -.c1 .c2 { - color: #343C42; -} - -.c0 { - font-family: 'Roboto','Noto Sans JP','Noto Sans CJK KR','Noto Sans Arabic UI','Noto Sans Devanagari UI','Noto Sans Hebrew','Noto Sans Thai UI','Helvetica','Arial',sans-serif; - list-style-type: none; - padding: 0.5rem 0; -} - -.c4 { - -webkit-align-items: center; - -webkit-box-align: center; - -ms-flex-align: center; - align-items: center; - color: #262D33; - font-size: 0.875rem; - font-weight: 400; - list-style-type: none; - outline: none; - -webkit-text-decoration: none; - text-decoration: none; - -webkit-transition: background 150ms cubic-bezier(0.86,0,0.07,1), color 150ms cubic-bezier(0.86,0,0.07,1); - transition: background 150ms cubic-bezier(0.86,0,0.07,1), color 150ms cubic-bezier(0.86,0,0.07,1); -} - -.c4 button, -.c4 a { - font-family: 'Roboto','Noto Sans JP','Noto Sans CJK KR','Noto Sans Arabic UI','Noto Sans Devanagari UI','Noto Sans Hebrew','Noto Sans Thai UI','Helvetica','Arial',sans-serif; - -webkit-align-items: center; - -webkit-box-align: center; - -ms-flex-align: center; - align-items: center; - background: transparent; - border: none; - color: inherit; - cursor: pointer; - display: -webkit-box; - display: -webkit-flex; - display: -ms-flexbox; - display: flex; - -webkit-flex: 1; - -ms-flex: 1; - flex: 1; - font-size: inherit; - font-weight: inherit; - min-height: 40px; - outline: none; - padding: 0.5rem 1rem; - text-align: left; - -webkit-text-decoration: none; - text-decoration: none; - width: 100%; -} - -.c4 button:hover, -.c4 a:hover, -.c4 button:focus, -.c4 a:focus { - color: inherit; - position: relative; - -webkit-text-decoration: none; - text-decoration: none; -} - -.c4 .Icon-sc-7y0t4i-0 { - -webkit-transition: color 150ms cubic-bezier(0.86,0,0.07,1); - transition: color 150ms cubic-bezier(0.86,0,0.07,1); -} - -.c4:hover { - background: #F5F6F7; -} - -.c4[aria-current='true'] { - background: #DEE1E5; - font-weight: 600; -} - -.c4[disabled] { - color: #939BA5; -} - -.c4[disabled] button, -.c4[disabled] a { - cursor: not-allowed; -} - -.c4[disabled]:hover { - background: #FFFFFF; - color: #939BA5; -} - -.c5 .Icon-sc-7y0t4i-0 { - -webkit-align-self: center; - -ms-flex-item-align: center; - align-self: center; -} - -
  • -
    -
    -

    - Questions -

    -
    -
      -
    • - -
    • -
    • - -
    • -
    • - -
    • -
    -
  • -`; - -exports[`MenuGroup 1`] = ` -.c0 { - font-family: 'Roboto','Noto Sans JP','Noto Sans CJK KR','Noto Sans Arabic UI','Noto Sans Devanagari UI','Noto Sans Hebrew','Noto Sans Thai UI','Helvetica','Arial',sans-serif; - list-style-type: none; - padding: 0.5rem 0; -} - -.c1 { - -webkit-align-items: center; - -webkit-box-align: center; - -ms-flex-align: center; - align-items: center; - color: #262D33; - font-size: 0.875rem; - font-weight: 400; - list-style-type: none; - outline: none; - -webkit-text-decoration: none; - text-decoration: none; - -webkit-transition: background 150ms cubic-bezier(0.86,0,0.07,1), color 150ms cubic-bezier(0.86,0,0.07,1); - transition: background 150ms cubic-bezier(0.86,0,0.07,1), color 150ms cubic-bezier(0.86,0,0.07,1); -} - -.c1 button, -.c1 a { - font-family: 'Roboto','Noto Sans JP','Noto Sans CJK KR','Noto Sans Arabic UI','Noto Sans Devanagari UI','Noto Sans Hebrew','Noto Sans Thai UI','Helvetica','Arial',sans-serif; - -webkit-align-items: center; - -webkit-box-align: center; - -ms-flex-align: center; - align-items: center; - background: transparent; - border: none; - color: inherit; - cursor: pointer; - display: -webkit-box; - display: -webkit-flex; - display: -ms-flexbox; - display: flex; - -webkit-flex: 1; - -ms-flex: 1; - flex: 1; - font-size: inherit; - font-weight: inherit; - min-height: 40px; - outline: none; - padding: 0.5rem 1rem; - text-align: left; - -webkit-text-decoration: none; - text-decoration: none; - width: 100%; -} - -.c1 button:hover, -.c1 a:hover, -.c1 button:focus, -.c1 a:focus { - color: inherit; - position: relative; - -webkit-text-decoration: none; - text-decoration: none; -} - -.c1 .Icon-sc-7y0t4i-0 { - -webkit-transition: color 150ms cubic-bezier(0.86,0,0.07,1); - transition: color 150ms cubic-bezier(0.86,0,0.07,1); -} - -.c1:hover { - background: #F5F6F7; -} - -.c1[aria-current='true'] { - background: #DEE1E5; - font-weight: 600; -} - -.c1[disabled] { - color: #939BA5; -} - -.c1[disabled] button, -.c1[disabled] a { - cursor: not-allowed; -} - -.c1[disabled]:hover { - background: #FFFFFF; - color: #939BA5; -} - -.c2 .Icon-sc-7y0t4i-0 { - -webkit-align-self: center; - -ms-flex-item-align: center; - align-self: center; -} - -
  • -
      -
    • - -
    • -
    -
  • -`; diff --git a/packages/components/src/Tabs/Tab.tsx b/packages/components/src/Tabs/Tab.tsx index c85da413a19..f5f7771878f 100644 --- a/packages/components/src/Tabs/Tab.tsx +++ b/packages/components/src/Tabs/Tab.tsx @@ -24,7 +24,7 @@ */ -import React, { forwardRef, Ref, useContext, useState } from 'react' +import React, { forwardRef, Ref, useState } from 'react' import styled from 'styled-components' import { CompatibleHTMLProps, @@ -38,7 +38,6 @@ import { TypographyProps, tabShadowColor, } from '@looker/design-tokens' -import { TabContext } from './TabContext' export interface TabProps extends Omit, 'type'>, @@ -104,7 +103,6 @@ const TabJSX = forwardRef((props: TabProps, ref: Ref) => { disabled, index, onBlur, - onKeyDown, onKeyUp, onSelect, selected, @@ -113,25 +111,11 @@ const TabJSX = forwardRef((props: TabProps, ref: Ref) => { const [isFocusVisible, setFocusVisible] = useState(false) - const { handleArrowLeft, handleArrowRight } = useContext(TabContext) - const handleOnKeyUp = (event: React.KeyboardEvent) => { setFocusVisible(true) onKeyUp && onKeyUp(event) } - const handleOnKeyDown = (event: React.KeyboardEvent) => { - switch (event.key) { - case 'ArrowLeft': - handleArrowLeft && handleArrowLeft(event) - break - case 'ArrowRight': - handleArrowRight && handleArrowRight(event) - break - } - onKeyDown && onKeyDown(event) - } - const handleOnBlur = (event: React.FocusEvent) => { setFocusVisible(false) onBlur && onBlur(event) @@ -153,13 +137,12 @@ const TabJSX = forwardRef((props: TabProps, ref: Ref) => { focusVisible={isFocusVisible} id={`tab-${index}`} onBlur={handleOnBlur} - onKeyDown={handleOnKeyDown} onClick={onClick} onKeyUp={handleOnKeyUp} ref={ref} role="tab" selected={selected} - tabIndex={selected ? 0 : -1} + tabIndex={-1} {...restProps} > {children} diff --git a/packages/components/src/Tabs/TabContext.ts b/packages/components/src/Tabs/TabContext.ts deleted file mode 100644 index 29c398298c2..00000000000 --- a/packages/components/src/Tabs/TabContext.ts +++ /dev/null @@ -1,34 +0,0 @@ -/* - - MIT License - - Copyright (c) 2020 Looker Data Sciences, Inc. - - Permission is hereby granted, free of charge, to any person obtaining a copy - of this software and associated documentation files (the "Software"), to deal - in the Software without restriction, including without limitation the rights - to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - copies of the Software, and to permit persons to whom the Software is - furnished to do so, subject to the following conditions: - - The above copyright notice and this permission notice shall be included in all - copies or substantial portions of the Software. - - THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE - SOFTWARE. - - */ - -import { createContext, KeyboardEvent } from 'react' - -export interface TabContextProps { - handleArrowLeft?: (e: KeyboardEvent) => void - handleArrowRight?: (e: KeyboardEvent) => void -} - -export const TabContext = createContext({}) diff --git a/packages/components/src/Tabs/TabList.tsx b/packages/components/src/Tabs/TabList.tsx index bce9752097c..819c4efe6ae 100644 --- a/packages/components/src/Tabs/TabList.tsx +++ b/packages/components/src/Tabs/TabList.tsx @@ -24,14 +24,7 @@ */ -import React, { - Children, - cloneElement, - forwardRef, - KeyboardEvent, - useRef, - Ref, -} from 'react' +import React, { Children, cloneElement, forwardRef, Ref } from 'react' import { fontSize, FontSizeProps, @@ -40,8 +33,7 @@ import { reset, } from '@looker/design-tokens' import styled, { css } from 'styled-components' -import { moveFocus, useForkedRef } from '../utils' -import { TabContext } from './TabContext' +import { useArrowKeyNav } from '../utils' import { Tab } from '.' export interface TabListProps extends PaddingProps, FontSizeProps { @@ -57,9 +49,6 @@ const TabListLayout = forwardRef( { children, selectedIndex, onSelectTab, className }: TabListProps, ref: Ref ) => { - const wrapperRef = useRef(null) - const forkedRef = useForkedRef(wrapperRef, ref) - const clonedChildren = Children.map( children, (child: JSX.Element, index: number) => { @@ -72,34 +61,12 @@ const TabListLayout = forwardRef( } ) - function handleArrowKey(direction: number, initial: number) { - moveFocus(direction, initial, wrapperRef.current) - } - - const context = { - handleArrowLeft: (e: KeyboardEvent) => { - e.preventDefault() - handleArrowKey(-1, -1) - return false - }, - handleArrowRight: (e: KeyboardEvent) => { - e.preventDefault() - handleArrowKey(1, 0) - return false - }, - } + const navProps = useArrowKeyNav({ axis: 'horizontal', ref }) return ( - -
    - {clonedChildren} -
    -
    +
    + {clonedChildren} +
    ) } ) diff --git a/packages/components/src/Tabs/Tabs.test.tsx b/packages/components/src/Tabs/Tabs.test.tsx index 3727698c201..0a5b2f5d023 100644 --- a/packages/components/src/Tabs/Tabs.test.tsx +++ b/packages/components/src/Tabs/Tabs.test.tsx @@ -27,7 +27,6 @@ import 'jest-styled-components' import '@testing-library/jest-dom/extend-expect' import { - assertSnapshotShallow, mountWithTheme, renderWithTheme, shallowWithTheme, @@ -40,23 +39,6 @@ import { TabPanel } from './TabPanel' import { TabPanels } from './TabPanels' import { Tabs, useTabs } from './Tabs' -test('Tabs snapshot works as expected', () => { - assertSnapshotShallow( - - - tab1 - tab2 - tab3 - - - this is tab1 content - this is tab2 content - this is tab3 content - - - ) -}) - test('shows the correct number of navigation tabs', () => { const tabs = shallowWithTheme( diff --git a/packages/components/src/Tabs/__snapshots__/Tabs.test.tsx.snap b/packages/components/src/Tabs/__snapshots__/Tabs.test.tsx.snap index a130cacc103..3248e17cb9d 100644 --- a/packages/components/src/Tabs/__snapshots__/Tabs.test.tsx.snap +++ b/packages/components/src/Tabs/__snapshots__/Tabs.test.tsx.snap @@ -1,7 +1,5 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Tabs snapshot works as expected 1`] = `ShallowWrapper {}`; - exports[`focus behavior Tab Focus: does not render focus ring after click 1`] = ` .c0 { font-family: 'Roboto','Noto Sans JP','Noto Sans CJK KR','Noto Sans Arabic UI','Noto Sans Devanagari UI','Noto Sans Hebrew','Noto Sans Thai UI','Helvetica','Arial',sans-serif; @@ -50,7 +48,7 @@ exports[`focus behavior Tab Focus: does not render focus ring after click 1`] = class="c0 Tab-eojndt-1" id="tab-0" role="tab" - tabindex="0" + tabindex="-1" > tab1 diff --git a/packages/components/src/utils/moveFocus.ts b/packages/components/src/utils/getNextFocus.ts similarity index 58% rename from packages/components/src/utils/moveFocus.ts rename to packages/components/src/utils/getNextFocus.ts index a6692487cac..45197d3ba2e 100644 --- a/packages/components/src/utils/moveFocus.ts +++ b/packages/components/src/utils/getNextFocus.ts @@ -24,30 +24,39 @@ */ -const getTabStops = (ref: HTMLElement): HTMLElement[] => - Array.from(ref.querySelectorAll('a,button:not(:disabled),[tabindex="0"]')) - -export const moveFocus = ( - direction: number, - initial: number, - element?: HTMLElement | null -) => { - if (element) { - const tabStops = getTabStops(element) +export const getTabStops = (ref: HTMLElement): HTMLElement[] => + Array.from( + ref.querySelectorAll( + 'a,button:not(:disabled),[tabindex="0"],[tabindex="-1"]:not(:disabled)' + ) + ) + +/** + * Returns the next focusable inside an element in a given direction + * @param direction 1 for forward -1 for reverse + * @param element the container element + */ +export const getNextFocus = (direction: 1 | -1, element: HTMLElement) => { + const tabStops = getTabStops(element) + if (tabStops.length > 0) { + const fallback = + direction === 1 ? tabStops[0] : tabStops[tabStops.length - 1] if ( document.activeElement && tabStops.includes(document.activeElement as HTMLElement) ) { const next = - tabStops.findIndex((f) => f === document.activeElement) + direction + tabStops.findIndex((el) => el === document.activeElement) + direction + + if (next === tabStops.length || !tabStops[next]) { + // Reached the end of tab stops for this direction + return fallback + } - if (next === tabStops.length) return - if (!tabStops[next]) return - tabStops[next].focus() - } else { - tabStops.slice(initial)[0].focus() + return tabStops[next] } + return fallback } - return false + return null } diff --git a/packages/components/src/utils/index.ts b/packages/components/src/utils/index.ts index 68fabde1f7e..2bc1e1ed13b 100644 --- a/packages/components/src/utils/index.ts +++ b/packages/components/src/utils/index.ts @@ -28,11 +28,12 @@ export * from './getNextFocusTarget' export * from './getWindowedListBoundaries' export * from './HoverDisclosure' export * from './mergeHandlers' -export * from './moveFocus' +export * from './getNextFocus' export * from './targetIsButton' export * from './undefinedCoalesce' export * from './useAnimationState' export * from './useClickable' +export * from './useArrowKeyNav' export * from './useControlWarn' export * from './useReadOnlyWarn' export * from './useCallbackRef' diff --git a/packages/components/src/utils/useArrowKeyNav.test.tsx b/packages/components/src/utils/useArrowKeyNav.test.tsx new file mode 100644 index 00000000000..11f3a683be2 --- /dev/null +++ b/packages/components/src/utils/useArrowKeyNav.test.tsx @@ -0,0 +1,155 @@ +/* + + MIT License + + Copyright (c) 2020 Looker Data Sciences, Inc. + + Permission is hereby granted, free of charge, to any person obtaining a copy + of this software and associated documentation files (the "Software"), to deal + in the Software without restriction, including without limitation the rights + to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice shall be included in all + copies or substantial portions of the Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + SOFTWARE. + + */ + +import { render, screen } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import React from 'react' +import { useArrowKeyNav, UseArrowKeyNavProps } from './useArrowKeyNav' + +const ArrowKeyNavComponent = ({ + axis, +}: { + axis?: UseArrowKeyNavProps['axis'] +}) => { + const navProps = useArrowKeyNav({ axis }) + return ( +
      +
    • first
    • +
    • second
    • +
    • third
    • +
    + ) +} + +describe('useArrowKeyNav', () => { + test('tabbing', () => { + render( + <> + + + + + ) + const before = screen.getByText('before') + const first = screen.getByText('first') + + userEvent.click(before) + userEvent.tab() + expect(first).toHaveFocus() + + // second and third are skipped due to tabIndex={-1} + userEvent.tab() + expect(screen.getByText('after')).toHaveFocus() + + userEvent.tab({ shift: true }) + expect(first).toHaveFocus() + + userEvent.tab({ shift: true }) + expect(before).toHaveFocus() + }) + + test('up/down arrow keys', () => { + render( + <> + + + + + ) + const before = screen.getByText('before') + const first = screen.getByText('first') + const second = screen.getByText('second') + const third = screen.getByText('third') + + userEvent.click(before) + userEvent.tab() + + userEvent.type(first, '{arrowdown}') + expect(second).toHaveFocus() + + userEvent.type(second, '{arrowdown}') + expect(third).toHaveFocus() + + // circles back + userEvent.type(third, '{arrowdown}') + expect(first).toHaveFocus() + + // circles back in reverse + userEvent.type(first, '{arrowup}') + expect(third).toHaveFocus() + + userEvent.type(third, '{arrowup}') + expect(second).toHaveFocus() + + userEvent.tab({ shift: true }) + expect(before).toHaveFocus() + + // Previous focus item is persisted + userEvent.tab() + expect(second).toHaveFocus() + }) + + test('left/right arrow keys', () => { + render( + <> + + + + + ) + const before = screen.getByText('before') + const first = screen.getByText('first') + const second = screen.getByText('second') + const third = screen.getByText('third') + + userEvent.click(before) + userEvent.tab() + + userEvent.type(first, '{arrowright}') + expect(second).toHaveFocus() + + userEvent.type(second, '{arrowright}') + expect(third).toHaveFocus() + + // circles back + userEvent.type(third, '{arrowright}') + expect(first).toHaveFocus() + + // circles back in reverse + userEvent.type(first, '{arrowleft}') + expect(third).toHaveFocus() + + userEvent.type(third, '{arrowleft}') + expect(second).toHaveFocus() + + userEvent.tab({ shift: true }) + expect(before).toHaveFocus() + + // Previous focus item is persisted + userEvent.tab() + expect(second).toHaveFocus() + }) +}) diff --git a/packages/components/src/utils/useArrowKeyNav.ts b/packages/components/src/utils/useArrowKeyNav.ts new file mode 100644 index 00000000000..f8f2c8766c0 --- /dev/null +++ b/packages/components/src/utils/useArrowKeyNav.ts @@ -0,0 +1,146 @@ +/* + + MIT License + + Copyright (c) 2020 Looker Data Sciences, Inc. + + Permission is hereby granted, free of charge, to any person obtaining a copy + of this software and associated documentation files (the "Software"), to deal + in the Software without restriction, including without limitation the rights + to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice shall be included in all + copies or substantial portions of the Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + SOFTWARE. + + */ + +import { FocusEvent, KeyboardEvent, Ref, useRef, useState } from 'react' +import { getNextFocus as getNextFocusDefault } from './getNextFocus' +import { useForkedRef } from './useForkedRef' +import { useWrapEvent } from './useWrapEvent' + +export interface UseArrowKeyNavProps { + /** + * vertical for up/down arrow keys, horizontal for left/right, both for all (grid) + * @default vertical + */ + axis?: 'vertical' | 'horizontal' | 'both' + /** + * A custom getter for the next item to focus + */ + getNextFocus?: ( + direction: 1 | -1, + element: E, + vertical?: boolean + ) => HTMLElement | null + /** + * will be merged with the ref in the return + */ + ref?: Ref + /** + * will be merged with the onBlur in the return + */ + onBlur?: (e: FocusEvent) => void + /** + * will be merged with the onFocus in the return + */ + onFocus?: (e: FocusEvent) => void + /** + * will be merged with the onKeyDown in the return + */ + onKeyDown?: (e: KeyboardEvent) => void +} + +/** + * Returns props to spread onto container element for arrow key navigation. + * Add tabIndex={-1} to child elements. + */ +export const useArrowKeyNav = ({ + axis = 'vertical', + getNextFocus = getNextFocusDefault, + ref, + onBlur, + onFocus, + onKeyDown, +}: UseArrowKeyNavProps) => { + const internalRef = useRef(null) + const [focusedItem, setFocusedItem] = useState(null) + const [focusInside, setFocusInside] = useState(false) + + const handleArrowKey = ( + e: KeyboardEvent, + direction: 1 | -1, + vertical: boolean + ) => { + if (internalRef.current) { + const newFocusedItem = getNextFocus( + direction, + internalRef.current, + vertical + ) + if (newFocusedItem) { + e.preventDefault() + newFocusedItem.focus() + setFocusedItem(newFocusedItem) + } + } + } + + const handleKeyDown = (e: KeyboardEvent) => { + switch (e.key) { + case 'ArrowUp': + axis !== 'horizontal' && handleArrowKey(e, -1, true) + break + case 'ArrowDown': + axis !== 'horizontal' && handleArrowKey(e, 1, true) + break + case 'ArrowLeft': + axis !== 'vertical' && handleArrowKey(e, -1, false) + break + case 'ArrowRight': + axis !== 'vertical' && handleArrowKey(e, 1, false) + break + } + } + + const handleFocus = (e: FocusEvent) => { + setFocusInside(true) + // When focus lands on the container + if (e.target === internalRef.current) { + // Check if there's a previously focused item that is still rendered + if (focusedItem && internalRef.current.contains(focusedItem)) { + focusedItem.focus() + } else { + const toFocus = getNextFocus(1, internalRef.current) + if (toFocus) { + // No need to update focusedItem with this since it's the default + toFocus.focus() + } + } + } + } + + const handleBlur = () => { + setFocusInside(false) + } + + return { + onBlur: useWrapEvent(handleBlur, onBlur), + onFocus: useWrapEvent(handleFocus, onFocus), + onKeyDown: useWrapEvent(handleKeyDown, onKeyDown), + ref: useForkedRef(internalRef, ref), + // Remove tabIndex from container if focus is inside to prevent focus from + // landing back on the container when shift-tabbing from the first item + tabIndex: focusInside ? undefined : 0, + } +} diff --git a/packages/components/src/utils/useWindow.tsx b/packages/components/src/utils/useWindow.tsx index c6cebbfb33b..7847ae23dbc 100644 --- a/packages/components/src/utils/useWindow.tsx +++ b/packages/components/src/utils/useWindow.tsx @@ -28,6 +28,7 @@ import React, { Children, ReactChild, Reducer, + Ref, useEffect, useMemo, useReducer, @@ -146,21 +147,23 @@ export type ChildHeightFunction = (child: ReactChild, index: number) => number export type WindowSpacerTag = 'div' | 'li' | 'tr' -export interface UseWindowProps { +export interface UseWindowProps { enabled?: boolean children?: JSX.Element | JSX.Element[] /** Derive the height of each child using props, type, etc. */ childHeight: number | ChildHeightFunction /** Tagname to use for the spacers above and below the window */ spacerTag?: WindowSpacerTag + ref?: Ref } -export const useWindow = ({ +export const useWindow = ({ children, enabled, childHeight, + ref, spacerTag = 'div', -}: UseWindowProps) => { +}: UseWindowProps) => { const childArray = useMemo(() => Children.toArray(children), [children]) const [totalHeight, childHeightLadder] = useMemo(() => { @@ -175,7 +178,7 @@ export const useWindow = ({ return [sum, ladder] }, [childHeight, childArray]) - const [containerElement, ref] = useCallbackRef() + const [containerElement, callbackRef] = useCallbackRef(ref) const { height } = useMeasuredElement(containerElement) const scrollPosition = useScrollPosition(containerElement) @@ -240,6 +243,6 @@ export const useWindow = ({ {after} ), - ref, + ref: callbackRef, } } diff --git a/playground/src/index.tsx b/playground/src/index.tsx index a7e2a0bd1d2..e999d92f9b7 100644 --- a/playground/src/index.tsx +++ b/playground/src/index.tsx @@ -26,12 +26,12 @@ import React from 'react' import { render } from 'react-dom' import { ComponentsProvider } from '@looker/components' -import { Basic } from '@looker/components/src/Menu/Menu.story' +import { LongMenus } from '@looker/components/src/Menu/Menu.story' const App = () => { return ( - + ) }