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

frontend: ActionsNotifier: Adjust keys for snackbars #2674

Merged
merged 1 commit into from
Dec 20, 2024

Conversation

skoeva
Copy link
Contributor

@skoeva skoeva commented Dec 12, 2024

This change adds a unique refKey for error states, separate from success states, and creates a unique key for every snackbar. This addresses a regression where failure notifications would not appear in the UI.

Fixes: #2655

Testing

  • Click on the "Create" button in Headlamp (either the one in the sidebar or for a resource like ConfigMap) and try to create a resource with an invalid name. Here's some sample YAML:
apiVersion: v1
kind: ConfigMap
metadata:
  name: configmap!
data: {}
  • Ensure that before the editor reopens with the error message, a failure notification appears in the bottom left corner

image

@skoeva skoeva added the frontend Issues related to the frontend label Dec 12, 2024
@skoeva skoeva requested a review from a team December 12, 2024 18:28
@skoeva skoeva self-assigned this Dec 12, 2024
@skoeva skoeva linked an issue Dec 12, 2024 that may be closed by this pull request
@dosubot dosubot bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Dec 12, 2024
@sniok
Copy link
Contributor

sniok commented Dec 13, 2024

I tried it and error notification leaves really quick and there are error message about duplicated key

Recording.2024-12-13.105714.mp4

I double checked that I'm on the right branch and up to date

➜  headlamp git:(fix-notifications) git pull
Already up to date.
➜  headlamp git:(fix-notifications) git status
On branch fix-notifications
Your branch is up to date with 'origin/fix-notifications'.

nothing to commit, working tree clean

@skoeva
Copy link
Contributor Author

skoeva commented Dec 13, 2024

I tried it and error notification leaves really quick and there are error message about duplicated key

@sniok looks like uniqueKey is not unique enough... 🗿
pushing a fix up now

@skoeva skoeva changed the title frontend: ActionsNotifier: Use unique refKey for errors frontend: ActionsNotifier: Adjust keys for snackbars Dec 13, 2024
This change adds a unique refKey for error states, separate from success
states, and creates a unique key for every snackbar. This addresses a
regression where failure notifications would not appear in the UI.

Fixes: #2655

Signed-off-by: Evangelos Skopelitis <[email protected]>

change
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Dec 13, 2024
Copy link
Collaborator

@joaquimrocha joaquimrocha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I tested Olek's case here and it doesn't happen anymore it seems.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Dec 20, 2024
@joaquimrocha joaquimrocha merged commit b3f862f into main Dec 20, 2024
18 checks passed
@joaquimrocha joaquimrocha deleted the fix-notifications branch December 20, 2024 18:35
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 lgtm This PR has been approved by a maintainer size:S This PR changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Notifications from editordialog failures not appearing
3 participants