Skip to content

feat: add running page keyboard shortcuts - #1298

Open
m-altaifi wants to merge 2 commits into
GyulyVGC:mainfrom
m-altaifi:feat/running-page-shortcuts
Open

m-altaifi wants to merge 2 commits into
GyulyVGC:mainfrom
m-altaifi:feat/running-page-shortcuts

Conversation

@m-altaifi

Copy link
Copy Markdown

Summary

  • adds Ctrl+1, Ctrl+2, and Ctrl+3 shortcuts for the Overview, Inspect, and Notifications pages
  • keeps the shortcuts scoped to the running view so settings and modals are not changed behind overlays
  • adds unit coverage for direct page switching and unread notification reset behavior

Testing

  • cargo fmt --check
  • cargo check
  • cargo test test_correctly_switch_running_page_with_shortcut (blocked locally: link.exe cannot find wpcap.lib)

Refs #97

@GyulyVGC GyulyVGC added the enhancement New feature, request, or improvement label Aug 31, 2026
Comment thread src/gui/types/message.rs
Comment thread src/gui/sniffer.rs Outdated
Address review feedback: drop the RunningPageShortcut message and emit
the existing ChangeRunningPage from the Ctrl+1/2/3 shortcuts. The
side-effect guard moves into change_running_page so the tab-bar path is
covered too.

Two existing tests drove ChangeRunningPage from states the guard now
rejects (no running page / settings open); they set the state explicitly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@m-altaifi

Copy link
Copy Markdown
Author

The requested refactor is now in the latest commit: the shortcuts reuse \ChangeRunningPage, and the shared handler owns the side-effect guard. The related tests now set the state required by that guard. Could you please re-review when convenient?

@GyulyVGC

GyulyVGC commented Sep 3, 2026

Copy link
Copy Markdown
Owner

Hey, thanks for the reminder

I already reviewed actually and everything looks good, but it'll take some time to merge this since I'm working on other items right now.

Hopefully I'll ship this with the next version of Sniffnet

@GyulyVGC

Copy link
Copy Markdown
Owner

Hey @m-altaifi I think this should be extended to work also with setting pages as I'm doing for the tab-based keyboard shortcut

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

Labels

enhancement New feature, request, or improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants