-
Notifications
You must be signed in to change notification settings - Fork 28
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
Remove jQuery from Instructor List (Fix #777 Part 1/2) #806
Conversation
@@ -1,4 +1,4 @@ | |||
import { render, screen, fireEvent } from '@testing-library/react'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just code cleanup on this file, nothing new
@@ -15,7 +15,7 @@ export default function InstructorList({ instructors, loading, currUni, onInacti | |||
const onSearch = (keyCode) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@angrave if we want, we can make the search happen 'live' with every state change. That way we won't need to even hit enter or click the button now that we're not using jQuery
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be a separate commit actually, I'll make this into a new issue so that this PR is fixed on just removing jquery.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good.
@harsh183
FYI how to properly debounce a search in react-
https://www.alexhughes.dev/blog/settimeout-with-hooks/
https://dev.to/imforja/5-steps-to-perform-a-search-when-user-stops-typing-using-react-hooks-in-a-controlled-component-5edd
For Instructor List we can replace $ find with a react state. I added test cases with a new library
@testing-library/user-event
to make sure we preserve the existing functionality of the view.