Skip to content

Conversation

@Lamparter
Copy link
Contributor

@Lamparter Lamparter commented Jan 10, 2026

Resolved / Related Issues

Steps used to test these changes

  1. Open Files
  2. Open release notes page
  3. Test 'close current tab' action shortcuts

This PR implements the "close current tab" action into the release notes page WebView by an injected event listener.

@Lamparter Lamparter marked this pull request as ready for review January 10, 2026 21:06
Copilot AI review requested due to automatic review settings January 10, 2026 21:06
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes the "close tab" action shortcuts (Ctrl+W and Ctrl+F4) on the Release Notes page by implementing JavaScript keyboard event listeners in the WebView that communicate with the Files.App action system.

Changes:

  • Added JavaScript keyboard event listener to capture close tab shortcuts within the WebView
  • Refactored link-click handling to use structured JSON messages instead of plain strings
  • Created a WebMessage DTO class to handle typed messages from JavaScript
  • Added JavaScriptModifiers mapping in HotKey class to translate C# modifiers to JavaScript event properties

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File Description
src/Files.App/Views/ReleaseNotesPage.xaml.cs Added keyboard shortcut handling via JavaScript injection, refactored message handlers to use typed WebMessage objects, and added key normalization helper
src/Files.App/Data/Messages/WebMessage.cs New DTO class for deserializing JSON messages from WebView JavaScript
src/Files.App/Data/Commands/HotKey/HotKey.cs Added JavaScriptModifiers dictionary to map KeyModifiers to JavaScript event property names

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Lamparter Lamparter changed the title Fix: Reroute the "close tab" action via WebView Fix: Reroute the "close tab" action via event listener in WebView Jan 10, 2026
@Lamparter
Copy link
Contributor Author

No idea why the tests are failing

@yaira2
Copy link
Member

yaira2 commented Jan 11, 2026

No idea why the tests are failing

The tests passed after I reran them 🎉

Does this also enable other keyboard shortcuts? (I haven't reviewed or tested these changes yet)

@yaira2 yaira2 added the ready for review Pull requests that are ready for review label Jan 11, 2026
@Lamparter
Copy link
Contributor Author

Does this also enable other keyboard shortcuts?

No, that would require additional work.

@Lamparter Lamparter changed the title Fix: Reroute the "close tab" action via event listener in WebView Fix: Rerouted the "close tab" action via event listener in WebView Jan 11, 2026
@yaira2
Copy link
Member

yaira2 commented Jan 11, 2026

The same issue preventing Ctrl+W from working on the Release Notes page is also preventing the other actions from working. I feel this needs a solution that works for all actions rather than implementing support for each one separately.

@Lamparter
Copy link
Contributor Author

I can do that, but it will require additional work.

@yaira2
Copy link
Member

yaira2 commented Jan 11, 2026

If we can figure out why the shortcuts aren't already detected, that would be ideal and would avoid a lot of extra work. It might even help us with the touchpad scrolling issue.

@Lamparter
Copy link
Contributor Author

I don't understand enough about webview2 in winappsdk to figure out why that happens unfortunately

}
});
window.addEventListener('keydown', function(event) {
Copy link
Member

Choose a reason for hiding this comment

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

Adding an event listener seems to be the simplest workaround. I wonder if we can then reroute the event to MainPage and let it handle the shortcut from there.

Copy link
Member

Choose a reason for hiding this comment

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

After further research, it looks like enabling AllowHostInputProcessing might be a better approach to take.

@yaira2 yaira2 added changes requested Changes are needed for this pull request and removed ready for review Pull requests that are ready for review labels Jan 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changes requested Changes are needed for this pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: App actions are not triggered from within the release notes page

2 participants