feat: set icons for each type of notification #17115
Conversation
| @@ -61,6 +61,7 @@ export const Markdown: Story = { | |||
| url: "https://dev.coder.com/workspaces?filter=template%3Acoder-with-ai", | |||
| }, | |||
| ], | |||
| icon: "DEFAULT_TEMPLATE_ICON", | |||
There was a problem hiding this comment.
@BrunoQuaresma We changed the format to "DEFAULT_ICON_*".
| }; | ||
|
|
||
| export const InboxAvatar: FC<InboxAvatarProps> = ({ icon }) => { | ||
| return <Avatar variant="icon">{inboxIcons[icon]}</Avatar>; |
There was a problem hiding this comment.
We need to make it future proof, so theoretically icon can be either a custom image link or "DEFAULT_ICON_*"
There was a problem hiding this comment.
Since we don't have plans for that I would not add that. I really like to avoid adding code to handle scenarios were are not planned to happen any time soon.
There was a problem hiding this comment.
Actually this is how backend is implemented, so when the notification icon has custom url, backend will return it. Frontend must be compliant here.
|
@mtojek I updated the codersdk types to better represent the logic and values. |
codersdk/inboxnotification.go
Outdated
| Targets []uuid.UUID `json:"targets" format:"uuid"` | ||
| Title string `json:"title"` | ||
| Content string `json:"content"` | ||
| Icon InboxNotificationFallbackIcon `json:"icon"` |
There was a problem hiding this comment.
I understand the idea behind - and love it. Unfortunately due to the fact that it can be a value different from the const list defined - I am not sure we want to keep it.
There was a problem hiding this comment.
Reverting the enums....
There was a problem hiding this comment.
An alternative option could be introducing a second property IconPath. Then, Icon can be used for default/fallback icons, and IconPath for custom src.
We can think about it in a follow up, not sure if this will make the API clean.
|
/cherry-pick release/2.21 |
Each notification type will have an icon to represent the context:
This depends on #17013