lyft / clutch

Extensible platform for infrastructure management
https://clutch.sh
Apache License 2.0
1.67k stars 116 forks source link

frontend: Support for Workflow / Entire App Notification #3084

Closed lucechal14 closed 1 week ago

lucechal14 commented 1 month ago

Description

Support for Notifications across the app. We have 3 types of notifications:

  1. Header: displayed in the header of the page with dynamic message, link and severity.
  2. Per Workflow: displayed at the top of the main section of the page for each workflow specified in the configuration of the notifications with dynamic title, message, link and severity per workflow.
  3. Multi Workflow: displayed at the top of the main section of the page for the workflows specified in the configuration of the notifications with dynamic title, message, link and severity.

Examples

Testing

You can use an object similar to this to test:

const banners = { header: { message: "header message", linkText: "link", link: "linkhere", severity: "info", }, perWorkflow: { workflowName1: { title: "per workflow title for test 2", message: "per workflow message for test 2", linkText: "link", link: "linkhere", severity: "info", }, workflowName2: { title: "per workflow title for test 3", message: "per workflow message for test 3", linkText: "link", link: "linkhere", severity: "info", }, }, multiWorkflow: { title: "multi workflow title", message: "multi workflow message", workflows: ["workflowName1", "workflowName3"], severity: "info", linkText: "link", link: "linkhere", }, };

Send this const as banners in the appConfiguration prop of your ClutchApp

jecr commented 1 week ago

LGTM, the only thing that feels odd is using the workflow's display name as key since it could be anything, didn't see an obvious alternative though 😅

jecr commented 1 week ago

Kind of an edge case for the header alert, a long link text can displace the description message. Also, long description messages don't take the available height and overflow at 2 lines. I don't think the component should hold huge chunks of text like that, but some handling could come handy just in case.

image
lucechal14 commented 1 week ago

Kind of an edge case for the header alert, a long link text can displace the description message. Also, long description messages don't take the available height and overflow at 2 lines. I don't think the component should hold huge chunks of text like that, but some handling could come handy just in case. image

Yes the description has a set height and the it will overflow in case the description is too big, for the link the idea is that it is a small text, overall the idea of the header banner is just small text.

jdslaugh commented 1 week ago

Kind of an edge case for the header alert, a long link text can displace the description message. Also, long description messages don't take the available height and overflow at 2 lines. I don't think the component should hold huge chunks of text like that, but some handling could come handy just in case. image

Yes the description has a set height and the it will overflow in case the description is too big, for the link the idea is that it is a small text, overall the idea of the header banner is just small text.

Good catch @jecr. @lucechal14 let's make sure it doesn't break free if it's too long. The idea is for it to not be very large but if it is I'd rather it be hidden or truncated versus breaking outside of the header. Can likely just overflow: scroll it and move the maxHeight to the alert itself.

lucechal14 commented 1 week ago

Kind of an edge case for the header alert, a long link text can displace the description message. Also, long description messages don't take the available height and overflow at 2 lines. I don't think the component should hold huge chunks of text like that, but some handling could come handy just in case. image

Yes the description has a set height and the it will overflow in case the description is too big, for the link the idea is that it is a small text, overall the idea of the header banner is just small text.

Good catch @jecr. @lucechal14 let's make sure it doesn't break free if it's too long. The idea is for it to not be very large but if it is I'd rather it be hidden or truncated versus breaking outside of the header. Can likely just overflow: scroll it and move the maxHeight to the alert itself.

@jdslaugh @jecr done, here is how it looks like Screen Recording 2024-09-05 at 12 39 47 p m