Refactor SuggestionsList to use hooks and change class to function component #36924
Conversation
…mponent Introduced functional component. In the component I'm using useState, useEffect. Renamed a few variables to get rid of syntax errors. Tested the component and it is working as expected.
ZebulanStanphill
added
[Package] Components
There was an error saying scrollIntoView function not found. I was not importing it properly now in this commit I have renamed scrollIntoView function from dom-scroll-into-view package is renamed as scrollView. I had to do this because there is already props with the same name ie scrollIntoView. Component test case was failing because of issue on the same lines that is now fixed. There was another error related to not using act from react-dom/test-utils package in test case. I have fixed it. All component test case are passing now.
There was a problem hiding this comment.
Thank you @amustaque97 for working on this ✌️
I'm going to give this a proper look later (either by end of day or on Friday), but I gave a quick look and code changes overall look good to me.
|
Hey @amustaque97, sorry if I haven't managed to take a proper look at this yet, but I had a few unforeseen PRs that took precedence. I will probably be able to do to in the new year, since I'm going to be AFK in the next couple of weeks. Hope that's ok! |
- Add `noop` to reduce number of functions creation - Move emptyList to a const then use it - Move `handleMouseDown` to top of the file to avoid recreating it on every render
|
All review comments looks logical to me. I have addressed review comments. Thank You for the review. 😄 |
There was a problem hiding this comment.
Thank you for working on this, @amustaque97 !
I took some time, had a proper look at the code and left some comments
There was a problem hiding this comment.
Thank you for addressing the feedback! Not crucial, but in the future it would be even better if you could split different sets of changes over separate commits
I just left a minor comment, otherwise the code changes LGTM and tests well.
Could you also add an entry to the CHANGELOG ?
Description
Introduced functional component. In the component I'm using useState, useEffect.
Renamed a few variables to get rid of syntax errors/conflicts. Tested the component and
it is working as expected.
How has this been tested?
2. Create multiple tagsEdit or create new post.
In the settings, try to add new tags.
It will show suggestion list drop-down
Screenshots
Types of changes
Code Quality / Refactor
Checklist:
*.native.jsfiles for terms that need renaming or removal).