Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Update Search component to support i18n #1192

Merged
merged 7 commits into from
May 5, 2021
Merged

feat: Update Search component to support i18n #1192

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ebbc678
Update Search component to pass in label instead of hardcoded string …
SirenaBorracha Apr 27, 2021
fefb023
Merge branch 'main' into ak-add-i18n-support-tosearch-component-1103
SirenaBorracha Apr 27, 2021
d64c85d
Merge branch 'main' into ak-add-i18n-support-tosearch-component-1103
SirenaBorracha Apr 28, 2021
cdd22c7
Simplify localization interface, update test, update Storybook examples
SirenaBorracha Apr 29, 2021
c76bf7a
Merge branch 'main' into ak-add-i18n-support-tosearch-component-1103
SirenaBorracha Apr 30, 2021
c5dfc77
Merge branch 'main' into ak-add-i18n-support-tosearch-component-1103
SirenaBorracha Apr 30, 2021
d87613b
Merge branch 'main' into ak-add-i18n-support-tosearch-component-1103
SirenaBorracha May 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions src/components/Search/Search.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ const mockSubmit = (): void => {
/* mock submit fn */
}

const sampleLocalization = {
buttonText: 'Buscar',
}

export const defaultSearch = (): React.ReactElement => (
<Search onSubmit={mockSubmit} />
)
Expand All @@ -36,3 +40,20 @@ export const smallSearch = (): React.ReactElement => (
onSubmit={mockSubmit}
/>
)

export const defaultSpanishSearch = (): React.ReactElement => (
<Search onSubmit={mockSubmit} i18n={sampleLocalization} />
)

export const bigSpanishSearch = (): React.ReactElement => (
<Search size="big" onSubmit={mockSubmit} i18n={sampleLocalization} />
)

export const smallSpanishSearch = (): React.ReactElement => (
<Search
placeholder="(Optional) Spanish Placeholder Text"
size="small"
onSubmit={mockSubmit}
i18n={sampleLocalization}
/>
)
16 changes: 16 additions & 0 deletions src/components/Search/Search.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ jest.mock('../../deprecation')
import { deprecationWarning } from '../../deprecation'
import { Search } from './Search'

const sampleLocalization = {
buttonText: 'Buscar',
}

describe('Search component', () => {
it('renders without errors', () => {
const mockSubmit = jest.fn()
Expand Down Expand Up @@ -32,6 +36,18 @@ describe('Search component', () => {
)
})

it('renders a label', () => {
const mockSubmit = jest.fn()
const { queryByLabelText } = render(
<Search
onSubmit={mockSubmit}
label="Buscar"
i18n={sampleLocalization}></Search>
)

expect(queryByLabelText('Buscar')).toBeInTheDocument()
})

describe('renders size classes', () => {
beforeEach(() => {
jest.clearAllMocks()
Expand Down
22 changes: 15 additions & 7 deletions src/components/Search/Search.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ import { Form, OptionalFormProps } from '../forms/Form/Form'
import { Label } from '../forms/Label/Label'
import { TextInput } from '../forms/TextInput/TextInput'

interface SearchLocalization {
buttonText: string
}

interface SearchInputProps {
onSubmit: (event: React.FormEvent<HTMLFormElement>) => void
inputId?: string
inputName?: string
size?: 'big' | 'small'
/**
* @deprecated since 1.6.0, use size
Expand All @@ -20,21 +22,25 @@ interface SearchInputProps {
* @deprecated since 1.6.0, use size
*/
small?: boolean
label?: React.ReactNode
className?: string
inputName?: string
inputId?: string
placeholder?: string
label?: React.ReactNode
i18n?: SearchLocalization
}

export const Search = ({
onSubmit,
inputId = 'search-field',
inputName = 'search',
size,
big,
small,
label = 'Search',
className,
placeholder,
inputName = 'search',
label = 'Search',
inputId = 'search-field',
i18n,
...formProps
}: SearchInputProps & OptionalFormProps): React.ReactElement => {
if (big) {
Expand All @@ -44,6 +50,8 @@ export const Search = ({
deprecationWarning('Search property small is deprecated. Use size')
}

const buttonText = i18n?.buttonText || 'Search'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want, you can even declare a default for i18n (such as the EN_US used in DatePicker). Since this is only one field within i18n, I'm good with it as is.


const isBig = size ? size === 'big' : big
const isSmall = size ? size === 'small' : small
const classes = classnames(
Expand Down Expand Up @@ -73,7 +81,7 @@ export const Search = ({
/>
<Button type="submit">
<span className={isSmall ? 'usa-sr-only' : 'usa-search__submit-text'}>
Search
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So while this works and may be perfectly acceptable, I can see a use case where someone may want to the Button text to be different from the label.

There's also the thought of following convention for localizations which we established in #990. There we added an i18n object prop which contained all the localization values. Since we only have one value to worry about here, that might feel like a bit much.

My overall preference with this is to follow convention, even if it feels like overkill, in the name of uniform codebase. Plus, to handle the scenario of wanting label and button text to be different we would have to introduce a new prop anyways, so that would be half way to implementing our established i18n pattern.

{buttonText}
</span>
</Button>
</Form>
Expand Down