Fix PR labels keyboard navigation and layout

packages/ui/src/components/dropdown-menu.tsx

@@ -108,19 +108,20 @@ const DropdownMenuContent = forwardRef<ElementRef<typeof DropdownMenuPrimitive.C
     ])
 
     /**
-     * !!! This code is executed only when inside a ShadowRoot
-     *
      * To navigate between items using the top and down arrow keys
      */
     const onKeyDownCaptureHandler = (e: KeyboardEvent<HTMLDivElement>) => {
+      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.
        */
       propOnKeyDownCapture?.(e)
       if (e.defaultPrevented || e.isDefaultPrevented?.()) return
 
-      if (e.key !== 'ArrowDown' && e.key !== 'ArrowUp' && e.key !== 'Tab') {
+      if ((isTargetInput && e.key !== 'ArrowDown' && e.key !== 'ArrowUp') || e.key === 'Tab') {
         e.stopPropagation()
         return
       }
@@ -132,6 +133,9 @@ const DropdownMenuContent = forwardRef<ElementRef<typeof DropdownMenuPrimitive.C
 
       if (!isShadowRoot) return
 
+      /**
+       * !!! This code is executed only when inside a ShadowRoot
+       */
       const items = Array.from(
         rootEl.querySelectorAll<HTMLElement>('[data-radix-collection-item]:not([data-disabled])[role*="menuitem"]')
       )
@@ -213,10 +217,13 @@ const DropdownBaseItem = ({
 }: DropdownBaseItemProps) => (
   <div className={cn('cn-dropdown-menu-base-item', className)}>
     {children}
-    <Layout.Grid gapX="2xs" className="w-fit">
-      {typeof title === 'string' ? <Text color="foreground-1">{title}</Text> : title}
-      {typeof description === 'string' ? <Text>{description}</Text> : description}
-    </Layout.Grid>
+    {(!!title || !!description) && (
+      <Layout.Grid gapX="2xs" className="w-fit">
+        {typeof title === 'string' ? <Text color="foreground-1">{title}</Text> : title}
+        {typeof description === 'string' ? <Text>{description}</Text> : description}
+      </Layout.Grid>
+    )}
+
     {tag && <Tag {...tag} />}
 
     <div className="ml-auto">

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<HTMLInputElement, InputProps>(
-  ({ theme, size, className, inputContainerClassName, prefix = null, suffix = null, ...props }, ref) => {
+  ({ theme, size, className, inputContainerClassName, prefix = null, suffix = null, autoFocus, ...props }, ref) => {
+    const inputRef = useRef<HTMLInputElement | null>(null)
+
+    const mergedRef = useMergeRefs<HTMLInputElement>([
+      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<HTMLInputElement, InputProps>(
     return (
       <div className={cn(inputVariants({ size, theme }), inputContainerClassName)}>
         {wrappedPrefix}
-        <input className={cn('cn-input-input', className)} ref={ref} {...props} />
+        <input className={cn('cn-input-input', className)} ref={mergedRef} {...props} />
         {wrappedSuffix}
       </div>
     )

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<InputProps, 'type' | 'onChange' | 'label' | 'prefix'> {
+export interface SearchInputProps extends Omit<InputProps, 'type' | 'onChange' | 'label'> {
   onChange?: (value: string) => void
   debounce?: number | boolean
 }
 
 const SearchInput = forwardRef<HTMLInputElement, SearchInputProps>(
-  ({ placeholder = 'Search', className, debounce = true, onChange, ...props }, ref) => {
+  ({ placeholder = 'Search', className, debounce = true, onChange, prefix: prefixProp, ...props }, ref) => {
+    const prefix = prefixProp ?? (
+      <div className="ml-1 grid w-8 shrink-0 place-items-center border-r-0">
+        <IconV2 name="search" size="sm" />
+      </div>
+    )
+
     const effectiveDebounce = useMemo(() => {
       if (debounce === true) {
         return 300
@@ -58,11 +64,7 @@ const SearchInput = forwardRef<HTMLInputElement, SearchInputProps>(
         ref={ref}
         className={cn('cn-input-search', className)}
         onChange={handleInputChange}
-        prefix={
-          <div className="ml-1 grid w-8 shrink-0 place-items-center border-r-0">
-            <IconV2 name="search" size="sm" />
-          </div>
-        }
+        prefix={prefix}
         placeholder={placeholder}
         {...props}
       />

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<HTMLInputElement, TextInputProps>((props, ref) => {
     orientation,
     informerProps,
     informerContent,
-    autoFocus,
     ...restProps
   } = props
-  const inputRef = useRef<HTMLInputElement | null>(null)
   const isHorizontal = orientation === 'horizontal'
 
   // override theme based on error and warning
@@ -34,23 +31,6 @@ const TextInput = forwardRef<HTMLInputElement, TextInputProps>((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<HTMLInputElement>([
-    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 (
     <ControlGroup.Root className={wrapperClassName} orientation={orientation}>
       {(!!label || (isHorizontal && !!caption)) && (
@@ -72,7 +52,7 @@ const TextInput = forwardRef<HTMLInputElement, TextInputProps>((props, ref) => {
       )}
 
       <ControlGroup.InputWrapper>
-        <BaseInput {...restProps} ref={mergedRef} theme={theme} id={inputId} disabled={disabled} />
+        <BaseInput {...restProps} ref={ref} theme={theme} id={inputId} disabled={disabled} />
 
         {error ? (
           <FormCaption disabled={disabled} theme="danger">

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<LabelValueSelectorProps> = ({ 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<LabelValueSelectorProps> = ({ 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 (
-    <DropdownMenu.Content className="w-80" align="end" sideOffset={2} alignOffset={0}>
-      <DropdownMenu.Header className="relative">
-        <SearchBox.Root
+    <DropdownMenu.Content className="w-80" align="end" sideOffset={2}>
+      <DropdownMenu.Header>
+        <SearchInput
+          ref={searchInputRef}
+          size="sm"
           autoFocus
-          className="w-full"
-          inputClassName="pl-1.5 pr-8"
+          id="search"
+          defaultValue={searchState}
+          onChange={setSearchState}
           placeholder={getSearchBoxPlaceholder()}
-          value={search}
-          handleChange={handleSearchChange}
-          showOnFocus
-          hasSearchIcon={false}
-          {...wrapConditionalObjectElement({ maxLength: 50 }, !!label?.isCustom)}
-        >
-          <div className="pr-2">
+          onKeyDown={handleSearchKeyDown}
+          suffix={
+            <Button iconOnly size="xs" variant="transparent" onClick={onSearchClean}>
+              <IconV2 name="xmark" size="2xs" />
+            </Button>
+          }
+          prefix={
             <LabelTag
+              className="border-0 pl-cn-xs"
               scope={label.scope ?? 0}
               color={label.color as ColorsEnum}
               labelKey={label.key ?? ''}
@@ -110,39 +122,37 @@ export const LabelValueSelector: FC<LabelValueSelectorProps> = ({ label, handleA
                 size: 'sm'
               }}
             />
-          </div>
-        </SearchBox.Root>
-
-        <Button
-          iconOnly
-          size="xs"
-          className="absolute right-2 top-2 z-20"
-          variant="transparent"
-          onClick={onSearchClean}
-        >
-          <IconV2 name="xmark" size="2xs" />
-        </Button>
+          }
+          {...wrapConditionalObjectElement({ maxLength: 50 }, !!label?.isCustom)}
+        />
       </DropdownMenu.Header>
 
-      {values.map(value => (
-        <DropdownMenu.Item
-          key={value.id}
-          onSelect={handleOnSelect(value)}
-          tag={{
-            variant: 'secondary',
-            size: 'sm',
-            theme: value.color as EnumLabelColor,
-            value: value.value ?? ''
-          }}
-          checkmark={label.selectedValueId === value.id}
-        />
-      ))}
+      {values.map((value, idx) => {
+        const { ref, onKeyDown } = getItemProps(idx)
+
+        return (
+          <DropdownMenu.Item
+            key={value.id}
+            ref={ref}
+            onSelect={handleOnSelect(value)}
+            tag={{
+              variant: 'secondary',
+              size: 'sm',
+              theme: value.color as EnumLabelColor,
+              value: value.value ?? ''
+            }}
+            checkmark={label.selectedValueId === value.id}
+            onKeyDown={onKeyDown}
+          />
+        )
+      })}
 
       {isAllowAddNewValue && !!label?.isCustom && !!values.length && <DropdownMenu.Separator />}
 
       {isAllowAddNewValue && !!label?.isCustom && (
         <DropdownMenu.Group label={t('views:pullRequests.addValue', 'Add new value')}>
           <DropdownMenu.Item
+            ref={refForNewValue}
             className="[&>.cn-dropdown-menu-base-item]:gap-0"
             onSelect={handleAddNewValue}
             tag={{
@@ -156,11 +166,12 @@ export const LabelValueSelector: FC<LabelValueSelectorProps> = ({ label, handleA
               labelClassName: 'grid grid-flow-col',
               valueClassName: 'grid grid-flow-col content-center'
             }}
+            onKeyDown={onKeyDownForNewValue}
           />
         </DropdownMenu.Group>
       )}
 
-      {!values.length && !label?.isCustom && (
+      {!values.length && (!label?.isCustom || (!!label?.isCustom && !isAllowAddNewValue)) && (
         <DropdownMenu.NoOptions>{t('views:pullRequests.labelNotFound', 'Label not found')}</DropdownMenu.NoOptions>
       )}
     </DropdownMenu.Content>