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

Fixed Debounce in AddClaimantModal #14454

Merged
merged 52 commits into from
Jun 8, 2020
Merged

Fixed Debounce in AddClaimantModal #14454

merged 52 commits into from
Jun 8, 2020

Conversation

jcq
Copy link
Contributor

@jcq jcq commented Jun 2, 2020

Connects #14430

Description

Fixes regression from #14202; this brings fully functioning debounce functionality in AddClaimantModal along with additional JS tests

Acceptance Criteria

  • Code compiles correctly
  • API calls are properly debounced in AddClaimantModal

JC Quirin and others added 30 commits May 1, 2020 14:40
(removes mention of Pulac Cerullo in URL)
added proptypes;
refactored into functional component
added story for `AddClaimantModal`;
updated `SelectClaimant` to allow opening of `AddClaimantModal` if `attorneyFees` feature toggle is enabled;
added ADD_CLAIMANT_MODAL_TITLE to COPY
(avoids errors with async functions at top level of files)
(default — optionally supports a different async fn as a prop)
added basic local/mock data & async search fn in `AddClaimantModal` story;
@va-bot
Copy link
Collaborator

va-bot commented Jun 2, 2020

1 Warning
⚠️ This PR modifies React components — consider adding/updating corresponding Storybook file

Generated by 🚫 Danger

@codeclimate
Copy link

codeclimate bot commented Jun 2, 2020

Code Climate has analyzed commit a86b33c and detected 0 issues on this pull request.

View more on Code Climate.

Copy link
Contributor

@leikkisa leikkisa left a comment

Choose a reason for hiding this comment

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

I am not 100% sure if I completely understand the behavior that was happening with the bug, I added a comment to the test. I think it'll make sense for Yang to check this out too since he's more familiar, but so far looks good to me.

Copy link
Contributor

@nanotone nanotone left a comment

Choose a reason for hiding this comment

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

One suggestion for the debounce test, but after that I'd say this is 👍 to go!

TIL that debounce comes with lodash, and leading vs trailing edges.

@jcq jcq added the Ready-to-Merge This PR is ready to be merged and will be picked up by va-bot to automatically merge to master label Jun 8, 2020
@va-bot va-bot merged commit e94dc74 into master Jun 8, 2020
@va-bot va-bot deleted the jc/14153-fix-debounce branch June 8, 2020 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready-to-Merge This PR is ready to be merged and will be picked up by va-bot to automatically merge to master Team: Foxtrot 🦊
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants