Course navigation sidebar #31
Conversation
locmai1
requested review from
johnny-t06,
nickbar01234,
wkim10 and
lnguyen2693
| const formattedCourseName = (course: IdentifiableCourse) => { | ||
| const formattedCourseId = course.id | ||
| .toUpperCase() | ||
| .replace(/([a-zA-Z]+)(\d+)/, "$1 $2"); |
There was a problem hiding this comment.
Double checking - should there be a space?
There was a problem hiding this comment.
Yeah I think so, according to Figma at least:
There was a problem hiding this comment.
I see some wireframe has it together - might be worth confirming with Fa and Sarah
src/app/components/Sidebar.tsx
Outdated
|
|
||
| React.useEffect(() => { | ||
| const loadCourses = async () => { | ||
| const allCourses = await getCourses(); |
There was a problem hiding this comment.
We probably should do the filtering on database side.
There was a problem hiding this comment.
is that making a server component?
There was a problem hiding this comment.
The query to firebase should already filter to the course ids we're interested in.
|
Double checking, we need to place this in |
According to the Figma, I don't think so? there's no Menu Icon |
src/app/services/client/course.ts
Outdated
| export const getCoursesByIds = async ( | ||
| courseIds: String[] | ||
| ): Promise<IdentifiableCourse[]> => { | ||
| const courses = courseIds.map((id) => getCourse(id)); |
There was a problem hiding this comment.
I think firestore should support filtering https://cloud.google.com/firestore/docs/query-data/queries#in_not-in_and_array-contains-any
Description
Added functionality to the MenuIcon. Only existing right now on 4 pages so far:
private/courseprivate/course/[courseId]private/course/[courseId]/queueprivate/course/[courseId]/profileIssues
None :)
Screenshots
Behavior on
[email protected]account (has all courses cs160 & cs40):Behavior on
[email protected]account (has only cs40):Should appear as Drawer upon clicking MenuIcon on the Header
Test
Test all courses in sidebar + manage courses on each of the 4 routes:
/private/course/[courseId]/profile/edit(not implemented yet but I think it's supposed to be like that according to Figma?)/private/course/cs160and presscs40, it should route to/private/course/cs40Possible Downsides
None :) (that I can think of)
Additional Documentations
Not sure that I did this the best way: used memo and a provider to reduce props drilling and should only re-render on change to userSession doc. Please let me know!