Repository: gravity-ui/uikit (ts)

Issue:
Do not show autocomplete for TextInput with inner label if no id is set
Default behaviour for TextInput with inner labels should be not to show autocomplete if no `id` prop is specified.

This will help users to avoid explicit mention of autocomplete="off" every time when using TextInput with inner label and there is not need for autocomplete suggestions.


Pull request:
fix(TextInput): no unexpected autocomplete when label prop present
fixes #507

- fixed unexpected autocomplete behavior for case with label prop as well
- split tests for plain input and textarea
- add some tests for autocomplete

Patch:
diff --git a/src/components/TextInput/TextInput.tsx b/src/components/TextInput/TextInput.tsx
index dbf8b110..49c0d1dd 100644
--- a/src/components/TextInput/TextInput.tsx
+++ b/src/components/TextInput/TextInput.tsx
@@ -62,14 +62,18 @@ export const TextInput = React.forwardRef<HTMLSpanElement, TextInputProps>(funct
     const [uncontrolledValue, setUncontrolledValue] = React.useState(defaultValue ?? '');
     const innerControlRef = React.useRef<HTMLTextAreaElement | HTMLInputElement>(null);
     const labelRef = React.useRef<HTMLLabelElement>(null);
-    const innerId = useUniqId();
-    const id = label ? originalId || innerId : originalId;
     const [hasVerticalScrollbar, setHasVerticalScrollbar] = React.useState(false);
 
     const isControlled = value !== undefined;
     const inputValue = isControlled ? value : uncontrolledValue;
     const isLabelVisible = !multiline && Boolean(label);
 
+    const innerId = useUniqId();
+    const id = isLabelVisible ? originalId || innerId : originalId;
+
+    const isAutoCompleteOff =
+        isLabelVisible && !originalId && !name && typeof autoComplete === 'undefined';
+
     const handleRef = useForkRef(props.controlRef, innerControlRef);
 
     const labelSize = useElementSize(isLabelVisible ? labelRef : null, size);
@@ -141,7 +145,7 @@ export const TextInput = React.forwardRef<HTMLSpanElement, TextInputProps>(funct
                 onUpdate(newValue);
             }
         },
-        autoComplete: prepareAutoComplete(autoComplete),
+        autoComplete: isAutoCompleteOff ? 'off' : prepareAutoComplete(autoComplete),
         controlProps,
     };
 


Test patch:
diff --git a/src/components/TextInput/__tests__/TextInput.input.test.tsx b/src/components/TextInput/__tests__/TextInput.input.test.tsx
new file mode 100644
index 00000000..1ab297e5
--- /dev/null
+++ b/src/components/TextInput/__tests__/TextInput.input.test.tsx
@@ -0,0 +1,167 @@
+import React from 'react';
+import userEvent from '@testing-library/user-event';
+import {render, screen, fireEvent} from '@testing-library/react';
+import {TextInput} from '../TextInput';
+
+describe('TextInput input', () => {
+    describe('without label prop', () => {
+        describe('basic', () => {
+            test('render input by default', () => {
+                render(<TextInput />);
+                const input = screen.getByRole('textbox');
+
+                expect(input).toBeVisible();
+                expect(input.tagName.toLowerCase()).toBe('input');
+            });
+
+            test('render error message with error prop', () => {
+                const {container} = render(<TextInput error="Some Error" />);
+
+                // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access
+                expect(container.querySelector('.yc-text-input__error')).toBeInTheDocument();
+                expect(screen.getByText('Some Error')).toBeVisible();
+            });
+
+            test('do not show error without error prop', () => {
+                const {container} = render(<TextInput />);
+
+                // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access
+                expect(container.querySelector('.yc-text-input__error')).not.toBeInTheDocument();
+            });
+
+            test('render clear button with hasClear prop', () => {
+                render(<TextInput hasClear />);
+
+                expect(screen.getByRole('button', {name: 'Clear input value'})).toBeInTheDocument();
+            });
+
+            test('do not render clear button without hasClear prop', () => {
+                render(<TextInput />);
+
+                expect(
+                    screen.queryByRole('button', {name: 'Clear input value'}),
+                ).not.toBeInTheDocument();
+            });
+
+            test('call onChange when input changes value', () => {
+                const onChangeFn = jest.fn();
+
+                render(<TextInput onChange={onChangeFn} />);
+                fireEvent.change(screen.getByRole('textbox'), {target: {value: '1'}});
+
+                expect(onChangeFn).toBeCalled();
+            });
+
+            test('call onUpdate with certain value when input changes value', () => {
+                const onUpdateFn = jest.fn();
+                const value = 'some';
+
+                render(<TextInput onUpdate={onUpdateFn} />);
+                fireEvent.change(screen.getByRole('textbox'), {target: {value}});
+
+                expect(onUpdateFn).toBeCalledWith(value);
+            });
+
+            test('call onChange when click to clean button', async () => {
+                const onChangeFn = jest.fn();
+                const user = userEvent.setup();
+                render(<TextInput hasClear onChange={onChangeFn} />);
+                const clear = screen.getByRole('button', {name: 'Clear input value'});
+
+                if (clear) {
+                    await user.click(clear);
+                }
+
+                expect(onChangeFn).toBeCalled();
+            });
+
+            test('call onUpdate with emply value when click to clean button', async () => {
+                const onUpdateFn = jest.fn();
+                const user = userEvent.setup();
+                render(<TextInput hasClear onUpdate={onUpdateFn} />);
+                const clear = screen.getByRole('button', {name: 'Clear input value'});
+
+                if (clear) {
+                    await user.click(clear);
+                }
+
+                expect(onUpdateFn).toBeCalledWith('');
+            });
+        });
+
+        describe('autocomplete', () => {
+            test('render no autocomplete attribute when no autoComplete, no id, no name props', () => {
+                render(<TextInput />);
+                const input = screen.getByRole('textbox');
+
+                expect(input.getAttribute('autocomplete')).toBeNull();
+            });
+
+            test('render autocomplete=on attribute with autoComplete prop', () => {
+                render(<TextInput autoComplete />);
+                const input = screen.getByRole('textbox');
+
+                expect(input.getAttribute('autocomplete')).toBe('on');
+            });
+
+            test('render autocomplete=off attribute with autoComplete=false prop', () => {
+                render(<TextInput autoComplete={false} />);
+                const input = screen.getByRole('textbox');
+
+                expect(input.getAttribute('autocomplete')).toBe('off');
+            });
+        });
+    });
+
+    describe('with label prop', () => {
+        describe('basic', () => {
+            test('render input with label', () => {
+                const {container} = render(<TextInput label="Label:" />);
+
+                // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access
+                const label = container.querySelector('.yc-text-input__label');
+
+                expect(label).toBeInTheDocument();
+                expect(label?.tagName.toLowerCase()).toBe('label');
+                expect(screen.getByText('Label:')).toBeVisible();
+            });
+        });
+
+        describe('autocomplete', () => {
+            test('render autocomplete=off attribute when no autoComplete, no id, no name props', () => {
+                render(<TextInput label="Label:" />);
+                const input = screen.getByRole('textbox');
+
+                expect(input.getAttribute('autocomplete')).toBe('off');
+            });
+
+            test('render no autocomplete attribute when no autoComplete prop, but id prop set', () => {
+                render(<TextInput id="yc-id" label="Label:" />);
+                const input = screen.getByRole('textbox');
+
+                expect(input.getAttribute('autocomplete')).toBeNull();
+            });
+
+            test('render no autocomplete attribute when no autoComplete prop, but name prop set', () => {
+                render(<TextInput name="yc-name" label="Label:" />);
+                const input = screen.getByRole('textbox');
+
+                expect(input.getAttribute('autocomplete')).toBeNull();
+            });
+
+            test('render autocomplete=on attribute when autoComplete prop "on"', () => {
+                render(<TextInput autoComplete="on" label="Label:" />);
+                const input = screen.getByRole('textbox');
+
+                expect(input.getAttribute('autocomplete')).toBe('on');
+            });
+
+            test('render autocomplete=off attribute when autoComplete prop "off"', () => {
+                render(<TextInput autoComplete="off" label="Label:" />);
+                const input = screen.getByRole('textbox');
+
+                expect(input.getAttribute('autocomplete')).toBe('off');
+            });
+        });
+    });
+});
diff --git a/src/components/TextInput/__tests__/TextInput.test.tsx b/src/components/TextInput/__tests__/TextInput.test.tsx
deleted file mode 100644
index b1060211..00000000
--- a/src/components/TextInput/__tests__/TextInput.test.tsx
+++ /dev/null
@@ -1,119 +0,0 @@
-import React from 'react';
-import userEvent from '@testing-library/user-event';
-import {render, screen, fireEvent} from '@testing-library/react';
-import {TextInput} from '../TextInput';
-
-describe('TextInput', () => {
-    test('render input by default', () => {
-        render(<TextInput />);
-        const input = screen.getByRole('textbox');
-
-        expect(input).toBeVisible();
-        expect(input.tagName.toLowerCase()).toBe('input');
-    });
-
-    test('render textarea with multiline prop', () => {
-        render(<TextInput multiline />);
-        const input = screen.getByRole('textbox');
-
-        expect(input).toBeVisible();
-        expect(input.tagName.toLowerCase()).toBe('textarea');
-    });
-
-    test('render input with label', () => {
-        const {container} = render(<TextInput label="Label:" />);
-
-        // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access
-        const label = container.querySelector('.yc-text-input__label');
-
-        expect(label).toBeInTheDocument();
-        expect(label?.tagName.toLowerCase()).toBe('label');
-        expect(screen.getByText('Label:')).toBeVisible();
-    });
-
-    test('render error message with error prop', () => {
-        const {container} = render(<TextInput error="Some Error" />);
-
-        // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access
-        expect(container.querySelector('.yc-text-input__error')).toBeInTheDocument();
-        expect(screen.getByText('Some Error')).toBeVisible();
-    });
-
-    test('do not show error without error prop', () => {
-        const {container} = render(<TextInput />);
-
-        // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access
-        expect(container.querySelector('.yc-text-input__error')).not.toBeInTheDocument();
-    });
-
-    test('render clear button with hasClear prop', () => {
-        render(<TextInput hasClear />);
-
-        expect(screen.getByRole('button', {name: 'Clear input value'})).toBeInTheDocument();
-    });
-
-    test('do not render clear button without hasClear prop', () => {
-        render(<TextInput />);
-
-        expect(screen.queryByRole('button', {name: 'Clear input value'})).not.toBeInTheDocument();
-    });
-
-    test('call onChange when input changes value', () => {
-        const onChangeFn = jest.fn();
-
-        render(<TextInput onChange={onChangeFn} />);
-        fireEvent.change(screen.getByRole('textbox'), {target: {value: '1'}});
-
-        expect(onChangeFn).toBeCalled();
-    });
-
-    test('call onUpdate with certain value when input changes value', () => {
-        const onUpdateFn = jest.fn();
-        const value = 'some';
-
-        render(<TextInput onUpdate={onUpdateFn} />);
-        fireEvent.change(screen.getByRole('textbox'), {target: {value}});
-
-        expect(onUpdateFn).toBeCalledWith(value);
-    });
-
-    test('call onChange when click to clean button', async () => {
-        const onChangeFn = jest.fn();
-        const user = userEvent.setup();
-        render(<TextInput hasClear onChange={onChangeFn} />);
-        const clear = screen.getByRole('button', {name: 'Clear input value'});
-
-        if (clear) {
-            await user.click(clear);
-        }
-
-        expect(onChangeFn).toBeCalled();
-    });
-
-    test('call onUpdate with emply value when click to clean button', async () => {
-        const onUpdateFn = jest.fn();
-        const user = userEvent.setup();
-        render(<TextInput hasClear onUpdate={onUpdateFn} />);
-        const clear = screen.getByRole('button', {name: 'Clear input value'});
-
-        if (clear) {
-            await user.click(clear);
-        }
-
-        expect(onUpdateFn).toBeCalledWith('');
-    });
-
-    test('render autocomplete=on attribute with autoComplete prop', () => {
-        render(<TextInput autoComplete />);
-        const input = screen.getByRole('textbox');
-
-        expect(input.getAttribute('autocomplete')).toBe('on');
-    });
-
-    test('render autocomplete=off attribute with autoComplete=false prop', () => {
-        render(<TextInput autoComplete={false} />);
-        const input = screen.getByRole('textbox');
-
-        expect(input.getAttribute('autocomplete')).toBe('off');
-    });
-});
diff --git a/src/components/TextInput/__tests__/TextInput.textarea.test.tsx b/src/components/TextInput/__tests__/TextInput.textarea.test.tsx
new file mode 100644
index 00000000..e8bdbbd7
--- /dev/null
+++ b/src/components/TextInput/__tests__/TextInput.textarea.test.tsx
@@ -0,0 +1,63 @@
+import React from 'react';
+import {render, screen} from '@testing-library/react';
+import {TextInput} from '../TextInput';
+
+describe('TextInput textarea', () => {
+    describe('without label prop', () => {
+        describe('basic', () => {
+            test('render textarea with multiline prop', () => {
+                render(<TextInput multiline />);
+                const input = screen.getByRole('textbox');
+
+                expect(input).toBeVisible();
+                expect(input.tagName.toLowerCase()).toBe('textarea');
+            });
+        });
+
+        describe('autocomplete', () => {
+            test('render no autocomplete attribute when no autoComplete, no id, no name props', () => {
+                render(<TextInput multiline />);
+                const input = screen.getByRole('textbox');
+
+                expect(input.getAttribute('autocomplete')).toBeNull();
+            });
+        });
+    });
+
+    describe('with label prop', () => {
+        describe('basic', () => {
+            test('render textarea without label', () => {
+                const {container} = render(<TextInput multiline label="Label:" />);
+
+                // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access
+                const label = container.querySelector('.yc-text-input__label');
+
+                expect(label).toBeNull();
+                expect(screen.queryByText('Label:')).toBeNull();
+            });
+        });
+
+        describe('autocomplete', () => {
+            test('render no autocomplete attribute when no autoComplete, no id, no name props', () => {
+                render(<TextInput multiline label="Label:" />);
+                const input = screen.getByRole('textbox');
+
+                expect(input.getAttribute('autocomplete')).toBeNull();
+            });
+
+            test('render no autocomplete attribute when no autoComplete prop, but id prop set', () => {
+                render(<TextInput multiline id="yc-id" label="Label:" />);
+                const input = screen.getByRole('textbox');
+
+                expect(input.getAttribute('autocomplete')).toBeNull();
+            });
+
+            test('render no autocomplete attribute when no autoComplete prop, but name prop set', () => {
+                render(<TextInput multiline name="yc-name" label="Label:" />);
+                const input = screen.getByRole('textbox');
+
+                expect(input.getAttribute('autocomplete')).toBeNull();
+            });
+        });
+    });
+});