Skip to content

fix: KeyboardHelper overrides kb event listener#1583

Merged
madhavilosetty-intel merged 3 commits intomainfrom
sb_fix_kb_helper
Feb 9, 2026
Merged

fix: KeyboardHelper overrides kb event listener#1583
madhavilosetty-intel merged 3 commits intomainfrom
sb_fix_kb_helper

Conversation

@shaoboon
Copy link
Contributor

@shaoboon shaoboon commented Feb 6, 2026

PR Checklist

  • Unit Tests have been added for new changes
  • API tests have been updated if applicable
  • All commented code has been removed
  • If you've added a dependency, you've ensured license is compatible with Apache 2.0 and clearly outlined the added dependency.

What are you changing?

In KeyboardHelper, use event listener for keypress instead of overwriting the handler from the front end.

Anything the reviewer should know when reviewing this PR?

This PR is to resolve #796 issue.

How-To-Test

This issue is not reproducible on sample-web-ui frontend, so to reproduce the issue, a temporary keypress listener is implemented into sample-web-ui (the temp listener code is just for testing purpose, will not check in)

NOTE: the 'P' keypress is listened by frontend and printed on screen/webpage, 'P' key is being pressed during an active KVM session

  • Before implementing the fix:
image
  • After implementing the fix ('P' key is detected by both KVM and frontend):
image

If the there are associated PRs in other repositories, please link them here (i.e. device-management-toolkit/repo#365 )

@shaoboon shaoboon requested a review from Copilot February 6, 2026 11:14
Copy link

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

Updates KeyBoardHelper to stop overwriting global document.on* handlers by registering/removing keyboard handlers via addEventListener/removeEventListener, with corresponding unit test updates.

Changes:

  • Bind key handler methods once in KeyBoardHelper constructor for stable listener references.
  • Replace document.onkeyup/onkeydown/onkeypress assignments with addEventListener and removeEventListener.
  • Update unit tests to assert listener registration/removal behavior via Jest spies.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 9 comments.

File Description
src/core/Utilities/KeyboardHelper.ts Switch keyboard hooking to event listeners and bind callbacks for correct removal.
src/test/keyboardhelper.spec.ts Update tests to validate addEventListener/removeEventListener usage instead of document.on* mutation.

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

@shaoboon shaoboon force-pushed the sb_fix_kb_helper branch 2 times, most recently from b592e99 to 298eeca Compare February 6, 2026 12:23
@madhavilosetty-intel madhavilosetty-intel enabled auto-merge (squash) February 9, 2026 18:35
@madhavilosetty-intel madhavilosetty-intel merged commit 15a9157 into main Feb 9, 2026
4 checks passed
@madhavilosetty-intel madhavilosetty-intel deleted the sb_fix_kb_helper branch February 9, 2026 18:37
RosieAMT pushed a commit that referenced this pull request Feb 9, 2026
## [3.3.10](v3.3.9...v3.3.10) (2026-02-09)

### Bug Fixes

* KeyboardHelper overrides kb event listener ([#1583](#1583)) ([15a9157](15a9157))
@RosieAMT
Copy link

RosieAMT commented Feb 9, 2026

🎉 This PR is included in version 3.3.10 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement fix: KeyboardHelper may damage site functionality

3 participants