diff --git a/frontend/src/metabase/hooks/use-list-keyboard-navigation/index.ts b/frontend/src/metabase/hooks/use-list-keyboard-navigation/index.ts new file mode 100644 index 0000000000000000000000000000000000000000..956715617685881e41736f5b746222c027b163dc --- /dev/null +++ b/frontend/src/metabase/hooks/use-list-keyboard-navigation/index.ts @@ -0,0 +1 @@ +export * from "./use-list-keyboard-navigation"; diff --git a/frontend/src/metabase/hooks/use-list-keyboard-navigation/use-list-keyboard-navigation.ts b/frontend/src/metabase/hooks/use-list-keyboard-navigation/use-list-keyboard-navigation.ts new file mode 100644 index 0000000000000000000000000000000000000000..dd7aa29d0a060bcb23be246cfa695a87c559b4a4 --- /dev/null +++ b/frontend/src/metabase/hooks/use-list-keyboard-navigation/use-list-keyboard-navigation.ts @@ -0,0 +1,66 @@ +import React, { useCallback, useEffect, useRef, useState } from "react"; + +interface ListKeyboardNavigationInput<T> { + ref?: React.MutableRefObject<any>; + list: T[]; + onEnter: (item: T) => void; + resetOnListChange?: boolean; +} + +export const useListKeyboardNavigation = <T>({ + ref, + list, + onEnter, + resetOnListChange = true, +}: ListKeyboardNavigationInput<T>) => { + const selectedRef = useRef<HTMLElement | null>(); + const [cursorIndex, setCursorIndex] = useState<number | null>(null); + + const reset = useCallback(() => setCursorIndex(null), []); + + useEffect(() => { + if (resetOnListChange) { + reset(); + } + }, [list, reset, resetOnListChange]); + + useEffect(() => { + selectedRef?.current?.scrollIntoView({ + block: "nearest", + }); + }, [cursorIndex]); + + const handleKeyDown = useCallback( + ({ key }: KeyboardEvent) => { + switch (key) { + case "ArrowDown": + setCursorIndex(((cursorIndex ?? -1) + 1) % list.length); + break; + case "ArrowUp": + setCursorIndex((list.length + (cursorIndex ?? 0) - 1) % list.length); + break; + case "Enter": + if (cursorIndex != null) { + onEnter(list[cursorIndex]); + } + break; + } + }, + [cursorIndex, list, onEnter], + ); + + useEffect(() => { + const element = ref?.current ?? window; + element.addEventListener("keydown", handleKeyDown); + return () => element.removeEventListener("keydown", handleKeyDown); + }, [handleKeyDown, ref]); + + const selectedItem = cursorIndex != null ? list?.[cursorIndex] : null; + + return { + reset, + cursorIndex, + selectedItem, + getRef: (item: T) => (item === selectedItem ? selectedRef : undefined), + }; +}; diff --git a/frontend/src/metabase/hooks/use-list-keyboard-navigation/use-list-keyboard-navigation.unit.spec.ts b/frontend/src/metabase/hooks/use-list-keyboard-navigation/use-list-keyboard-navigation.unit.spec.ts new file mode 100644 index 0000000000000000000000000000000000000000..18d70a9ca59e63091d50a26953c6cbadcd840ccc --- /dev/null +++ b/frontend/src/metabase/hooks/use-list-keyboard-navigation/use-list-keyboard-navigation.unit.spec.ts @@ -0,0 +1,112 @@ +import { fireEvent } from "@testing-library/react"; +import { renderHook, act } from "@testing-library/react-hooks"; + +import { useListKeyboardNavigation } from "./use-list-keyboard-navigation"; + +const list = [ + { id: 1, name: "first" }, + { id: 2, name: "second" }, + { id: 3, name: "third" }, +]; + +const assertItemWithIndexSelected = ( + index: number, + result: ReturnType<typeof useListKeyboardNavigation>, +) => { + const { selectedItem, getRef, cursorIndex } = result; + expect(selectedItem).toBe(list[index]); + expect(cursorIndex).toBe(index); + + list.forEach((item, i) => { + if (index === i) { + expect(getRef(item)).not.toBeUndefined(); + } else { + expect(getRef(item)).toBeUndefined(); + } + }); +}; + +const fireKey = (key: string) => + act(() => { + fireEvent.keyDown(window, { key }); + }); + +describe("useListKeyboardNavigation", () => { + it("no selected item initially", () => { + const { result } = renderHook(() => + useListKeyboardNavigation<unknown>({ list, onEnter: jest.fn() }), + ); + const { selectedItem, getRef } = result.current; + + expect(selectedItem).toBeNull(); + expect(list.some(getRef)).toBeFalsy(); + }); + + it("navigates list downwards", () => { + const { result } = renderHook(() => + useListKeyboardNavigation<unknown>({ list, onEnter: jest.fn() }), + ); + + fireKey("ArrowDown"); + assertItemWithIndexSelected(0, result.current); + + fireKey("ArrowDown"); + assertItemWithIndexSelected(1, result.current); + + fireKey("ArrowDown"); + assertItemWithIndexSelected(2, result.current); + + fireKey("ArrowDown"); + assertItemWithIndexSelected(0, result.current); + }); + + it("navigates list upwards", () => { + const { result } = renderHook(() => + useListKeyboardNavigation<unknown>({ list, onEnter: jest.fn() }), + ); + + fireKey("ArrowUp"); + assertItemWithIndexSelected(2, result.current); + + fireKey("ArrowUp"); + assertItemWithIndexSelected(1, result.current); + + fireKey("ArrowUp"); + assertItemWithIndexSelected(0, result.current); + + fireKey("ArrowUp"); + assertItemWithIndexSelected(2, result.current); + }); + + it("navigates mixed upwards and downwards", () => { + const { result } = renderHook(() => + useListKeyboardNavigation<unknown>({ list, onEnter: jest.fn() }), + ); + + fireKey("ArrowUp"); + assertItemWithIndexSelected(2, result.current); + + fireKey("ArrowDown"); + assertItemWithIndexSelected(0, result.current); + }); + + it("calls onEnter when Enter pressed", () => { + const onEnterMock = jest.fn(); + const { unmount } = renderHook(() => + useListKeyboardNavigation({ list, onEnter: onEnterMock }), + ); + + fireKey("Enter"); + expect(onEnterMock).not.toHaveBeenCalled(); + + fireKey("ArrowDown"); + fireKey("Enter"); + expect(onEnterMock).toHaveBeenCalledWith(list[0]); + onEnterMock.mockClear(); + + unmount(); + + fireKey("Enter"); + expect(onEnterMock).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/metabase/nav/components/RecentsList.jsx b/frontend/src/metabase/nav/components/RecentsList.jsx index ba068aad7fbbabd2a68bb25de1bda9da4292ec14..8d309f7284f931df41010a5831f0dd7eb74e5f66 100644 --- a/frontend/src/metabase/nav/components/RecentsList.jsx +++ b/frontend/src/metabase/nav/components/RecentsList.jsx @@ -2,6 +2,8 @@ import React, { useState, useEffect } from "react"; import PropTypes from "prop-types"; import { t } from "ttag"; import _ from "underscore"; +import { connect } from "react-redux"; +import { push } from "react-router-redux"; import RecentItems from "metabase/entities/recent-items"; import Text from "metabase/components/type/Text"; @@ -17,6 +19,7 @@ import { import { ItemIcon } from "metabase/search/components/SearchResult"; import EmptyState from "metabase/components/EmptyState"; import LoadingAndErrorWrapper from "metabase/components/LoadingAndErrorWrapper"; +import { useListKeyboardNavigation } from "metabase/hooks/use-list-keyboard-navigation"; import { getTranslatedEntityName } from "../utils"; import { @@ -37,9 +40,16 @@ const propTypes = { }), ), loading: PropTypes.bool, + onChangeLocation: PropTypes.func, }; -function RecentsList({ list, loading }) { +const getItemUrl = item => (isItemActive(item) ? Urls.modelToUrl(item) : ""); + +function RecentsList({ list, loading, onChangeLocation }) { + const { getRef, cursorIndex } = useListKeyboardNavigation({ + list, + onEnter: item => onChangeLocation(getItemUrl(item)), + }); const [canShowLoader, setCanShowLoader] = useState(false); const hasRecents = list?.length > 0; @@ -59,18 +69,23 @@ function RecentsList({ list, loading }) { <React.Fragment> {hasRecents && ( <ul> - {list.map(item => { + {list.map((item, index) => { const key = getItemKey(item); const title = getItemName(item); const type = getTranslatedEntityName(item.model); const active = isItemActive(item); const loading = isItemLoading(item); - const url = active ? Urls.modelToUrl(item) : ""; + const url = getItemUrl(item); const moderatedStatus = getModeratedStatus(item); return ( - <li key={key}> - <ResultLink to={url} compact={true} active={active}> + <li key={key} ref={getRef(item)}> + <ResultLink + to={url} + compact={true} + active={active} + isSelected={cursorIndex === index} + > <RecentListItemContent align="start" data-testid="recently-viewed-item" @@ -151,6 +166,9 @@ const isItemLoading = ({ model, model_object }) => { }; export default _.compose( + connect(null, { + onChangeLocation: push, + }), RecentItems.loadList({ wrapped: true, reload: true, diff --git a/frontend/src/metabase/nav/components/SearchBar.tsx b/frontend/src/metabase/nav/components/SearchBar.tsx index 9906a592c401ac3884939fb6ecec4f17f66d6623..e75425920cc167186a7e5c8e616ddaf74f85e3ca 100644 --- a/frontend/src/metabase/nav/components/SearchBar.tsx +++ b/frontend/src/metabase/nav/components/SearchBar.tsx @@ -21,7 +21,7 @@ import { useToggle } from "metabase/hooks/use-toggle"; import { isSmallScreen } from "metabase/lib/dom"; import MetabaseSettings from "metabase/lib/settings"; -import { SearchResults } from "./SearchResults"; +import SearchResults from "./SearchResults"; import RecentsList from "./RecentsList"; import { SearchInputContainer, diff --git a/frontend/src/metabase/nav/components/SearchResults.jsx b/frontend/src/metabase/nav/components/SearchResults.jsx index 7bad9701eebddc28f7935d350688acc9dc21b2aa..7562a0097f54c5fce7dc0aff1df20e592f25c9dd 100644 --- a/frontend/src/metabase/nav/components/SearchResults.jsx +++ b/frontend/src/metabase/nav/components/SearchResults.jsx @@ -1,46 +1,70 @@ -import React from "react"; +import React, { useEffect } from "react"; import PropTypes from "prop-types"; import { t } from "ttag"; +import { connect } from "react-redux"; +import { push } from "react-router-redux"; +import _ from "underscore"; import { DEFAULT_SEARCH_LIMIT } from "metabase/lib/constants"; import Search from "metabase/entities/search"; import SearchResult from "metabase/search/components/SearchResult"; import EmptyState from "metabase/components/EmptyState"; import { EmptyStateContainer } from "./SearchResults.styled"; +import { useListKeyboardNavigation } from "metabase/hooks/use-list-keyboard-navigation"; const propTypes = { + list: PropTypes.array, + onChangeLocation: PropTypes.func, searchText: PropTypes.string, }; -export const SearchResults = ({ searchText }) => { +const SearchResults = ({ list, onChangeLocation, searchText }) => { + const { reset, getRef, cursorIndex } = useListKeyboardNavigation({ + list, + onEnter: item => onChangeLocation(item.getUrl()), + resetOnListChange: false, + }); + + useEffect(() => { + reset(); + }, [searchText, reset]); + + const hasResults = list.length > 0; + return ( - <Search.ListLoader - query={{ q: searchText, limit: DEFAULT_SEARCH_LIMIT }} - wrapped - reload - debounced - > - {({ list }) => { - const hasResults = list.length > 0; - - return ( - <ul data-testid="search-results-list"> - {hasResults ? ( - list.map(item => ( - <li key={`${item.model}:${item.id}`}> - <SearchResult result={item} compact={true} /> - </li> - )) - ) : ( - <EmptyStateContainer> - <EmptyState message={t`Didn't find anything`} icon="search" /> - </EmptyStateContainer> - )} - </ul> - ); - }} - </Search.ListLoader> + <ul data-testid="search-results-list"> + {hasResults ? ( + list.map((item, index) => ( + <li key={`${item.model}:${item.id}`} ref={getRef(item)}> + <SearchResult + result={item} + compact={true} + isSelected={cursorIndex === index} + /> + </li> + )) + ) : ( + <EmptyStateContainer> + <EmptyState message={t`Didn't find anything`} icon="search" /> + </EmptyStateContainer> + )} + </ul> ); }; SearchResults.propTypes = propTypes; + +export default _.compose( + connect(null, { + onChangeLocation: push, + }), + Search.loadList({ + wrapped: true, + reload: true, + debounced: true, + query: (_state, props) => ({ + q: props.searchText, + limit: DEFAULT_SEARCH_LIMIT, + }), + }), +)(SearchResults); diff --git a/frontend/src/metabase/search/components/SearchResult.jsx b/frontend/src/metabase/search/components/SearchResult.jsx index 253304f22daadf19c8201f8253fba3335e6ced25..10513ad15e990b0781fd9a5b50104b56a329f639 100644 --- a/frontend/src/metabase/search/components/SearchResult.jsx +++ b/frontend/src/metabase/search/components/SearchResult.jsx @@ -94,12 +94,14 @@ export default function SearchResult({ compact, hasDescription = true, onClick, + isSelected, }) { const active = isItemActive(result); const loading = isItemLoading(result); return ( <ResultLink + isSelected={isSelected} active={active} compact={compact} to={!onClick ? result.getUrl() : ""} diff --git a/frontend/src/metabase/search/components/SearchResult.styled.jsx b/frontend/src/metabase/search/components/SearchResult.styled.jsx index 383b13af23a8347fd1ce94802d05930b351c5164..a69cc29eb2a410f4267c63ce2fae704c2a28affc 100644 --- a/frontend/src/metabase/search/components/SearchResult.styled.jsx +++ b/frontend/src/metabase/search/components/SearchResult.styled.jsx @@ -48,7 +48,8 @@ export const Title = styled("h3")` export const ResultLink = styled(Link)` display: block; - background-color: transparent; + background-color: ${props => + props.isSelected ? lighten("brand", 0.63) : "transparent"}; min-height: ${props => (props.compact ? "36px" : "54px")}; padding-top: ${space(1)}; padding-bottom: ${space(1)}; @@ -60,7 +61,8 @@ export const ResultLink = styled(Link)` background-color: ${props => (props.acitve ? lighten("brand", 0.63) : "")}; h3 { - color: ${props => (props.active ? color("brand") : "")}; + color: ${props => + props.active || props.isSelected ? color("brand") : ""}; } } @@ -87,6 +89,7 @@ export const ResultLink = styled(Link)` line-height: 1.2em; word-wrap: break-word; margin-bottom: 0; + color: ${props => (props.active && props.isSelected ? color("brand") : "")}; } .Icon-info { diff --git a/frontend/test/metabase/scenarios/onboarding/search/recently-viewed.cy.spec.js b/frontend/test/metabase/scenarios/onboarding/search/recently-viewed.cy.spec.js index 75d3705b93ecbfcc03c7f101fe751dac0544ecdb..ce069a66818c8bf5372952577a0278e45b6e7a29 100644 --- a/frontend/test/metabase/scenarios/onboarding/search/recently-viewed.cy.spec.js +++ b/frontend/test/metabase/scenarios/onboarding/search/recently-viewed.cy.spec.js @@ -9,9 +9,6 @@ describe(`search > recently viewed`, () => { restore(); cy.signInAsAdmin(); cy.intercept("POST", "/api/dataset").as("dataset"); - }); - - it("shows list of recently viewed items", () => { cy.visit("/browse/1-sample-database"); // "People" table @@ -30,8 +27,10 @@ describe(`search > recently viewed`, () => { // which elicits a ViewLog entry cy.visit("/"); - cy.findByPlaceholderText("Search…").click(); + }); + + it("shows list of recently viewed items", () => { cy.findByTestId("loading-spinner").should("not.exist"); assertRecentlyViewedItem( @@ -43,6 +42,14 @@ describe(`search > recently viewed`, () => { assertRecentlyViewedItem(1, "Orders", "Question", "/question/1-orders"); assertRecentlyViewedItem(2, "People", "Table", "/question#?db=1&table=3"); }); + + it("allows to select an item from keyboard", () => { + cy.get("body").trigger("keydown", { key: "ArrowDown" }); + cy.get("body").trigger("keydown", { key: "ArrowDown" }); + cy.get("body").trigger("keydown", { key: "Enter" }); + + cy.url().should("match", /\/question\/1-orders$/); + }); }); const assertRecentlyViewedItem = (index, title, type, link) => { diff --git a/frontend/test/metabase/scenarios/onboarding/search/search.cy.spec.js b/frontend/test/metabase/scenarios/onboarding/search/search.cy.spec.js index 62fa13241faa8a70edb5d88938d15747ac594944..cbdc10c0476ee35e18846a8ead45cea069c2c428 100644 --- a/frontend/test/metabase/scenarios/onboarding/search/search.cy.spec.js +++ b/frontend/test/metabase/scenarios/onboarding/search/search.cy.spec.js @@ -37,6 +37,21 @@ describe("scenarios > auth > search", () => { cy.findByPlaceholderText("Search…").type("product{enter}"); cy.findByText("Didn't find anything"); }); + + it("allows select a search result from keyboard", () => { + cy.intercept("GET", "/api/search*").as("search"); + + cy.signInAsNormalUser(); + cy.visit("/"); + cy.findByPlaceholderText("Search…").type("ord"); + cy.wait("@search"); + + cy.get("body").trigger("keydown", { key: "ArrowDown" }); + cy.get("body").trigger("keydown", { key: "ArrowDown" }); + cy.get("body").trigger("keydown", { key: "Enter" }); + + cy.url().should("match", /\/question\/1-orders$/); + }); }); }); diff --git a/package.json b/package.json index 5f11ae3e8be693df4a0b7b3b6df31d17edbf5ecc..d5c3bd310f194006b4bdd87b7299b3f739224f1f 100644 --- a/package.json +++ b/package.json @@ -128,7 +128,7 @@ "@testing-library/dom": "^7.29.0", "@testing-library/jest-dom": "^4.0.0", "@testing-library/react": "^11.0.2", - "@testing-library/react-hooks": "^7.0.2", + "@testing-library/react-hooks": "^8.0.0", "@testing-library/user-event": "^12.6.0", "@types/babel__core": "^7.1.16", "@types/babel__preset-env": "^7.9.2", diff --git a/yarn.lock b/yarn.lock index 78a2135bed139b4c0b6f4cbfc7f91eec89a8eac8..3c343f8885efa42a3872387efcd104d4600d8ee5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4391,15 +4391,12 @@ pretty-format "^24.0.0" redent "^3.0.0" -"@testing-library/react-hooks@^7.0.2": - version "7.0.2" - resolved "https://registry.yarnpkg.com/@testing-library/react-hooks/-/react-hooks-7.0.2.tgz#3388d07f562d91e7f2431a4a21b5186062ecfee0" - integrity sha512-dYxpz8u9m4q1TuzfcUApqi8iFfR6R0FaMbr2hjZJy1uC8z+bO/K4v8Gs9eogGKYQop7QsrBTFkv/BCF7MzD2Cg== +"@testing-library/react-hooks@^8.0.0": + version "8.0.0" + resolved "https://registry.yarnpkg.com/@testing-library/react-hooks/-/react-hooks-8.0.0.tgz#7d0164bffce4647f506039de0a97f6fcbd20f4bf" + integrity sha512-uZqcgtcUUtw7Z9N32W13qQhVAD+Xki2hxbTR461MKax8T6Jr8nsUvZB+vcBTkzY2nFvsUet434CsgF0ncW2yFw== dependencies: "@babel/runtime" "^7.12.5" - "@types/react" ">=16.9.0" - "@types/react-dom" ">=16.9.0" - "@types/react-test-renderer" ">=16.9.0" react-error-boundary "^3.1.0" "@testing-library/react@^11.0.2": @@ -5105,13 +5102,6 @@ dependencies: "@types/react" "*" -"@types/react-dom@>=16.9.0": - version "17.0.11" - resolved "https://registry.yarnpkg.com/@types/react-dom/-/react-dom-17.0.11.tgz#e1eadc3c5e86bdb5f7684e00274ae228e7bcc466" - integrity sha512-f96K3k+24RaLGVu/Y2Ng3e1EbZ8/cVJvypZWd7cy0ofCBaf2lcM46xNhycMZ2xGwbBjRql7hOlZ+e2WlJ5MH3Q== - dependencies: - "@types/react" "*" - "@types/react-dom@~17.0.9": version "17.0.9" resolved "https://registry.yarnpkg.com/@types/react-dom/-/react-dom-17.0.9.tgz#441a981da9d7be117042e1a6fd3dac4b30f55add" @@ -5186,13 +5176,6 @@ dependencies: "@types/react" "*" -"@types/react-test-renderer@>=16.9.0": - version "17.0.1" - resolved "https://registry.yarnpkg.com/@types/react-test-renderer/-/react-test-renderer-17.0.1.tgz#3120f7d1c157fba9df0118dae20cb0297ee0e06b" - integrity sha512-3Fi2O6Zzq/f3QR9dRnlnHso9bMl7weKCviFmfF6B4LS1Uat6Hkm15k0ZAQuDz+UBq6B3+g+NM6IT2nr5QgPzCw== - dependencies: - "@types/react" "*" - "@types/react-textarea-autosize@^4.3.6": version "4.3.6" resolved "https://registry.yarnpkg.com/@types/react-textarea-autosize/-/react-textarea-autosize-4.3.6.tgz#f56f7b41aee9fb0310b6e32a8d2a77eb9a5893db" @@ -5224,15 +5207,6 @@ "@types/scheduler" "*" csstype "^3.0.2" -"@types/react@>=16.9.0": - version "17.0.39" - resolved "https://registry.yarnpkg.com/@types/react/-/react-17.0.39.tgz#d0f4cde092502a6db00a1cded6e6bf2abb7633ce" - integrity sha512-UVavlfAxDd/AgAacMa60Azl7ygyQNRwC/DsHZmKgNvPmRR5p70AJ5Q9EAmL2NWOJmeV+vVUI4IAP7GZrN8h8Ug== - dependencies: - "@types/prop-types" "*" - "@types/scheduler" "*" - csstype "^3.0.2" - "@types/react@~16.14.17": version "16.14.17" resolved "https://registry.yarnpkg.com/@types/react/-/react-16.14.17.tgz#c57fcfb05efa6423f5b65fcd4a75f63f05b162bf"