fix(notifications): add esc key to return to notification prompt#779
fix(notifications): add esc key to return to notification prompt#779dlvhdr merged 3 commits intodlvhdr:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds an escape key binding to allow users to exit from PR/Issue detail view within notifications and return to the notification prompt. This fixes a UX issue where users were stuck in detail mode after viewing a notification, with keybindings continuing to apply to the detailed PR/Issue view rather than the notification list.
Changes:
- Added
BackToNotificationkeybinding (esc key) to return from PR/Issue detail view to notification prompt - Implemented state cleanup when exiting detail mode, resetting both subject data and help keybindings
- Added comprehensive test coverage for the new functionality
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| internal/tui/keys/notificationKeys.go | Added BackToNotification keybinding definition and help text |
| internal/tui/ui.go | Implemented backToNotification() function and integrated keybinding handler; added state resets in syncSidebar() and onViewedRowChanged() |
| internal/tui/ui_test.go | Added two comprehensive tests validating escape key behavior for both PR and Issue notification subjects |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Yeah that was my bug, but I believe we subsequently fixed that in 6f20475, right?
Big +1 — that’s a great refinement. Seems kind of essential, actually. It rightly should have been part of the initial implementation.
That’s a nice improvement too. As a user of the Notifications dashboard, those are both changes I’d like to have. |
Yes, sorry, I can confirm that commit was actually fixing that issue which I mentioned there, I was originally testing on the released version. Thank you for the feedback (and for the feature 😉) |
|
o]]hkjkjjk f; |
|
this is great! can we also add that key under https://github.com/dlvhdr/gh-dash/blob/main/internal/tui/ui.go?plain=1#L1174, saying something like |
|
Done ✅ Added an |
|
Thank you! |
Hi @dlvhdr,
First of all, thanks for adding the notifications feature to the project. Super nice addition 🙌 I was working on it as well on my fork for a bit, but I never got it in a "nice" state enough for firing off for a pull request.
When I tried it out today, I got into a bit of a confusing state though, when I "viewed" one of the notifications and later moved on to another one, I wasn't able to interact with them anymore, because the keybindings were messed up after that subview.
I added another keybinding for being able to move back to the none view mode now and added the following changes:
Let me know what you think.