Instructor Search #137
Instructor Search #137
Conversation
…gle professor entry by hiding loading for professors list but showing it for individual pages
|
@Lhwhatever have made the refactoring from "professor" to "instructors". Have also used fuse.js for fuzzy searching. It's not perfect but it may be slightly better than the previous version. Also refactored some code so that searching is speedier so now it should be just as fast (if not faster) than the course search. Do let me know if there are any other comments! |
frontend/src/app/cache.ts
Outdated
| includeScore: true, | ||
| }; | ||
|
|
||
| const fuse = new Fuse(state.allInstructors, options); |
There was a problem hiding this comment.
not sure if this could be a global variable --- is this expensive to instantiate for every search?
frontend/src/app/cache.ts
Outdated
| @@ -45,6 +52,11 @@ const initialState: CacheState = { | |||
| coursesLoading: false, | |||
| exactResultsCourses: [], | |||
| allCourses: [], | |||
| allInstructors: [], | |||
| fuseIndex: null, | |||
There was a problem hiding this comment.
the type for fuseIndex seems not to be nullable here?
| state.instructorsLoading = false; | ||
| if (action.payload) { | ||
| state.allInstructors = action.payload; | ||
| state.fuseIndex = Fuse.createIndex(["instructor"], action.payload).toJSON(); |
There was a problem hiding this comment.
I'm not too sure on the Fuse library API so I will trust this here
There was a problem hiding this comment.
Was basing it of this https://www.fusejs.io/api/indexing.html#fuse-createindex
Description
How Has This Been Tested?
To test, set serializableCheck in store.ts:81 to false so that you don't get this error:
Test Configuration:
Checklist: