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.

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

Editing one policy parameter affects other displayed parameters #705

Merged
merged 1 commit into from
Aug 27, 2023

Conversation

patrickhanl
Copy link
Contributor

@patrickhanl patrickhanl commented Aug 22, 2023

Fix 695

Update input field to use useEffect to clear the input field when focus changes & keep placeholder in state so that it does not change when user deletes their entry.

input-policy-parameters.mp4

🤖 Generated by Copilot at 1e5792e

Summary

🧪🔎🔄

This pull request improves the user experience and testing of the InputField component, which allows users to edit policy parameters. It uses the useSearchParams hook to control the focus and clear the input based on the URL query, and updates the tests to mock this hook. It also adds a useEffect hook to handle focus changes.

useSearchParams to control the field of doom
Clear the input and reset the placeholder of gloom
Run a side effect when the focus shifts
InputField tests mock the hook that lifts

Walkthrough

  • Add useSearchParams hook to InputField component and test file (link, link)
  • Mock useSearchParams hook in InputField test file and simulate different query parameters (link, link)
  • Wrap existing tests in InputField test file in a describe block with a clear label (link)
  • Add savedPlaceholder state variable to InputField component to store the initial placeholder prop (link)
  • Add useEffect hook to InputField component to clear the input field and reset the placeholder when the focus changes (link)
  • Replace placeholder prop with savedPlaceholder state variable in input element of InputField component (link)

Copy link
Contributor

@nikhilwoodruff nikhilwoodruff left a comment

Choose a reason for hiding this comment

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

This looks good to me and works locally too. Thanks @patrickhanl!

@nikhilwoodruff nikhilwoodruff merged commit 08d5edb into PolicyEngine:master Aug 27, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Editing one policy parameter affects other displayed parameters
2 participants