fix(ui): Fullscreen keyboard lock logic by IDisposable · Pull Request #535 · jetkvm/kvm

IDisposable · 2025-05-28T00:34:43Z

When the keyboard lock is supposed to be active (in full-screen mode), hitting the escape key (NOT long-pressing) should NOT dismiss the full-screen mode, and should send the Escape key through to the remote.

Added awaits to the browser calls that need to complete in order.
Cleaned up (mostly) duplicate code in the Absolute/Relative mouse handling.
Ensure we don't overrun any existing keyboard lock or pointer lock.
Release the keyboard lock when leaving full-screen.
Per standards, we need to acquire the keyboard and pointer locks before entering full-screen or the user may get multiple messages about exiting.
Fixed all the missing/excess React dependencies.
Moved the PointerLockBar up so it is visible.
Change PointerLockBar animations to ensure it is visible when needed.
Fixed readonly warnings.

IDisposable · 2025-05-28T00:42:16Z

ui/src/components/VideoOverlay.tsx


 interface OverlayContentProps {
-  children: React.ReactNode;
+  readonly children: React.ReactNode;


There's a warning in VS Code that all these properties should be declared readonly (which they should), so while I was in here I updated them. We need to do a sweep through the UI to get the rest... another PR will arrive for ONLY warning-fixes.

IDisposable · 2025-05-28T00:44:40Z

ui/src/components/VideoOverlay.tsx

-          initial={{ y: 20, opacity: 0, zIndex: 0 }}
-          animate={{ opacity: 1, y: 0 }}
-          exit={{ y: 43, zIndex: 0 }}
+          className="flex w-full items-center justify-between bg-transparent"


Animating the location just plain broke the layout and resulted in this button/action just being behind and invisible to the user. I couldn't get the thing centered the way I wanted, but in this form it animates into and out of visibility correctly. Also, setting the motion.div tobg-transparent helps ensure it doesn't look ugly.

IDisposable · 2025-05-28T00:46:14Z

ui/src/components/VideoOverlay.tsx

-          exit={{ y: 43, zIndex: 0 }}
+          className="flex w-full items-center justify-between bg-transparent"
+          initial={{ opacity: 0, zIndex: 0 }}
+          animate={{ opacity: 1, zIndex: 20 }}


Even though the div was supposed to be z-20, not animating to that destination left the div as z-index: 0 and thus invisible. Just changing this was NOT enough because it was still positioned all wonky relative to the action bar.

IDisposable · 2025-05-28T00:46:32Z

ui/src/components/VideoOverlay.tsx

+          className="flex w-full items-center justify-between bg-transparent"
+          initial={{ opacity: 0, zIndex: 0 }}
+          animate={{ opacity: 1, zIndex: 20 }}
+          exit={{ opacity: 0, zIndex: 0 }}


If we fade in, we have to fade out :)

IDisposable · 2025-05-28T00:47:24Z

ui/src/components/WebRTCVideo.tsx

  // Pointer lock and keyboard lock related
-  const isPointerLockPossible = window.location.protocol === "https:";
-
+  const isPointerLockPossible = window.location.protocol === "https:" || window.location.hostname === "localhost";


Also allowing pointer lock on "localhost" so we can test during development (this will not be an issue in deployed code)

Good catch! I didn't realize that it could work on localhost. :-)

IDisposable · 2025-05-28T00:56:41Z

ui/src/components/WebRTCVideo.tsx

  );

  const videoKeyUpHandler = useCallback((e: KeyboardEvent) => {
+    if (!videoElm.current) return;


Cleaner to bail out so we don't need ?. below.

IDisposable · 2025-05-28T00:57:41Z

ui/src/components/WebRTCVideo.tsx


-      // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-      // @ts-expect-error
-      window.clearKeys = () => sendKeyboardEvent([], []);


We should not be polluting the global window namespace with this! I searched and found no references to clearKeys, so elided that line.

We can probably use useRef or other methods to expose the API.@adamshiervani Any better idea?

If you have a use-case, I can restore that line... but there are NO references to clearKeys anywhere else in the code-base

IDisposable · 2025-05-28T00:58:21Z

ui/src/components/WebRTCVideo.tsx

-      mouseWheelHandler,
-      videoKeyUpHandler,
-    ],
+    [onVideoPlaying, videoKeyUpHandler],


Here and elsewhere, I deleted all the dependencies and let VSCode put back the ones really needed.

IDisposable · 2025-05-28T00:59:03Z

ui/src/components/WebRTCVideo.tsx

  );

-  // Setup Absolute Mouse Events
+  // Setup Mouse Events


Merged the (nearly) duplicate handling for relative vs. absolute mouse handling.

IDisposable · 2025-05-28T00:59:48Z

ui/src/components/WebRTCVideo.tsx

+        );
+      } else {
+        // Reset the mouse position when the window is blurred or the document is hidden
+        window.addEventListener("blur", resetMousePosition, { signal });


I suspect we should be doing this in both cases, but wasn't sure how to confirm that, so I left it only on absolute mouse mode.

IDisposable · 2025-05-28T01:00:45Z

Ready for you @ym @adamshiervani

IDisposable · 2025-05-28T23:38:40Z

Before this change, on entering full screen you get the message To exit full screen, press Esc and if you hit an Escape key it will leave full screen mode.

After this change, on entering full screen you get the message To exit full screen, press and hold Esc and if you hit an Escape key it will NOT leave full screen mode.

When the keyboard lock is supposed to be active (in full-screen mode), hitting the escape key (NOT long-pressing) should NOT dismiss the full-screen mode, and should send the Escape key through to the remote. - Added awaits to the browser calls that need to complete in order. - Cleaned up (mostly) duplicate code in the Absolute/Relative mouse handling - Ensure we don't overrun any existing keyboard lock or pointer lock - Release the keyboard lock when leaving full-screen - Per standards, we need to acquire the keyboard and pointer locks before entering full-screen or the user may get multiple messages about exiting. - Fixed all the missing/excess React dependencies. - Moved the pointer lock bar up so it is visible. - Somewhere along the way, the prompt to click the video when in relative-mouse-mode stopped being visible, restored it's visibility - Fixed all the "should be readonly" warnings.

adrianmeraz · 2025-05-31T16:05:21Z

Looks great, looking forward to being able to using escape on my remote machines (which I do all the time by accident)

literallylara · 2025-06-19T20:14:26Z

ui/src/components/WebRTCVideo.tsx

+
  const checkNavigatorPermissions = useCallback(async (permissionName: string) => {
    const name = permissionName as PermissionName;
    const { state } = await navigator.permissions.query({ name });


This will throw an error in browsers that do understand a permission, e.g. Firefox, causing the fullscreen button to not do anything. See: https://developer.mozilla.org/en-US/docs/Web/API/Permissions/query

Thrown if retrieving the PermissionDescriptor information failed in some way, or the permission doesn't exist or is unsupported by the user agent.

I don't use anything but Chrome and Edge, so thanks for finding this. I can wrap that with a check.

Addressed in #631

I don't use anything but Chrome and Edge, so thanks for finding this. I can wrap that with a check.

Unfortunately it is still necessary to do cross-browser and cross-device testing in the world of web development today. I can recommend caniuse.com.

PR #631 submitted

Per jetkvm#535 (comment) we need to handle Firefox/Mozilla browsers not having these pointer-lock or keyboard-lock permissions checks. Also might as well not throw if something goes wrong when trying to check permissions or apply for locks.

adrianmeraz · 2025-06-25T17:12:29Z

@IDisposable Did this really get merged into the latest update? I don't see the changes, doesn't required a long press ESC

IDisposable · 2025-06-25T20:05:15Z

It can only do a keyboard lock if you're hitting it in HTTPS, so enable that or use app.jetkvm.com to access the JetKVM

amatriain · 2025-06-26T08:22:39Z

Still not working for me with Firefox on arch linux. App version 0.4.5, System version 0.2.5. Accessing jetKVM through the LAN, not cloud, with HTTPS mode enabled, using the self-signed cert option.

literallylara · 2025-06-26T11:52:23Z

@amatriain Keyboard locking is in experimental state and not currently suppported by Firefox, see: https://caniuse.com/mdn-api_keyboard_lock

When the keyboard lock is supposed to be active (in full-screen mode), hitting the escape key (NOT long-pressing) should NOT dismiss the full-screen mode, and should send the Escape key through to the remote. - Added awaits to the browser calls that need to complete in order. - Cleaned up (mostly) duplicate code in the Absolute/Relative mouse handling - Ensure we don't overrun any existing keyboard lock or pointer lock - Release the keyboard lock when leaving full-screen - Per standards, we need to acquire the keyboard and pointer locks before entering full-screen or the user may get multiple messages about exiting. - Fixed all the missing/excess React dependencies. - Moved the pointer lock bar up so it is visible. - Somewhere along the way, the prompt to click the video when in relative-mouse-mode stopped being visible, restored it's visibility - Fixed all the "should be readonly" warnings.

IDisposable force-pushed the keyboard-lock branch from 6688369 to a9895ce Compare May 28, 2025 00:39

IDisposable commented May 28, 2025

View reviewed changes

IDisposable mentioned this pull request May 28, 2025

Can't write/paste certain special characters using key combinations #30

Closed

IDisposable marked this pull request as draft May 28, 2025 01:38

IDisposable marked this pull request as ready for review May 28, 2025 01:38

IDisposable force-pushed the keyboard-lock branch from a9895ce to 5d79df6 Compare May 30, 2025 16:15

ym merged commit f4bb47c into jetkvm:dev Jun 2, 2025
2 checks passed

IDisposable deleted the keyboard-lock branch June 3, 2025 01:20

literallylara reviewed Jun 19, 2025

View reviewed changes

IDisposable mentioned this pull request Jun 19, 2025

fix(ui)/firefox permissions error handling #631

Merged

Conversation

IDisposable commented May 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IDisposable commented May 28, 2025

Uh oh!

IDisposable commented May 28, 2025

Uh oh!

adrianmeraz commented May 31, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adrianmeraz commented Jun 25, 2025

Uh oh!

IDisposable commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

amatriain commented Jun 26, 2025

Uh oh!

literallylara commented Jun 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

IDisposable commented Jun 25, 2025 •

edited

Loading