Add the ability to search courses in the modal

[donnapep]

Partial fix for #4959.

Changes proposed in this Pull Request

This PR adds the ability to search for specific courses in the modal.

Testing instructions

• Open the modal on the Students page.
• The spinner should be displayed while courses are being searched.
• Open the Network > Fetch/XHR tab in DevTools.
• Enter some text to search for.
• Depending on how fast you type, you should not see an API call for every keystroke. The API is called 400ms after you stop typing.
• Ensure that the height of the course list (bordered area) remains fixed and is not jumpy when moving between the following states:
  • When the loading spinner is displayed
  • When courses are displayed
  • When no courses are found

Screenshot / Video

Loading

No Courses

Course List

[donnapep]

@gabrielcaires I think I found another case where useCallback is a good idea (i.e. when debouncing).

[donnapep]

@Luchadores Please see the screen shots in the description. I set a fixed height of 300px on the box where the courses display to prevent the modal's height from growing / shrinking as the user searches for courses.

[Luchadores]

Hey @donnapep 👋 Looks good!

• Could we make the search bar a bit taller? 36px height should make it look better.
• The width of the entire modal seems quite big. I'm not sure as I'm not seeing it in context. But should be around 460px in width.

Thank you 🙌

[gabrielcaires]

@gabrielcaires I think I found [another case where useCallback is a good idea

Oh interesting , I never saw this kind of usage before! 🎉

@donnapep I think this PR just be integrated with the PR #5017 instead of feature/student-management

[merkushin]

Unfortunately, it doesn't work for me :(

CleanShot.2022-04-17.at.01.46.45.mp4

It looks like I get a response from WP, but it does not render the list.
Also, I noticed the modal looks weird: I see a scrollbar when open the inspector.

[donnapep]

It looks like I get a response from WP, but it does not render the list.

I had to move this back to Draft status a bit ago in order to get Gabriel's changes. I'm leaving it in Draft for now because the changes broke some things.

[donnapep]

@gabrielcaires You may be annoyed by me. 😳 I removed the isMounted check as it completely broke search (the component would unmount, and after the API call returned the isMounted.current test would not pass and the spinner would just keep spinning).

I opted to remove it since getting this PR moving felt more important than fixing the original React warning. I know you were looking into an alternate solution as well, so hopefully we can address it in a separate PR and find a solution that eliminates the original warning while also not impacting the search functionality.

[gabrielcaires]

After the removal, Are you seeing warning messages related to run set state after components was unmounted?

[donnapep]

After the removal, Are you seeing warning messages related to run set state after components was unmounted?

Yup, in the tests I do, and I'm aware that this was the original issue you were trying to fix.

[merkushin]

Works well 👍

On the list, I found we escape titles too many times:

Original titles:

Also, there is a conflict here.

[donnapep]

On the list, I found we escape titles too many times

I'll create a new card for this since it's unrelated to this particular PR. I resolved the conflict though.