manager
manager copied to clipboard
refactor: [M3-8186] - Clean up DebouncedSearchTextField and fix instances where debouncing is not happening
Description π
This PR fixes the issue of debouncing is not happening for the instances of DebouncedSearchTextField component.
Changes π
List any change relevant to the reviewer.
- Fix debounce issue in PlacementGroupsLanding.
- Fix debounce issue in LinodeSelectTable.
- Removed customeValue prop usage in LinodeSelectTable and SelectLinodePanel.
Target release date ποΈ
09/02
How to test π§ͺ
Verification steps
(How to verify changes)
- Checkout the branch and verify there is no regression for all the references of
PlacementGroupsLanding,SelectLinodePanelandLinodeSelectTable - Verify debounce behavior for
PlacementGroupsLandingandLinodeSelectTable
As an Author I have considered π€
Check all that apply
- [ ] π Doing a self review
- [ ] β Our contribution guidelines
- [ ] π€ Splitting feature into small PRs
- [ ] β Adding a changeset
- [ ] π§ͺ Providing/Improving test coverage
- [ ] π Removing all sensitive information from the code and PR description
- [ ] π© Using a feature flag to protect the release
- [ ] π£ Providing comprehensive reproduction steps
- [ ] π Providing or updating our documentation
- [ ] π Scheduling a pair reviewing session
- [ ] π± Providing mobile support
- [ ] βΏ Providing accessibility support
Coverage Report: β
Base Coverage: 86.15%
Current Coverage: 86.15%
@hkhalil-akamai @mjac0bs Great feedback.
@mjac0bs Good call, Actually, the root cause of the issue is having onSearch as a dependency in the useEffect, which is causing unwanted re-triggers. Removing it from the dependencies would prevent this from happening in the future. Also, I don't see the use case for having it in the dependency list. The only concern I see with renaming onSearch to onChange is that we might override the onChange prop of TextFieldPropsOverrides. In my opinion, onSearch is a wrapper for onChange in DebouncedSearchTextField. Let me know your thoughtsβI'm open to changing the prop name to onChange as well.
@mjac0bs Addressed the above feedback in the commit 9bb33ae Changes include
- Memoized the debounced onChange handler to prevent unnecessary re-creations.
- Improved the useEffect hook to handle empty values and reduce unnecessary state updates.
- removed redundant logic of the clear icon from the instanced of DebouncedSearchTextField.
The onSearch and value props are now required to ensure that the component can function correctly. The onSearch prop is essential for handling search. The value prop is necessary for managing the controlled input state. Making these props required enforces proper usage and helps prevent unintended behavior if they are accidentally omitted.
@mjac0bs fixed the console error in Longview a1b2e8f