From c76774e83fefebc5c2b95dff81ef9c6a498ddca6 Mon Sep 17 00:00:00 2001 From: Jason Date: Thu, 21 Mar 2024 09:10:07 -0500 Subject: [PATCH] fix(react): prevent search field input from inheriting unintended styles from default TextField (#1425) --- .../SearchField/SearchField.test.tsx | 7 +-- .../src/components/SearchField/index.tsx | 10 ++-- .../TextFieldWrapper.test.tsx | 57 +++++++++++-------- packages/styles/text-field-wrapper.css | 1 + 4 files changed, 42 insertions(+), 33 deletions(-) diff --git a/packages/react/src/components/SearchField/SearchField.test.tsx b/packages/react/src/components/SearchField/SearchField.test.tsx index 19d9895fd..45b5591f6 100644 --- a/packages/react/src/components/SearchField/SearchField.test.tsx +++ b/packages/react/src/components/SearchField/SearchField.test.tsx @@ -144,10 +144,9 @@ test('should support ref prop', () => { test('should support className prop', () => { render({ label: 'search field', className: 'banana' }); - expect(screen.getByRole('searchbox', { name: 'search field' })).toHaveClass( - 'Field__text-input', - 'banana' - ); + expect( + screen.getByRole('searchbox', { name: 'search field' }).parentElement + ).toHaveClass('TextFieldWrapper', 'banana'); }); test('should support name prop', () => { diff --git a/packages/react/src/components/SearchField/index.tsx b/packages/react/src/components/SearchField/index.tsx index a93e0bab6..03c906296 100644 --- a/packages/react/src/components/SearchField/index.tsx +++ b/packages/react/src/components/SearchField/index.tsx @@ -27,6 +27,7 @@ interface SearchFieldProps const SearchField = forwardRef( ( { + className, label, defaultValue = '', onChange, @@ -36,7 +37,7 @@ const SearchField = forwardRef( id: propId, value: propValue, trailingChildren, - ...otherProps + ...inputProps }: SearchFieldProps, ref ) => { @@ -90,8 +91,8 @@ const SearchField = forwardRef( )} @@ -101,8 +102,7 @@ const SearchField = forwardRef( onChange={handleChange} placeholder={placeholder} ref={ref} - {...otherProps} - className={classNames(otherProps.className, 'Field__text-input')} + {...inputProps} type="search" /> {trailingChildren} diff --git a/packages/react/src/components/internal/TextFieldWrapper/TextFieldWrapper.test.tsx b/packages/react/src/components/internal/TextFieldWrapper/TextFieldWrapper.test.tsx index b951c6582..51e9ab300 100644 --- a/packages/react/src/components/internal/TextFieldWrapper/TextFieldWrapper.test.tsx +++ b/packages/react/src/components/internal/TextFieldWrapper/TextFieldWrapper.test.tsx @@ -1,51 +1,60 @@ import React from 'react'; -import { render as testingRender, screen } from '@testing-library/react'; +import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { spy } from 'sinon'; import { axe } from 'jest-axe'; import TextFieldWrapper, { TextFieldWrapperProps } from './'; -type RenderProps = Partial & { - [key: string]: any; -}; - -const render = ({ className, children, ...otherProps }: RenderProps = {}) => - testingRender( - - {children ||

Children

} +test('should render children', () => { + render( + +

Children

); - -test('should render children', () => { - render(); expect(screen.getByText('Children')).toBeInTheDocument(); }); -test('should render TextFieldWrapper with default className', () => { - render(); - expect(screen.getByText('Children').parentElement).toHaveClass( - 'TextFieldWrapper' +test('should support className prop', () => { + render( + + + ); -}); - -test('should render TextFieldWrapper with custom className', () => { - render({ className: 'banana' }); - expect(screen.getByText('Children').parentElement).toHaveClass( + expect(screen.getByRole('textbox').parentElement).toHaveClass( 'TextFieldWrapper', 'banana' ); }); +test('should support ref prop', () => { + const ref = React.createRef(); + render( + + + + ); + expect(ref.current).toBeDefined(); + expect(ref.current).toBeInstanceOf(HTMLDivElement); +}); + test('should render TextFieldWrapper with other props', () => { - render({ id: 'banana' }); - expect(screen.getByText('Children').parentElement).toHaveAttribute( + render( + + + + ); + expect(screen.getByRole('textbox').parentElement).toHaveAttribute( 'id', 'banana' ); }); test('should have no axe violations with TextFieldWrapper', async () => { - const { container } = render(); + const { container } = render( + +

Children

+
+ ); expect(await axe(container)).toHaveNoViolations(); }); diff --git a/packages/styles/text-field-wrapper.css b/packages/styles/text-field-wrapper.css index 07ad95164..fe7686232 100644 --- a/packages/styles/text-field-wrapper.css +++ b/packages/styles/text-field-wrapper.css @@ -55,6 +55,7 @@ margin-bottom: var(--space-half); box-shadow: inset 0 1px 2px rgba(0, 0, 0, 0.05); background-color: var(--text-field-wrapper-background-color); + min-width: min(var(--input-min-width), 100%); } .cauldron--theme-dark .TextFieldWrapper {