-
Notifications
You must be signed in to change notification settings - Fork 141
Made "County name" a searchable component on US site #2679
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.
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
base: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your work on this @v-lerie, just requested some minor changes.
|
||
export default function VariableSearch(props) { | ||
const { metadata, callback } = props; | ||
const countryId = window.location.pathname.split("/")[1]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue, minor: If possible at this point in the code, use the useCountryId
hook, which you'll find used in many other components, or pull the country ID from the metadata
that you have passed down (I believe it's metadata.countryId
).
value: | ||
countryId === "us" | ||
? "input.geography.countyName" | ||
: "input.household.countyName", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue, blocking: We don't want to push input.household.countyName
outside of the US context, we just don't want to modify it at all if it's present for some reason
If we add it the way you do here, it'll show up on our UK and Canadian sites, where it's actually not an option.
Description
Fixes #2604
Screenshots