From 4eb735d552d2574e9285d286e71ab5c278a0b88e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarik=20G=C3=BCrsoy?= Date: Thu, 28 Sep 2023 16:16:17 +0200 Subject: [PATCH] Fixing the OmniSearch component The OmniSearch component had several issues which have been resolved in this fix. It now makes use of the new combobox component. --- gradle/changelog/omnisearch_refactor.yaml | 2 + .../src/combobox/Combobox.stories.tsx | 29 ++ scm-ui/ui-forms/src/combobox/Combobox.tsx | 11 +- scm-ui/ui-forms/src/index.ts | 1 + .../src/components/HeaderDropDown.tsx | 60 +-- .../src/containers/NavigationBar.tsx | 8 +- .../src/containers/Notifications.tsx | 9 +- .../ui-webapp/src/containers/OmniSearch.tsx | 383 +++++------------- scm-ui/ui-webapp/src/search/SyntaxHelp.tsx | 5 +- 9 files changed, 177 insertions(+), 331 deletions(-) create mode 100644 gradle/changelog/omnisearch_refactor.yaml diff --git a/gradle/changelog/omnisearch_refactor.yaml b/gradle/changelog/omnisearch_refactor.yaml new file mode 100644 index 0000000000..1e7651f43b --- /dev/null +++ b/gradle/changelog/omnisearch_refactor.yaml @@ -0,0 +1,2 @@ +- type: changed + description: OmniSearchbar now makes use of the Combobox diff --git a/scm-ui/ui-forms/src/combobox/Combobox.stories.tsx b/scm-ui/ui-forms/src/combobox/Combobox.stories.tsx index 9aef988efe..2191924bc0 100644 --- a/scm-ui/ui-forms/src/combobox/Combobox.stories.tsx +++ b/scm-ui/ui-forms/src/combobox/Combobox.stories.tsx @@ -27,6 +27,7 @@ import React, { Fragment, useState } from "react"; import Combobox from "./Combobox"; import { Combobox as HeadlessCombobox } from "@headlessui/react"; import { Option } from "@scm-manager/ui-types"; +import { Link, BrowserRouter } from "react-router-dom"; const waitFor = (ms: number) => function (result: T) { @@ -39,6 +40,8 @@ const data = [ { label: "Zaphod", value: "3" }, ]; +const linkData = [{ label: "Link111111111111111111111111111111111111", value: "1" }]; + storiesOf("Combobox", module) .add("Options array", () => { const [value, setValue] = useState>(); @@ -93,4 +96,30 @@ storiesOf("Combobox", module) )} ); + }) + .add("Links as render props", () => { + const [value, setValue] = useState>(); + const [query, setQuery] = useState("Hello"); + return ( + + + {(o) => ( + + {({ active }) => ( + + {o.label} + + )} + + )} + + + ); }); diff --git a/scm-ui/ui-forms/src/combobox/Combobox.tsx b/scm-ui/ui-forms/src/combobox/Combobox.tsx index 9442e2626f..4ec61919e6 100644 --- a/scm-ui/ui-forms/src/combobox/Combobox.tsx +++ b/scm-ui/ui-forms/src/combobox/Combobox.tsx @@ -25,7 +25,6 @@ import React, { ForwardedRef, Fragment, - KeyboardEvent, KeyboardEventHandler, ReactElement, Ref, @@ -48,7 +47,8 @@ const OptionsWrapper = styled(HeadlessCombobox.Options).attrs({ border: var(--scm-border); background-color: var(--scm-secondary-background); max-width: 35ch; - + width: 35ch; + &:empty { border: 0; clip: rect(0 0 0 0); @@ -73,6 +73,9 @@ const StyledComboboxOption = styled.li.attrs({ opacity: 40%; cursor: unset !important; } + > a { + color: inherit !important; + } `; type BaseProps = { @@ -138,7 +141,6 @@ function ComboboxComponent(props: ComboboxProps, ref: ForwardedRef) => props.onChange && props.onChange(e)} disabled={props.disabled || props.readOnly} - onKeyDown={(e: KeyboardEvent) => props.onKeyDown && props.onKeyDown(e)} name={props.name} form={props.form} defaultValue={props.defaultValue} @@ -159,6 +161,9 @@ function ComboboxComponent(props: ComboboxProps, ref: ForwardedRef { + props.onKeyDown && props.onKeyDown(e) + }} {...createAttributesForTesting(props.testId)} /> {options} diff --git a/scm-ui/ui-forms/src/index.ts b/scm-ui/ui-forms/src/index.ts index 40a9a96316..0bcbda8c59 100644 --- a/scm-ui/ui-forms/src/index.ts +++ b/scm-ui/ui-forms/src/index.ts @@ -45,6 +45,7 @@ export { default as ComboboxField } from "./combobox/ComboboxField"; export { default as Input } from "./input/Input"; export { default as Select } from "./select/Select"; export * from "./resourceHooks"; +export { default as Label } from "./base/label/Label"; export const Form = Object.assign(FormCmp, { Row: FormRow, diff --git a/scm-ui/ui-webapp/src/components/HeaderDropDown.tsx b/scm-ui/ui-webapp/src/components/HeaderDropDown.tsx index 552bdcc8b5..048c579242 100644 --- a/scm-ui/ui-webapp/src/components/HeaderDropDown.tsx +++ b/scm-ui/ui-webapp/src/components/HeaderDropDown.tsx @@ -43,7 +43,7 @@ const DropDownMenu = styled.div` } @media screen and (max-width: ${devices.mobile.width}px) { - ${props => + ${(props) => props.mobilePosition === "right" && css` right: -1.5rem; @@ -84,7 +84,7 @@ const DropDownMenu = styled.div` right: 1.375rem; } - ${props => + ${(props) => props.mobilePosition === "right" && css` @media screen and (max-width: ${devices.mobile.width}px) { @@ -143,7 +143,7 @@ type CounterProps = { const Counter = styled.span` position: absolute; top: -0.75rem; - right: ${props => (props.count.length <= 1 ? "-0.25" : "-0.50")}rem; + right: ${(props) => (props.count.length <= 1 ? "-0.25" : "-0.50")}rem; `; type IconWrapperProps = { @@ -158,46 +158,52 @@ const IconWrapper: FC = ({ icon, count }) => ( ); -type Props = DropDownMenuProps & { - className?: string; - icon: React.ReactNode; - count?: string; - error?: Error | null; - isLoading?: boolean; -}; +type Props = React.PropsWithChildren< + DropDownMenuProps & { + className?: string; + icon: React.ReactNode; + count?: string; + error?: Error | null; + isLoading?: boolean; + } +>; const DropDownTrigger = styled.div` padding: 0.65rem 0.75rem; `; -const HeaderDropDown: FC = ({ className, icon, count, error, isLoading, mobilePosition, children }) => { - const [open, setOpen] = useState(false); +const HeaderDropDown = React.forwardRef( + ({ className, icon, count, error, isLoading, mobilePosition, children }, ref) => { + const [open, setOpen] = useState(false); - useEffect(() => { - const close = () => setOpen(false); - window.addEventListener("click", close); - return () => window.removeEventListener("click", close); - }, []); + useEffect(() => { + const close = () => setOpen(false); + window.addEventListener("click", close); + return () => window.removeEventListener("click", close); + }, []); - return ( - <> -
e.stopPropagation()} + onClick={(e) => e.stopPropagation()} tabIndex={0} + ref={ref} > setOpen(o => !o)} + onClick={() => setOpen((o) => !o)} > @@ -206,9 +212,9 @@ const HeaderDropDown: FC = ({ className, icon, count, error, isLoading, m {isLoading ? : null} {children} -
- - ); -}; + + ); + } +); export default HeaderDropDown; diff --git a/scm-ui/ui-webapp/src/containers/NavigationBar.tsx b/scm-ui/ui-webapp/src/containers/NavigationBar.tsx index 3e14398a86..24891004d2 100644 --- a/scm-ui/ui-webapp/src/containers/NavigationBar.tsx +++ b/scm-ui/ui-webapp/src/containers/NavigationBar.tsx @@ -22,7 +22,7 @@ * SOFTWARE. */ -import React, { FC, useEffect, useState } from "react"; +import React, {FC, useEffect, useRef, useState} from "react"; import { Links } from "@scm-manager/ui-types"; import classNames from "classnames"; import styled from "styled-components"; @@ -105,6 +105,7 @@ type Props = { const NavigationBar: FC = ({ links }) => { const [burgerActive, setBurgerActive] = useState(false); const [t] = useTranslation("commons"); + const notificationsRef = useRef(null); useEffect(() => { const close = () => { if (burgerActive) { @@ -114,7 +115,6 @@ const NavigationBar: FC = ({ links }) => { window.addEventListener("click", close); return () => window.removeEventListener("click", close); }, [burgerActive]); - return (
@@ -139,8 +139,8 @@ const NavigationBar: FC = ({ links }) => {
- - + +
diff --git a/scm-ui/ui-webapp/src/containers/Notifications.tsx b/scm-ui/ui-webapp/src/containers/Notifications.tsx index f7d6160593..af97043448 100644 --- a/scm-ui/ui-webapp/src/containers/Notifications.tsx +++ b/scm-ui/ui-webapp/src/containers/Notifications.tsx @@ -247,11 +247,11 @@ const count = (data?: NotificationCollection) => { } }; -type NotificationProps = { +type NotificationProps = React.PropsWithChildren<{ className?: string; -}; +}>; -const Notifications: FC = ({ className }) => { +const Notifications = React.forwardRef(({ className }, ref) => { const { data, isLoading, error, refetch } = useNotifications(); const { notifications, remove, clear } = useNotificationSubscription(refetch, data); @@ -265,11 +265,12 @@ const Notifications: FC = ({ className }) => { icon={} count={count(data)} mobilePosition="left" + ref={ref} > {data ? : null} ); -}; +}); export default Notifications; diff --git a/scm-ui/ui-webapp/src/containers/OmniSearch.tsx b/scm-ui/ui-webapp/src/containers/OmniSearch.tsx index deb1ffbba0..459d9da007 100644 --- a/scm-ui/ui-webapp/src/containers/OmniSearch.tsx +++ b/scm-ui/ui-webapp/src/containers/OmniSearch.tsx @@ -21,41 +21,29 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ -import React, { - ComponentProps, - Dispatch, - FC, - KeyboardEvent as ReactKeyboardEvent, - MouseEvent, - ReactElement, - SetStateAction, - useCallback, - useEffect, - useMemo, - useRef, - useState, -} from "react"; -import { Hit, Links, Repository, ValueHitField } from "@scm-manager/ui-types"; +import React, { FC, Fragment, RefObject, useCallback, useEffect, useMemo, useRef, useState } from "react"; +import { Hit, Links, Repository, ValueHitField, Option } from "@scm-manager/ui-types"; import styled from "styled-components"; import { useNamespaceAndNameContext, useOmniSearch, useSearchTypes } from "@scm-manager/ui-api"; import classNames from "classnames"; -import { Link, useHistory, useLocation } from "react-router-dom"; +import { useHistory, useLocation } from "react-router-dom"; import { useTranslation } from "react-i18next"; -import { devices, Icon, RepositoryAvatar } from "@scm-manager/ui-components"; -import SyntaxHelp from "../search/SyntaxHelp"; +import { RepositoryAvatar, Icon } from "@scm-manager/ui-components"; import SyntaxModal from "../search/SyntaxModal"; -import SearchErrorNotification from "../search/SearchErrorNotification"; import queryString from "query-string"; import { orderTypes } from "../search/Search"; import { useShortcut } from "@scm-manager/ui-shortcuts"; +import { Label, Combobox } from "@scm-manager/ui-forms"; +import { Combobox as HeadlessCombobox } from "@headlessui/react"; -const Input = styled.input` - border-radius: 4px !important; +const ResultHeading = styled.div` + border-top: 1px solid lightgray; `; type Props = { shouldClear: boolean; ariaId: string; + nextFocusRef: RefObject; }; type GuardProps = Props & { @@ -68,29 +56,6 @@ const namespaceAndName = (hit: Hit) => { return `${namespace}/${name}`; }; -type HitsProps = { - entries: ReactElement>[]; - hits: Hit[]; - showHelp: () => void; - ariaId: string; -}; - -const QuickSearchNotification: FC = ({ children }) =>
{children}
; - -const ResultHeading = styled.h3` - border-top: 1px solid lightgray; -`; - -const DropdownMenu = styled.div` - max-width: 20rem; -`; - -const SearchInput = styled(Input)` - @media screen and (max-width: ${devices.mobile.width}px) { - width: 9rem; - } -`; - const AvatarSection: FC<{ repository: Repository }> = ({ repository }) => { if (!repository) { return null; @@ -103,150 +68,29 @@ const AvatarSection: FC<{ repository: Repository }> = ({ repository }) => { ); }; -const HitList: FC> = ({ entries, ariaId }) => { - return ( -
    - {entries} -
- ); -}; - const HitEntry: FC<{ - selected: boolean; link: string; label: string; - clear: () => void; repository?: Repository; - ariaId: string; -}> = ({ selected, link, label, clear, repository, ariaId }) => { - return ( -
  • e.preventDefault()} - onClick={clear} - role="option" - aria-selected={selected} - id={selected ? `omni-search-selected-option-${ariaId}` : undefined} - > - - {repository ? : } - {label} - -
  • - ); -}; - -type ScreenReaderHitSummaryProps = { - hits: Hit[]; -}; - -const ScreenReaderHitSummary: FC = ({ hits }) => { - const [t] = useTranslation("commons"); - const key = hits.length > 0 ? "screenReaderHint" : "screenReaderHintNoResult"; - return ( - - {t(`search.quickSearch.${key}`, { count: hits.length })} - - ); -}; - -const Hits: FC = ({ entries, hits, showHelp, ...rest }) => { - const [t] = useTranslation("commons"); - - return ( - <> -
    - - - - {t("search.quickSearch.resultHeading")} - - -
    - - ); -}; - -const useKeyBoardNavigation = ( - entries: HitsProps["entries"], - clear: () => void, - hideResults: () => void, - index: number, - setIndex: Dispatch>, - defaultLink: string -) => { + query: string; +}> = ({ link, label, repository, query }) => { const history = useHistory(); - - const onKeyDown = (e: ReactKeyboardEvent) => { - // We use e.which, because ie 11 does not support e.code - // https://caniuse.com/keyboardevent-code - switch (e.which) { - case 40: // e.code: ArrowDown - e.preventDefault(); - setIndex((idx) => { - if (idx < entries.length - 1) { - return idx + 1; - } - return idx; - }); - break; - case 38: // e.code: ArrowUp - e.preventDefault(); - setIndex((idx) => { - if (idx > 0) { - return idx - 1; - } - return idx; - }); - break; - case 13: // e.code: Enter - if ((e.target as HTMLInputElement).value.length >= 2) { - if (index < 0) { - history.push(defaultLink); - } else { - const entry = entries[index]; - if (entry?.props.link) { - history.push(entry.props.link); - } - } - clear(); - hideResults(); - } - - break; - case 27: // e.code: Escape - if (index >= 0) { - setIndex(-1); - } else { - clear(); - } - break; - } - }; - - return { - onKeyDown, - index, - }; + return ( + history.push(link), displayValue: label }} + key={label} + as={Fragment} + > + {({ active }) => ( + +
    + {repository ? : } + +
    +
    + )} +
    + ); }; const useDebounce = (value: string, delay: number) => { @@ -262,48 +106,6 @@ const useDebounce = (value: string, delay: number) => { return debouncedValue; }; -const isOnmiSearchElement = (element: Element) => { - return element.getAttribute("data-omnisearch"); -}; - -const useShowResultsOnFocus = () => { - const [showResults, setShowResults] = useState(false); - useEffect(() => { - if (showResults) { - const close = () => { - setShowResults(false); - }; - - const onKeyUp = (e: KeyboardEvent) => { - if (e.which === 9) { - // tab - const element = document.activeElement; - if (!element || !isOnmiSearchElement(element)) { - close(); - } - } - }; - - window.addEventListener("click", close); - window.addEventListener("keyup", onKeyUp); - return () => { - window.removeEventListener("click", close); - window.removeEventListener("keyup", onKeyUp); - }; - } - }, [showResults]); - return { - showResults, - onClick: (e: MouseEvent) => { - e.stopPropagation(); - setShowResults(true); - }, - onKeyPress: () => setShowResults(true), - onFocus: () => setShowResults(true), - hideResults: () => setShowResults(false), - }; -}; - const useSearchParams = () => { const location = useLocation(); const pathname = location.pathname; @@ -334,35 +136,42 @@ const useSearchParams = () => { }; }; -const OmniSearch: FC = ({ shouldClear, ariaId }) => { +const OmniSearch: FC = ({ shouldClear, ariaId, nextFocusRef, ...props }) => { const [t] = useTranslation("commons"); - const { searchType, initialQuery } = useSearchParams(); + const { initialQuery } = useSearchParams(); const [query, setQuery] = useState(initialQuery); + const [value, setValue] = useState void) | undefined> | undefined>({ label: query, value: query }); const searchInputRef = useRef(null); const debouncedQuery = useDebounce(query, 250); + const [showDropdown, setDropdown] = useState(true); const context = useNamespaceAndNameContext(); - const { data, isLoading, error } = useOmniSearch(debouncedQuery, { + const comboBoxRef = useRef(null); + const { data, isLoading } = useOmniSearch(debouncedQuery, { type: "repository", pageSize: 5, }); - const { showResults, hideResults, ...handlers } = useShowResultsOnFocus(); const [showHelp, setShowHelp] = useState(false); - const [index, setIndex] = useState(-1); - useEffect(() => { - setIndex(-1); + const handleChange = useCallback((value: Option<(() => void) | undefined>) => { + setValue(value); + value.value && value.value(); + setDropdown(true); }, []); useEffect(() => { setQuery(shouldClear ? "" : initialQuery); + setValue(shouldClear ? { label: "", value: undefined } : { label: initialQuery, value: undefined }); }, [shouldClear, initialQuery]); + const clearInput = () => { + shouldClear = true; + setValue({ label: "", value: undefined }); + setQuery(""); + setDropdown(false); + }; + const openHelp = () => setShowHelp(true); + const closeHelp = () => setShowHelp(false); - const clearQuery = useCallback(() => { - if (shouldClear) { - setQuery(""); - } - }, [shouldClear]); const hits = data?._embedded?.hits || []; const searchTypes = useSearchTypes({ @@ -384,13 +193,11 @@ const OmniSearch: FC = ({ shouldClear, ariaId }) => { newEntries.push( ); } @@ -398,44 +205,33 @@ const OmniSearch: FC = ({ shouldClear, ariaId }) => { newEntries.push( ); } newEntries.push( ); - const length = newEntries.length; hits?.forEach((hit, idx) => { newEntries.push( ); }); return newEntries; - }, [ariaId, clearQuery, context.name, context.namespace, hits, id, index, query, searchTypes, t]); - - const defaultLink = `/search/${searchType}/?q=${encodeURIComponent(query)}`; - const { onKeyDown } = useKeyBoardNavigation(entries, clearQuery, hideResults, index, setIndex, defaultLink); - + }, [context.name, context.namespace, hits, id, query, searchTypes, t]); return (
    {showHelp ? : null} @@ -444,50 +240,55 @@ const OmniSearch: FC = ({ shouldClear, ariaId }) => { "is-loading": isLoading, })} > -
    -
    - setQuery(e.target.value)} - onKeyDown={onKeyDown} - value={query} - role="combobox" - aria-autocomplete="both" - data-omnisearch="true" - aria-expanded={query.length > 2} - aria-label={t("search.ariaLabel")} - aria-owns={`omni-search-results-${ariaId}`} - aria-activedescendant={index >= 0 ? `omni-search-selected-option-${ariaId}` : undefined} - ref={searchInputRef} - {...handlers} - /> - {isLoading ? null : ( - - - - )} -
    - e.preventDefault()}> - {error ? ( - - - - ) : null} - {!error && data ? : null} - -
    + { + // This is hacky but it seems to be one of the only solutions right now + if (e.key === "Tab") { + nextFocusRef?.current?.focus(); + e.preventDefault(); + clearInput(); + comboBoxRef.current.value = ""; + } else { + setDropdown(true); + } + }} + > + {showDropdown ? entries : null} + {showDropdown ? ( + + {({ active }) => ( + + +
    + + +
    +
    +
    + )} +
    + ) : null} +
    ); }; -const OmniSearchGuard: FC = ({ links, shouldClear, ariaId }) => { +const OmniSearchGuard: FC = ({ links, ...props }) => { if (!links.search) { return null; } - return ; + return ; }; export default OmniSearchGuard; diff --git a/scm-ui/ui-webapp/src/search/SyntaxHelp.tsx b/scm-ui/ui-webapp/src/search/SyntaxHelp.tsx index ea3e391c25..e168c2df6a 100644 --- a/scm-ui/ui-webapp/src/search/SyntaxHelp.tsx +++ b/scm-ui/ui-webapp/src/search/SyntaxHelp.tsx @@ -28,13 +28,14 @@ import { HelpIcon, NoStyleButton } from "@scm-manager/ui-components"; type Props = { onClick: () => void; + className?: string }; -const SyntaxHelp: FC = ({ onClick }) => { +const SyntaxHelp: FC = ({ onClick, className }) => { const [t] = useTranslation("commons"); return ( <> - +