Notifications at cost and perf levels #175
Conversation
bhanvimenghani
changed the title
|
@dinogun |
bhanvimenghani
force-pushed
the
notification
branch
from
5862129
to
e13d11f
Compare
|
Fixed conflicts |
|
|
||
| const [showSuccessAlert, setShowSuccessAlert] = useState(false); | ||
| const [alertMessage, setAlertMessage] = useState(''); | ||
| const [notificationType, setNotificationType] = useState<alertVarient>('info'); |
There was a problem hiding this comment.
this useState<alertVarient> is the same type alertVarient on line 98 hence the same spelling. (I guess the spelling i can correct to 'variant' at both places)
| setNotificationType('info'); | ||
| } | ||
| else if (notification.type == "error"){ | ||
| setNotificationType('danger'); |
There was a problem hiding this comment.
Any reason on setting a different value for notification type ?
If notification.type is "error" , why to set as "danger" instead of "error" itself ?
There was a problem hiding this comment.
patternfly has "info" and "Danger" as the notification variants while from the backend we get "notice" and "error", so over here im just trying to match the notification type from backend to the patternfly notifications. reference https://www.patternfly.org/components/alert
There was a problem hiding this comment.
As I don't see warning and critical related notifications, can you please add them as well.
|
As discussed, we should be able to view all the notifications related to one recommendation at a time. |
|
here is the updated view of the ui with before (in perf tab) and after (in cost tab) link |
|
@kusumachalasani can u re-review the pr please. |
@bhanvimenghani I'm unable to view the video. It says 'may have been deleted or archived by its creator.' |
|
https://www.loom.com/share/08fdf9c9ec094c56a1996c1a075411f4?sid=11d5c43e-429b-452c-911f-78aacb954a9f |
|
image for testing |
|
Couldn't test with the image because of 'Unable to Generate Recommendations' error in openshift. |
This change would be part of a different sprint, out of scope for this pr |
bhanvimenghani
force-pushed
the
notification
branch
from
ed2c4fa
to
4b34bf1
Compare
This pr adds alerts for over and under resource utilization in the UI. This pr has been made as per the json structure shared here. A demo video reflecting the changes is attached here