Improvement: Enable Searching For Courses By Course Number #501
Conversation
|
@caramurphyy is attempting to deploy a commit to the atcupps' projects Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Mostly LGTM. Please read my comments and consider whether or not to require a minimum digit length for the course number when searching (I am in favor of requiring all three digits, but if you think it would be useful to let users search with 1 or 2 digits then the choice is up to you). Also, please dynamically generate the deptList as noted in my other comment.
Thank you Cara for your first contribution to Jupiterp!
Lessons for me from reviewing this PR:
- I need to improve my own documentation possibly with some examples
- This would be a lot easier with SQL than a huge JSON file
| } | ||
|
|
||
| // If the search input is just numbers, match courses with that number | ||
| const deptList = ["AASP", "AAST", "ABRM", "AGNR", "AGST", "AMSC", "AMST", "ANSC", "ANTH", "AOSC", "ARAB", "ARCH", "AREC", "ARHU", "ARMY", "ARSC", "ARTH", "ARTT", "ASTR", "BCHM", "BIOE", "BIOI", "BIOL", "BIOM", "BIPH", "BISI", "BMGT", "BMSO", "BSCI", "BSOS", "BSST", "BUAC", "BUDT", "BUFN", "BULM", "BUMK", "BUSI", "BUSM", "BUSO", "CBMG", "CCJS", "CHBE", "CHEM", "CHIN", "CHPH", "CHSE", "CINE", "CLAS", "CLFS", "CMLT", "CMSC", "COMM", "CPBE", "CPCV", "CPET", "CPGH", "CPJT", "CPMS", "CPPL", "CPSA", "CPSF", "CPSG", "CPSN", "CPSP", "CPSS", "DANC", "DATA", "ECON", "EDCP", "EDHD", "EDHI", "EDMS", "EDSP", "EDUC", "EMBA", "ENAE", "ENBC", "ENCE", "ENCO", "ENEB", "ENEE", "ENES", "ENFP", "ENGL", "ENMA", "ENME", "ENPM", "ENRE", "ENSE", "ENSP", "ENST", "ENTM", "ENTS", "EPIB", "FGSM", "FIRE", "FMSC", "FREN", "GBHL", "GEMS", "GEOG", "GEOL", "GERS", "GREK", "GVPT", "HACS", "HBUS", "HDCC", "HEBR", "HESI", "HESP", "HGLO", "HHUM", "HISP", "HIST", "HLSA", "HLSC", "HLTH", "HNUH", "HONR", "IDEA", "IMDM", "IMMR", "INAG", "INFM", "INST", "ISRL", "ITAL", "JAPN", "JOUR", "JWST", "KNES", "KORA", "LACS", "LARC", "LATN", "LBSC", "LEAD", "LGBT", "LING", "MAIT", "MATH", "MEES", "MIEH", "MITH", "MLAW", "MSML", "MSQC", "MUED", "MUSC", "NACS", "NAVY", "NEUR", "NFSC", "NIAP", "NIAV", "PEER", "PERS", "PHIL", "PHPE", "PHSC", "PHYS", "PLCY", "PLSC", "PORT", "PSYC", "RDEV", "RELS", "RUSS", "SLAA", "SLLC", "SMLP", "SOCY", "SPAN", "SPHL", "STAT", "SURV", "TDPS", "THET", "TLPL", "TLTC", "UMEI", "UNIV", "URSP", "USLT", "VMSC", "WEID", "WGSS", "XPER"]; |
There was a problem hiding this comment.
Issue: These department codes can change multiple times per semester, so having this hardcoded will likely require more maintenance to update it manually in the future. Course-search already has this data, just not in the format that we want; in CourseSearch.svelte, which calls this function, the depts: Department[] variable contains Department objects where the field name is the four letter department code listed here iirc. So maybe you could use that to create deptList within CourseSearch.svelte and pass it to this function as an argument?
There was a problem hiding this comment.
Oh awesome, I changed it to use that and pass it as a param
|
|
||
| // If the search input is just numbers, match courses with that number | ||
| const deptList = ["AASP", "AAST", "ABRM", "AGNR", "AGST", "AMSC", "AMST", "ANSC", "ANTH", "AOSC", "ARAB", "ARCH", "AREC", "ARHU", "ARMY", "ARSC", "ARTH", "ARTT", "ASTR", "BCHM", "BIOE", "BIOI", "BIOL", "BIOM", "BIPH", "BISI", "BMGT", "BMSO", "BSCI", "BSOS", "BSST", "BUAC", "BUDT", "BUFN", "BULM", "BUMK", "BUSI", "BUSM", "BUSO", "CBMG", "CCJS", "CHBE", "CHEM", "CHIN", "CHPH", "CHSE", "CINE", "CLAS", "CLFS", "CMLT", "CMSC", "COMM", "CPBE", "CPCV", "CPET", "CPGH", "CPJT", "CPMS", "CPPL", "CPSA", "CPSF", "CPSG", "CPSN", "CPSP", "CPSS", "DANC", "DATA", "ECON", "EDCP", "EDHD", "EDHI", "EDMS", "EDSP", "EDUC", "EMBA", "ENAE", "ENBC", "ENCE", "ENCO", "ENEB", "ENEE", "ENES", "ENFP", "ENGL", "ENMA", "ENME", "ENPM", "ENRE", "ENSE", "ENSP", "ENST", "ENTM", "ENTS", "EPIB", "FGSM", "FIRE", "FMSC", "FREN", "GBHL", "GEMS", "GEOG", "GEOL", "GERS", "GREK", "GVPT", "HACS", "HBUS", "HDCC", "HEBR", "HESI", "HESP", "HGLO", "HHUM", "HISP", "HIST", "HLSA", "HLSC", "HLTH", "HNUH", "HONR", "IDEA", "IMDM", "IMMR", "INAG", "INFM", "INST", "ISRL", "ITAL", "JAPN", "JOUR", "JWST", "KNES", "KORA", "LACS", "LARC", "LATN", "LBSC", "LEAD", "LGBT", "LING", "MAIT", "MATH", "MEES", "MIEH", "MITH", "MLAW", "MSML", "MSQC", "MUED", "MUSC", "NACS", "NAVY", "NEUR", "NFSC", "NIAP", "NIAV", "PEER", "PERS", "PHIL", "PHPE", "PHSC", "PHYS", "PLCY", "PLSC", "PORT", "PSYC", "RDEV", "RELS", "RUSS", "SLAA", "SLLC", "SMLP", "SOCY", "SPAN", "SPHL", "STAT", "SURV", "TDPS", "THET", "TLPL", "TLTC", "UMEI", "UNIV", "URSP", "USLT", "VMSC", "WEID", "WGSS", "XPER"]; | ||
| if (simpleInput.length >= 1 && /^[0-9]+$/i.test(simpleInput)) { |
There was a problem hiding this comment.
Suggestion: When I tested your PR on my device, the response time when I searched for one number (ex. "4") was quite slow, but it was much more tolerable when I had two or three numbers (ex. "417"). I think it might be better to change the condition to be that length >= 2 or length == 3, since searching when the user types only one number will probably slow down the overall user experience especially on less powerful machines and probably wouldn't be very useful since it would show so many courses (at least in my mind, lmk if you disagree).
There was a problem hiding this comment.
hmm yes, I agree I changed it to >=2
Description
I typed 417 and wanted CMSC417 to come up, instead I had to delete 417 and type the whole thing! Too much time. Need courses that match the number to pop up when a course number is searched. This PR also shows partial matches so they can they any length of number.
Changes
searchCoursesfunction inCourseSearch.tsxto check for numeric input then search for courses with that numberTesting