Skip to content

Close notification using the escape key#5835

Draft
GogoVega wants to merge 2 commits into
node-red:devfrom
GogoVega:escape-key-closes-notification
Draft

Close notification using the escape key#5835
GogoVega wants to merge 2 commits into
node-red:devfrom
GogoVega:escape-key-closes-notification

Conversation

@GogoVega

Copy link
Copy Markdown
Contributor

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Proposed changes

Request from: https://discourse.nodered.org/t/escape-key-closes-notification/101315

  • Close the first notification using the escape key.
  • Close all notifications using the ctrl-escape key.

Checklist

  • I have read the contribution guidelines
  • For non-bugfix PRs, I have discussed this change on the forum/slack team.
  • I have run npm run test to verify the unit tests pass
  • I have added suitable unit tests to cover the new/changed functionality

@GogoVega

Copy link
Copy Markdown
Contributor Author

While experimenting with the feature, a limitation became apparent: is it permissible for the user to close a notification that is awaiting user action? For example, if I am selecting nodes for the catch node and I close the notification, the workspace is in a broken state.

  • What type of notifications do we allow the shortcut to handle?
  • How do we trigger the workflow that the cancel button is supposed to initiate?

@GogoVega GogoVega marked this pull request as draft June 19, 2026 11:25
@knolleary

Copy link
Copy Markdown
Member

I think you are over complicating it. We don't need actions to close notifications. We have other places where Escape is handled outside the regular keyboard handling. That's all we need here; if a notification adds a cancel button, add a short-lived event handler to trigger the close.

@GogoVega

Copy link
Copy Markdown
Contributor Author

We have no way of knowing which button cancels the action.

Notifications that aren't fixed and optionally have a timeout already have click-based triggering. Not certain that adding the shortcut would be useful for them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants