From 4f9439a2a358c4087211d3d947001cb8e25b8b15 Mon Sep 17 00:00:00 2001 From: Alex Zemlyakov Date: Thu, 4 Sep 2025 15:22:37 +0300 Subject: [PATCH] Fix PR labels keyboard navigation and layout --- packages/ui/src/components/dropdown-menu.tsx | 21 ++-- .../ui/src/components/inputs/base-input.tsx | 29 ++++- .../ui/src/components/inputs/search-input.tsx | 16 +-- .../ui/src/components/inputs/text-input.tsx | 24 +---- .../labels/label-value-selector.tsx | 101 ++++++++++-------- .../labels/pull-request-labels-header.tsx | 33 +----- 6 files changed, 109 insertions(+), 115 deletions(-) diff --git a/packages/ui/src/components/dropdown-menu.tsx b/packages/ui/src/components/dropdown-menu.tsx index b20e2e64ab..5f3bb11d4d 100644 --- a/packages/ui/src/components/dropdown-menu.tsx +++ b/packages/ui/src/components/dropdown-menu.tsx @@ -108,11 +108,12 @@ const DropdownMenuContent = forwardRef) => { + const target = e.target + const isTargetInput = target instanceof HTMLElement && !!target.closest('input') + /** * To block further code execution in onKeyDownCaptureHandler, * need to call e.preventDefault() inside propOnKeyDownCapture. @@ -120,7 +121,7 @@ const DropdownMenuContent = forwardRef('[data-radix-collection-item]:not([data-disabled])[role*="menuitem"]') ) @@ -213,10 +217,13 @@ const DropdownBaseItem = ({ }: DropdownBaseItemProps) => (
{children} - - {typeof title === 'string' ? {title} : title} - {typeof description === 'string' ? {description} : description} - + {(!!title || !!description) && ( + + {typeof title === 'string' ? {title} : title} + {typeof description === 'string' ? {description} : description} + + )} + {tag && }
diff --git a/packages/ui/src/components/inputs/base-input.tsx b/packages/ui/src/components/inputs/base-input.tsx index 2ab0ecf28f..d8c656c5a5 100644 --- a/packages/ui/src/components/inputs/base-input.tsx +++ b/packages/ui/src/components/inputs/base-input.tsx @@ -5,10 +5,12 @@ import { isValidElement, PropsWithChildren, ReactElement, - ReactNode + ReactNode, + useEffect, + useRef } from 'react' -import { cn } from '@utils/cn' +import { cn, useMergeRefs } from '@/utils' import { cva, type VariantProps } from 'class-variance-authority' function InputAffix({ children, isPrefix = false }: PropsWithChildren<{ isPrefix?: boolean }>) { @@ -49,7 +51,26 @@ export interface InputProps extends BaseInputProps { } const BaseInput = forwardRef( - ({ theme, size, className, inputContainerClassName, prefix = null, suffix = null, ...props }, ref) => { + ({ theme, size, className, inputContainerClassName, prefix = null, suffix = null, autoFocus, ...props }, ref) => { + const inputRef = useRef(null) + + const mergedRef = useMergeRefs([ + node => { + if (!node) return + + inputRef.current = node + }, + ref + ]) + + useEffect(() => { + if (autoFocus && inputRef.current) { + const t = setTimeout(() => inputRef.current?.focus(), 0) + + return () => clearTimeout(t) + } + }, [autoFocus]) + // Check if prefix/suffix is a valid React element const isPrefixComponent = isValidElement(prefix) const isSuffixComponent = isValidElement(suffix) @@ -74,7 +95,7 @@ const BaseInput = forwardRef( return (
{wrappedPrefix} - + {wrappedSuffix}
) diff --git a/packages/ui/src/components/inputs/search-input.tsx b/packages/ui/src/components/inputs/search-input.tsx index 3a4b4ea7c9..cbb6cbbf76 100644 --- a/packages/ui/src/components/inputs/search-input.tsx +++ b/packages/ui/src/components/inputs/search-input.tsx @@ -7,13 +7,19 @@ import { debounce as debounceFn } from 'lodash-es' import { BaseInput, InputProps } from './base-input' // Custom onChange handler for search that works with strings instead of events -export interface SearchInputProps extends Omit { +export interface SearchInputProps extends Omit { onChange?: (value: string) => void debounce?: number | boolean } const SearchInput = forwardRef( - ({ placeholder = 'Search', className, debounce = true, onChange, ...props }, ref) => { + ({ placeholder = 'Search', className, debounce = true, onChange, prefix: prefixProp, ...props }, ref) => { + const prefix = prefixProp ?? ( +
+ +
+ ) + const effectiveDebounce = useMemo(() => { if (debounce === true) { return 300 @@ -58,11 +64,7 @@ const SearchInput = forwardRef( ref={ref} className={cn('cn-input-search', className)} onChange={handleInputChange} - prefix={ -
- -
- } + prefix={prefix} placeholder={placeholder} {...props} /> diff --git a/packages/ui/src/components/inputs/text-input.tsx b/packages/ui/src/components/inputs/text-input.tsx index 7eb193731b..09781b0dfd 100644 --- a/packages/ui/src/components/inputs/text-input.tsx +++ b/packages/ui/src/components/inputs/text-input.tsx @@ -1,7 +1,6 @@ -import { forwardRef, useEffect, useMemo, useRef } from 'react' +import { forwardRef, useMemo } from 'react' import { CommonInputsProp, ControlGroup, FormCaption, Label } from '@/components' -import { useMergeRefs } from '@/utils' import { BaseInput, InputProps } from './base-input' @@ -22,10 +21,8 @@ const TextInput = forwardRef((props, ref) => { orientation, informerProps, informerContent, - autoFocus, ...restProps } = props - const inputRef = useRef(null) const isHorizontal = orientation === 'horizontal' // override theme based on error and warning @@ -34,23 +31,6 @@ const TextInput = forwardRef((props, ref) => { // Generate a unique ID if one isn't provided const inputId = useMemo(() => props.id || `input-${Math.random().toString(36).substring(2, 9)}`, [props.id]) - const mergedRef = useMergeRefs([ - node => { - if (!node) return - - inputRef.current = node - }, - ref - ]) - - useEffect(() => { - if (autoFocus && inputRef.current) { - const t = setTimeout(() => inputRef.current?.focus(), 0) - - return () => clearTimeout(t) - } - }, [autoFocus]) - return ( {(!!label || (isHorizontal && !!caption)) && ( @@ -72,7 +52,7 @@ const TextInput = forwardRef((props, ref) => { )} - + {error ? ( diff --git a/packages/ui/src/views/repo/pull-request/components/labels/label-value-selector.tsx b/packages/ui/src/views/repo/pull-request/components/labels/label-value-selector.tsx index dae59f14e6..f3f33236af 100644 --- a/packages/ui/src/views/repo/pull-request/components/labels/label-value-selector.tsx +++ b/packages/ui/src/views/repo/pull-request/components/labels/label-value-selector.tsx @@ -1,8 +1,15 @@ import { FC, useMemo, useState } from 'react' -import { Button, DropdownMenu, getScopeType, IconV2, scopeTypeToIconMap, SearchBox } from '@/components' +import { + Button, + DropdownMenu, + getScopeType, + IconV2, + scopeTypeToIconMap, + SearchInput, + useSearchableDropdownKeyboardNavigation +} from '@/components' import { useTranslation } from '@/context' -import { useDebounceSearch } from '@/hooks' import { wrapConditionalObjectElement } from '@/utils' import { ColorsEnum, EnumLabelColor, HandleAddLabelType, LabelTag, TypesLabelValueInfo } from '@/views' @@ -18,11 +25,6 @@ export const LabelValueSelector: FC = ({ label, handleA const { t } = useTranslation() const [searchState, setSearchState] = useState('') - const { search, handleSearchChange } = useDebounceSearch({ - handleChangeSearchValue: setSearchState, - searchValue: searchState - }) - const { values, isAllowAddNewValue } = useMemo(() => { if (!label?.values) return { @@ -86,22 +88,32 @@ export const LabelValueSelector: FC = ({ label, handleA return '' } + const { searchInputRef, handleSearchKeyDown, getItemProps } = useSearchableDropdownKeyboardNavigation({ + itemsLength: values.length + (isAllowAddNewValue && !!label?.isCustom ? 1 : 0) + }) + + const { ref: refForNewValue, onKeyDown: onKeyDownForNewValue } = useMemo(() => getItemProps(values.length), [values]) + return ( - - - + + -
+ onKeyDown={handleSearchKeyDown} + suffix={ + + } + prefix={ = ({ label, handleA size: 'sm' }} /> -
-
- - + } + {...wrapConditionalObjectElement({ maxLength: 50 }, !!label?.isCustom)} + />
- {values.map(value => ( - - ))} + {values.map((value, idx) => { + const { ref, onKeyDown } = getItemProps(idx) + + return ( + + ) + })} {isAllowAddNewValue && !!label?.isCustom && !!values.length && } {isAllowAddNewValue && !!label?.isCustom && ( = ({ label, handleA labelClassName: 'grid grid-flow-col', valueClassName: 'grid grid-flow-col content-center' }} + onKeyDown={onKeyDownForNewValue} /> )} - {!values.length && !label?.isCustom && ( + {!values.length && (!label?.isCustom || (!!label?.isCustom && !isAllowAddNewValue)) && ( {t('views:pullRequests.labelNotFound', 'Label not found')} )}
diff --git a/packages/ui/src/views/repo/pull-request/components/labels/pull-request-labels-header.tsx b/packages/ui/src/views/repo/pull-request/components/labels/pull-request-labels-header.tsx index 704e719783..eb44a6e544 100644 --- a/packages/ui/src/views/repo/pull-request/components/labels/pull-request-labels-header.tsx +++ b/packages/ui/src/views/repo/pull-request/components/labels/pull-request-labels-header.tsx @@ -1,4 +1,4 @@ -import { KeyboardEvent, useMemo, useRef, useState } from 'react' +import { useMemo, useRef, useState } from 'react' import { Button, @@ -61,7 +61,6 @@ export const LabelsHeader = ({ isLabelsLoading }: LabelsHeaderProps) => { const { t } = useTranslation() - const footerLinkRef = useRef(null) const [labelWithValuesToShow, setLabelWithValuesToShow] = useState(null) const handleSearchQuery = (query: string) => { @@ -120,21 +119,6 @@ export const LabelsHeader = ({ itemsLength: labelsListWithValues?.length ?? 0 }) - const handleLinkKeyDown = (e: KeyboardEvent) => { - if (e.key === 'Tab') { - e.preventDefault() - searchInputRef.current?.focus() - } - } - - const handleContentKeyDownCapture = (e: KeyboardEvent) => { - if (e.key === 'Tab') { - e.preventDefault() - footerLinkRef.current?.focus() - return - } - } - return (
@@ -157,12 +141,7 @@ export const LabelsHeader = ({ )} {!labelWithValuesToShow && ( - + - + {t('views:pullRequests.editLabels', 'Edit labels')}