Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
frontend/types.ts
Outdated
| email?: string; | ||
| }; | ||
|
|
||
| export type Notification = Answer | Comment; |
There was a problem hiding this comment.
Actually, it might be a better idea to use the Notification type as we have it under the models. Sorry for the confusion!
| <Pane className={styles.notificationIcon}> | ||
| {isAnswer(notification) ? ( | ||
| <MdQuestionAnswer className={styles.notificationIconSVG} /> | ||
| ) : ( | ||
| <MdOutlineQuestionAnswer | ||
| className={styles.notificationIconSVG} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
Can we put the conditional rendering out of the return? Maybe conditional rendering at the top where the consts are, and use that variable here
| <Pane className={styles.notificationContent}> | ||
| <Text className={styles.notificationText}> | ||
| {isAnswer(notification) ? ( | ||
| <> |
There was a problem hiding this comment.
Same comment as above^
frontend/types.ts
Outdated
| export type Notification = { | ||
| id: number; | ||
| user: HoagieUser; | ||
| question: number; |
There was a problem hiding this comment.
Let's make the question, answer, and comment actually of type Question, Answer, and Comment. With this change, we don't need to query the backend when rendering the notifs
| Icon: IconType; | ||
| title: string; | ||
| } { | ||
| const iconMap: Record<NotificationType, IconType> = { |
There was a problem hiding this comment.
Maybe pull these maps out as constants
2 participants
References
Proposed Changes