Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Something went wrong" alert needs some work #2314

Closed
sniok opened this issue Sep 6, 2024 · 3 comments · Fixed by #2336
Closed

"Something went wrong" alert needs some work #2314

sniok opened this issue Sep 6, 2024 · 3 comments · Fixed by #2336
Assignees
Labels
frontend Issues related to the frontend

Comments

@sniok
Copy link
Contributor

sniok commented Sep 6, 2024

Tested on 0.25.1, disabled the network and this is how it looks like

On wide screens it's way too short
image

Medium screens, covers the logo
image

On narrow screens, padding pushes the content all the way to the right
image
image

This seems like a good component to use

https://mui.com/material-ui/react-alert/

@sniok sniok added the frontend Issues related to the frontend label Sep 6, 2024
@sniok
Copy link
Contributor Author

sniok commented Sep 6, 2024

Also I think the message should be "Lost connection to the cluster" because it checks if the browser is online or if healthz endpoint of a cluster responds.

@skoeva
Copy link
Contributor

skoeva commented Sep 6, 2024

+1 this banner has been really buggy and the message isn't at all visibly traceable to the issue

@skoeva
Copy link
Contributor

skoeva commented Sep 16, 2024

Got around to checking out the alert component, looks promising. We can keep the width at 100% as before, but I think it would be cleaner and more convenient if the alert was a bit smaller (maybe something closer to this):

image

@sniok WDYT?

@skoeva skoeva self-assigned this Sep 17, 2024
skoeva added a commit that referenced this issue Sep 17, 2024
This change addresses issues with accessibility and clarity for the
AlertNotification component. This now uses the React Alert component
from Material UI and displays a more descriptive error message: "Lost
connection to the cluster".

Fixes: #2314

Signed-off-by: Evangelos Skopelitis <eskopelitis@microsoft.com>
skoeva added a commit that referenced this issue Sep 17, 2024
This change addresses issues with accessibility and clarity for the
AlertNotification component. This now uses the React Alert component
from Material UI and displays a more descriptive error message: "Lost
connection to the cluster".

Fixes: #2314

Signed-off-by: Evangelos Skopelitis <eskopelitis@microsoft.com>
@skoeva skoeva linked a pull request Sep 17, 2024 that will close this issue
skoeva added a commit that referenced this issue Sep 17, 2024
This change addresses issues with accessibility and clarity for the
AlertNotification component. This now uses the React Alert component
from Material UI and displays a more descriptive error message: "Lost
connection to the cluster".

Fixes: #2314

Signed-off-by: Evangelos Skopelitis <eskopelitis@microsoft.com>
skoeva added a commit that referenced this issue Sep 17, 2024
This change addresses issues with accessibility and clarity for the
AlertNotification component. This now uses the React Alert component
from Material UI and displays a more descriptive error message: "Lost
connection to the cluster".

Fixes: #2314

Signed-off-by: Evangelos Skopelitis <eskopelitis@microsoft.com>
@illume illume closed this as completed in cdc023e Sep 30, 2024
skoeva added a commit that referenced this issue Oct 1, 2024
This change addresses issues with accessibility and clarity for the
AlertNotification component. This now uses the React Alert component
from Material UI and displays a more descriptive error message: "Lost
connection to the cluster".

Fixes: #2314

Signed-off-by: Evangelos Skopelitis <eskopelitis@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend Issues related to the frontend
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants