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

fix: accept id and name props for Search component input field #183

Merged
merged 4 commits into from
May 20, 2020
Merged

fix: accept id and name props for Search component input field #183

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
745c5b0
fix: accept id and name props for Search component input field
duncan-truss May 19, 2020
6c72a1c
fix: qualify id and name props for input field to not clash with form
duncan-truss May 20, 2020
73e52a6
fix(yarn.lock): update yarn.lock from develop
duncan-truss May 20, 2020
7902c7e
fix(yarn.lock): revert yarn.lock to branch checkout commit
duncan-truss May 20, 2020
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
2 changes: 1 addition & 1 deletion src/components/Search/Search.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export default {
info: `
USWDS 2.0 Search component
Source: https://designsystem.digital.gov/components/header/
Source: https://designsystem.digital.gov/components/search/
`,
},
}
Expand Down
8 changes: 6 additions & 2 deletions src/components/Search/Search.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import { TextInput } from '../forms/TextInput/TextInput'

interface SearchInputProps {
onSubmit: (event: React.FormEvent<HTMLFormElement>) => void
id?: string
name?: string
big?: boolean
small?: boolean
label?: React.ReactNode
Expand All @@ -19,6 +21,8 @@ export const Search = (
): React.ReactElement => {
const {
onSubmit,
id = 'search-field',
Copy link
Contributor

@haworku haworku May 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Lets name this something like inputId since id is an existing HTML attribute that could be captured in...formProps and passed down to the <form> also.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep that occurred to me, thanks for the explanation. Would className ever clash as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

since className is part of the destructure assignment, it won't be included in formProps so it should be fine. if it wasn't destructured_ (and appended to the other classes using classnames) then yes, it could clash with the existing className prop, so it's important to handle that one specially :D

name = 'search',
big,
small,
label = 'Search',
Expand All @@ -41,10 +45,10 @@ export const Search = (
role="search"
search={true}
{...formProps}>
<Label srOnly={true} htmlFor="search-field">
<Label srOnly={true} htmlFor={id}>
{label}
</Label>
<TextInput id="search-field" type="search" name="search" />
<TextInput id={id} type="search" name={name} />
<Button type="submit">
<span className={small ? 'usa-sr-only' : 'usa-search__submit-text'}>
Search
Expand Down
30 changes: 0 additions & 30 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2404,16 +2404,6 @@
regexpp "^3.0.0"
tsutils "^3.17.1"

"@typescript-eslint/experimental-utils@2.32.0":
version "2.32.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion - delete the yarn.lock if you didn't change any dependencies in the PR. Maybe we need to make a separate PR to update develop but there's also a release in flight that will get merged back down ... hoping that will clear this out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 I'll revert it to what's on develop

resolved "https://registry.yarnpkg.com/@typescript-eslint/experimental-utils/-/experimental-utils-2.32.0.tgz#bee7fbe1d21d13a273066d70abc82549d0b7943e"
integrity sha512-oDWuB2q5AXsQ/mLq2N4qtWiBASWXPf7KhqXgeGH4QsyVKx+km8F6Vfqd3bspJQyhyCqxcbLO/jKJuIV3DzHZ6A==
dependencies:
"@types/json-schema" "^7.0.3"
"@typescript-eslint/typescript-estree" "2.32.0"
eslint-scope "^5.0.0"
eslint-utils "^2.0.0"

"@typescript-eslint/experimental-utils@2.34.0":
version "2.34.0"
resolved "https://registry.yarnpkg.com/@typescript-eslint/experimental-utils/-/experimental-utils-2.34.0.tgz#d3524b644cdb40eebceca67f8cf3e4cc9c8f980f"
Expand All @@ -2434,19 +2424,6 @@
"@typescript-eslint/typescript-estree" "2.34.0"
eslint-visitor-keys "^1.1.0"

"@typescript-eslint/typescript-estree@2.32.0":
version "2.32.0"
resolved "https://registry.yarnpkg.com/@typescript-eslint/typescript-estree/-/typescript-estree-2.32.0.tgz#0e4ae2e883557f94039b13ac0ecfcfbb09835b8d"
integrity sha512-hQpbWM/Y2iq6jB9FHYJBqa3h1R9IEGodOtajhb261cVHt9cz30AKjXM6WP7LxJdEPPlyJ9rPTZVgBUgZgiyPgw==
dependencies:
debug "^4.1.1"
eslint-visitor-keys "^1.1.0"
glob "^7.1.6"
is-glob "^4.0.1"
lodash "^4.17.15"
semver "^7.3.2"
tsutils "^3.17.1"

"@typescript-eslint/typescript-estree@2.34.0":
version "2.34.0"
resolved "https://registry.yarnpkg.com/@typescript-eslint/typescript-estree/-/typescript-estree-2.34.0.tgz#14aeb6353b39ef0732cc7f1b8285294937cf37d5"
Expand Down Expand Up @@ -9040,13 +9017,6 @@ log-symbols@^2.1.0, log-symbols@^2.2.0:
dependencies:
chalk "^2.0.1"

log-symbols@^3.0.0:
version "3.0.0"
resolved "https://registry.yarnpkg.com/log-symbols/-/log-symbols-3.0.0.tgz#f3a08516a5dea893336a7dee14d18a1cfdab77c4"
integrity sha512-dSkNGuI7iG3mfvDzUuYZyvk5dD9ocYCYzNU6CYDE6+Xqd+gwme6Z00NS3dUh8mq/73HaEtT7m6W+yUPtU6BZnQ==
dependencies:
chalk "^2.4.2"

log-symbols@^4.0.0:
version "4.0.0"
resolved "https://registry.yarnpkg.com/log-symbols/-/log-symbols-4.0.0.tgz#69b3cc46d20f448eccdb75ea1fa733d9e821c920"
Expand Down