fix(DSTSUP-24): unify clear buttons in search fields (#3690)

* search field done * lgtm * add a11y stuff * fix tests * remove unused test-id * Create curvy-llamas-pump.md --------- Co-authored-by: Marcel Köhler <[email protected]>
marigold-ui · Feb 5, 2024 · b37c3ee · b37c3ee
1 parent d61b0fc
commit b37c3ee
Show file tree

Hide file tree

Showing 9 changed files with 155 additions and 138 deletions.
diff --git a/.changeset/curvy-llamas-pump.md b/.changeset/curvy-llamas-pump.md
@@ -0,0 +1,7 @@
+---
+"@marigold/components": patch
+"@marigold/theme-b2b": patch
+"@marigold/theme-core": patch
+---
+
+fix: unify clear buttons in search fields
diff --git a/packages/components/src/Autocomplete/Autocomplete.test.tsx b/packages/components/src/Autocomplete/Autocomplete.test.tsx
@@ -85,7 +85,7 @@ afterEach(cleanup);
 // ---------------
 test('renders an input', () => {
   render(
-    <Autocomplete label="vegetables" data-testid="input-field">
+    <Autocomplete label="vegetables">
       <Autocomplete.Item key="spinach">Spinach</Autocomplete.Item>
       <Autocomplete.Item key="carrots">Carrots</Autocomplete.Item>
       <Autocomplete.Item key="broccoli">Broccoli</Autocomplete.Item>
@@ -95,13 +95,13 @@ test('renders an input', () => {
 
   const textField = screen.getByLabelText('vegetables');
   expect(textField).toBeInTheDocument();
-  expect(textField).toHaveAttribute('type', 'text');
+  expect(textField).toHaveAttribute('type', 'search');
   expect(textField instanceof HTMLInputElement).toBeTruthy();
 });
 
 test('renders a label', () => {
   render(
-    <Autocomplete label="Label" data-testid="input-field">
+    <Autocomplete label="Label">
       <Autocomplete.Item key="spinach">Spinach</Autocomplete.Item>
       <Autocomplete.Item key="carrots">Carrots</Autocomplete.Item>
       <Autocomplete.Item key="broccoli">Broccoli</Autocomplete.Item>
@@ -116,7 +116,7 @@ test('renders a label', () => {
 
 test('supports disabled', () => {
   render(
-    <Autocomplete label="Label" data-testid="input-field" disabled>
+    <Autocomplete label="Label" disabled>
       <Autocomplete.Item key="spinach">Spinach</Autocomplete.Item>
       <Autocomplete.Item key="carrots">Carrots</Autocomplete.Item>
     </Autocomplete>
@@ -128,7 +128,7 @@ test('supports disabled', () => {
 
 test('supports required', () => {
   render(
-    <Autocomplete label="Label" data-testid="input-field" required>
+    <Autocomplete label="Label" required>
       <Autocomplete.Item key="spinach">Spinach</Autocomplete.Item>
       <Autocomplete.Item key="carrots">Carrots</Autocomplete.Item>
     </Autocomplete>
@@ -140,7 +140,7 @@ test('supports required', () => {
 
 test('supports readonly', () => {
   render(
-    <Autocomplete label="Label" data-testid="input-field" readOnly>
+    <Autocomplete label="Label" readOnly>
       <Autocomplete.Item key="spinach">Spinach</Autocomplete.Item>
       <Autocomplete.Item key="carrots">Carrots</Autocomplete.Item>
     </Autocomplete>
@@ -174,7 +174,7 @@ test('uses field structure', () => {
 
 test('opens the suggestions on user input', async () => {
   render(
-    <Autocomplete label="Label" data-testid="input-field">
+    <Autocomplete label="Label">
       <Autocomplete.Item key="spinach">Spinach</Autocomplete.Item>
       <Autocomplete.Item key="carrots">Carrots</Autocomplete.Item>
       <Autocomplete.Item key="broccoli">Broccoli</Autocomplete.Item>
@@ -191,7 +191,7 @@ test('opens the suggestions on user input', async () => {
 
 test('opens the suggestions on focus', async () => {
   render(
-    <Autocomplete label="Label" data-testid="input-field" menuTrigger="focus">
+    <Autocomplete label="Label" menuTrigger="focus">
       <Autocomplete.Item key="spinach">Spinach</Autocomplete.Item>
       <Autocomplete.Item key="carrots">Carrots</Autocomplete.Item>
       <Autocomplete.Item key="broccoli">Broccoli</Autocomplete.Item>
@@ -208,7 +208,7 @@ test('opens the suggestions on focus', async () => {
 
 test('opens the suggestions on arrow down (manual)', async () => {
   render(
-    <Autocomplete label="Label" data-testid="input-field" menuTrigger="manual">
+    <Autocomplete label="Label" menuTrigger="manual">
       <Autocomplete.Item key="spinach">Spinach</Autocomplete.Item>
       <Autocomplete.Item key="carrots">Carrots</Autocomplete.Item>
       <Autocomplete.Item key="broccoli">Broccoli</Autocomplete.Item>
@@ -225,7 +225,7 @@ test('opens the suggestions on arrow down (manual)', async () => {
 
 test('shows suggestions based on user input', async () => {
   render(
-    <Autocomplete label="Label" data-testid="input-field">
+    <Autocomplete label="Label">
       <Autocomplete.Item key="spinach">Spinach</Autocomplete.Item>
       <Autocomplete.Item key="carrots">Carrots</Autocomplete.Item>
       <Autocomplete.Item key="broccoli">Broccoli</Autocomplete.Item>
@@ -245,11 +245,7 @@ test('shows suggestions based on user input', async () => {
 
 test('supports disabling suggestions', async () => {
   render(
-    <Autocomplete
-      label="Label"
-      data-testid="input-field"
-      disabledKeys={['spinach']}
-    >
+    <Autocomplete label="Label" disabledKeys={['spinach']}>
       <Autocomplete.Item id="spinach">Spinach</Autocomplete.Item>
       <Autocomplete.Item id="carrots">Carrots</Autocomplete.Item>
       <Autocomplete.Item id="broccoli">Broccoli</Autocomplete.Item>
@@ -266,11 +262,7 @@ test('supports disabling suggestions', async () => {
 
 test('supporst showing a help text', () => {
   render(
-    <Autocomplete
-      label="Label"
-      data-testid="input-field"
-      description="This is a description"
-    >
+    <Autocomplete label="Label" description="This is a description">
       <Autocomplete.Item key="spinach">Spinach</Autocomplete.Item>
       <Autocomplete.Item key="carrots">Carrots</Autocomplete.Item>
       <Autocomplete.Item key="broccoli">Broccoli</Autocomplete.Item>
@@ -283,12 +275,7 @@ test('supporst showing a help text', () => {
 
 test('supporst showing an error', () => {
   render(
-    <Autocomplete
-      label="Label"
-      data-testid="input-field"
-      error
-      errorMessage="Error!"
-    >
+    <Autocomplete label="Label" error errorMessage="Error!">
       <Autocomplete.Item key="spinach">Spinach</Autocomplete.Item>
       <Autocomplete.Item key="carrots">Carrots</Autocomplete.Item>
       <Autocomplete.Item key="broccoli">Broccoli</Autocomplete.Item>
@@ -301,7 +288,7 @@ test('supporst showing an error', () => {
 
 test('supports default value', () => {
   render(
-    <Autocomplete label="Label" data-testid="input-field" defaultValue="garlic">
+    <Autocomplete label="Label" defaultValue="garlic">
       <Autocomplete.Item key="spinach">Spinach</Autocomplete.Item>
       <Autocomplete.Item key="carrots">Carrots</Autocomplete.Item>
       <Autocomplete.Item key="broccoli">Broccoli</Autocomplete.Item>
@@ -317,12 +304,7 @@ test('can be controlled', async () => {
     const [value, setValue] = React.useState('');
     return (
       <>
-        <Autocomplete
-          label="Label"
-          data-testid="input-field"
-          value={value}
-          onChange={setValue}
-        >
+        <Autocomplete label="Label" value={value} onChange={setValue}>
           <Autocomplete.Item id="spinach">Spinach</Autocomplete.Item>
           <Autocomplete.Item id="carrots">Carrots</Autocomplete.Item>
           <Autocomplete.Item id="broccoli">Broccoli</Autocomplete.Item>
@@ -343,7 +325,7 @@ test('can be controlled', async () => {
 
 test('supports autocompletion', async () => {
   render(
-    <Autocomplete label="Label" data-testid="input-field">
+    <Autocomplete label="Label">
       <Autocomplete.Item id="spinach">Spinach</Autocomplete.Item>
       <Autocomplete.Item id="carrots">Carrots</Autocomplete.Item>
       <Autocomplete.Item id="broccoli">Broccoli</Autocomplete.Item>
@@ -362,7 +344,7 @@ test('supports autocompletion', async () => {
 
 test('supports clear input value', async () => {
   render(
-    <Autocomplete label="Label" data-testid="input-field">
+    <Autocomplete label="Label">
       <Autocomplete.Item id="spinach">Spinach</Autocomplete.Item>
       <Autocomplete.Item id="carrots">Carrots</Autocomplete.Item>
       <Autocomplete.Item id="broccoli">Broccoli</Autocomplete.Item>
@@ -373,7 +355,7 @@ test('supports clear input value', async () => {
   const input = screen.getByRole('combobox');
   await user.type(input, 'sp');
 
-  const clearButton = screen.getByTestId('clear-button');
+  const clearButton = screen.getByLabelText('Clear search');
   expect(clearButton).toBeInTheDocument();
   await user.click(clearButton);
   expect(input).toHaveValue('');
@@ -383,7 +365,7 @@ test('supports submit handler', async () => {
   const spy = jest.fn();
 
   render(
-    <Autocomplete label="Label" data-testid="input-field" onSubmit={spy}>
+    <Autocomplete label="Label" onSubmit={spy}>
       <Autocomplete.Item id="spinach">Spinach</Autocomplete.Item>
       <Autocomplete.Item id="carrots">Carrots</Autocomplete.Item>
       <Autocomplete.Item id="broccoli">Broccoli</Autocomplete.Item>

diff --git a/packages/components/src/Autocomplete/Autocomplete.tsx b/packages/components/src/Autocomplete/Autocomplete.tsx
@@ -9,30 +9,30 @@ import { ComboBox, ComboBoxStateContext, Key } from 'react-aria-components';
 import type RAC from 'react-aria-components';
 
 import { FieldBase, FieldBaseProps } from '../FieldBase';
-import { Input } from '../Input';
+import { SearchInput } from '../Input/SearchInput';
 import { ListBox } from '../ListBox';
 import { Popover } from '../Overlay/Popover';
-import { AutocompleteClearButton } from './ClearButton';
 
 // Search Input (we can't use our SearchField because of FieldBase)
 //----------------
-interface SearchInputProps {
+interface AutocompleteInputProps {
   onSubmit?: (key: Key | null, value: string | null) => void;
   onClear?: () => void;
   ref?: Ref<HTMLInputElement> | undefined;
 }
-const SearchInput = ({ onSubmit, onClear, ref }: SearchInputProps) => {
+const AutocompleteInput = ({
+  onSubmit,
+  onClear,
+  ref,
+}: AutocompleteInputProps) => {
   const state = React.useContext(ComboBoxStateContext);
 
   return (
-    <Input
+    <SearchInput
       ref={ref}
-      icon={<SearchIcon />}
-      action={
-        state?.inputValue !== '' ? (
-          <AutocompleteClearButton onClear={onClear} />
-        ) : undefined
-      }
+      className={{
+        action: state?.inputValue === '' ? 'hidden' : undefined,
+      }}
       onKeyDown={e => {
         if (e.key === 'Enter' || e.key === 'Escape') {
           e.preventDefault();
@@ -43,25 +43,17 @@ const SearchInput = ({ onSubmit, onClear, ref }: SearchInputProps) => {
           }
         }
       }}
+      onClear={() => {
+        state?.setInputValue('');
+        state?.setSelectedKey(null);
+        onClear?.();
+      }}
     />
   );
 };
 
-// Search Icon
-//----------------
-const SearchIcon = (props: { className?: string }) => (
-  <svg
-    xmlns="http://www.w3.org/2000/svg"
-    viewBox="0 0 24 24"
-    fill="currentColor"
-    width={24}
-    height={24}
-    {...props}
-  >
-    <path d="M16.1865 14.5142H15.3057L14.9936 14.2131C16.0862 12.9421 16.744 11.292 16.744 9.497C16.744 5.49443 13.4996 2.25 9.497 2.25C5.49443 2.25 2.25 5.49443 2.25 9.497C2.25 13.4996 5.49443 16.744 9.497 16.744C11.292 16.744 12.9421 16.0862 14.2131 14.9936L14.5142 15.3057V16.1865L20.0888 21.75L21.75 20.0888L16.1865 14.5142ZM9.49701 14.5142C6.72085 14.5142 4.47986 12.2732 4.47986 9.49701C4.47986 6.72085 6.72085 4.47986 9.49701 4.47986C12.2732 4.47986 14.5142 6.72085 14.5142 9.49701C14.5142 12.2732 12.2732 14.5142 9.49701 14.5142Z" />
-  </svg>
-);
-
+// Props
+// ---------------
 type RemovedProps =
   | 'className'
   | 'style'
@@ -110,6 +102,9 @@ interface AutocompleteComponent
   > {
   Item: typeof ListBox.Item;
 }
+
+// Component
+// ---------------
 const _Autocomplete = forwardRef<HTMLInputElement, AutocompleteProps>(
   (
     {
@@ -141,14 +136,12 @@ const _Autocomplete = forwardRef<HTMLInputElement, AutocompleteProps>(
     };
 
     return (
-      <>
-        <FieldBase as={ComboBox} {...props}>
-          <SearchInput onSubmit={onSubmit} onClear={onClear} ref={ref} />
-          <Popover>
-            <ListBox>{children}</ListBox>
-          </Popover>
-        </FieldBase>
-      </>
+      <FieldBase as={ComboBox} {...props}>
+        <AutocompleteInput onSubmit={onSubmit} onClear={onClear} ref={ref} />
+        <Popover>
+          <ListBox>{children}</ListBox>
+        </Popover>
+      </FieldBase>
     );
   }
 ) as AutocompleteComponent;

diff --git a/packages/components/src/Autocomplete/ClearButton.tsx b/packages/components/src/Autocomplete/ClearButton.tsx
diff --git a/packages/components/src/Input/Input.tsx b/packages/components/src/Input/Input.tsx
@@ -29,24 +29,24 @@ const _Input = forwardRef<HTMLInputElement, InputProps>(
 
     const inputIcon = icon
       ? cloneElement(icon, {
+          ...icon.props,
           className: cn(
             'pointer-events-none absolute',
             classNames.icon,
             icon.props.className
           ),
-          ...icon.props,
         })
       : null;
 
     const inputAction =
       action && !props.readOnly
         ? cloneElement(action, {
+            ...action.props,
             className: cn(
-              'absolute',
+              'absolute right-0',
               classNames.action,
               action.props.className
             ),
-            ...action.props,
           })
         : null;