Add county inputs #2429
Merged
Add county inputs #2429
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new county input component for US households, enabling users to select a county that is then mapped to its corresponding FIPS code.
- Added a new County input component (County.jsx) that displays and filters counties based on the selected state.
- Updated VariableSearch and HouseholdPage to integrate the new county input.
- Added helper functions in the counties data file and corresponding tests.
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/pages/household/input/County.jsx | New county input component with county list filtering and FIPS mapping. |
| src/pages/household/VariableSearch.jsx | Added a new search option for county input, though the identifier is inconsistent with other components. |
| src/pages/HouseholdPage.jsx | Integrated the county input component and updated display tree logic. |
| src/data/counties.js | Provided county data and mapping functionality using the FIPS package. |
| src/tests/pages/household/input/County.test.js | Added tests for the County component to ensure correct rendering and behavior. |
Files not reviewed (1)
- package.json: Language not supported
Comments suppressed due to low confidence (1)
src/pages/household/VariableSearch.jsx:17
- The option for the county input is using the value 'input.household.countyName', but the County component and display tree use 'input.geography.countyName'. Consider changing the option value to 'input.geography.countyName' for consistency.
options.push({
nikhilwoodruff
approved these changes
Apr 2, 2025
Collaborator
nikhilwoodruff
left a comment
nikhilwoodruff
left a comment
There was a problem hiding this comment.
LGTM on addressing comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Fixes #2413.
Requires PolicyEngine/policyengine-us-data#195, then PolicyEngine/policyengine-us#5735 to be merged.
Note: this cannot yet be merged, as the API update that incorporates the changes from PolicyEngine/policyengine-us#5735 hasn't yet deployed successfully. Will revert to draft until that's complete.
Changes
This PR introduces a new household input component, "County name," that allows a user to select a county from a dropdown, then maps that behind the scenes to a FIPS code to allow for easier maintenance and fewer bugs for either deprecated counties or counties with special characters or spelling inconsistencies (including Mayagüez Municipio, PR; Windham County, CT; and LeFlore/Le Flore County, OK).
Note for reviewers: Would appreciate attention applied to some of the manual overriding I did to enable this component to exist, despite it not coinciding with a specific input variable. I wanted to still allow the inputting of county FIPS directly, hence why I didn't just replace that input with this one, but also didn't want to add it as a default input, hence some of the manual overrides.
Screenshots
Please see the Vercel launch below. If specific screenshots are desired, happy to add.
Tests
Tests have been added to ensure that the component properly displays counties and properly filters counties down to a state input, if one is given. Due to the limitations of Ant Design and
react-router-dom, more technical unit tests have not been added.