[PLAYER-19] keybinding (post refacto dialog) #82

jparez · 2024-06-14T12:12:51Z

Description

Adding keymapping feature. Somes refacto over the store, handle events, ... was recommended or necessery.
I have left some comments to helping to review.

Gesture handle

d-pad
swipe
tap
mouse

Config

You can find some json config example at the top of keymapping plugin file. Cooordonate are always in percent

Api added

setConfig to change the config, supply a valid json config.
activeKeyMappingDebug with 2 boolean in parameter
- if first parameter is true, click on <video> will display a square with x / y coordonate
- the second display a grid over <video> slice by 10%

v1

the d-pad isn't well handle on every games (fps could be nonfunctionnal)
mouse event not handle

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

I've read & comply with the contributing guidelines
I have tested my code for new features & regressions on both mobile & desktop devices, using the latest version of major browsers.
I have made corresponding changes to the documentation (README.md).
I've checked my modifications for any breaking changes.

…imple touch

jparez · 2024-06-26T08:51:40Z

src/APIManager.js

Added a category for API manager exposed fn => window.player.media.mute()

jparez · 2024-06-26T08:53:20Z

src/DeviceRenderer.js

@@ -88,17 +87,6 @@ module.exports = class DeviceRenderer {
        // last accessed x/y position
        this.x = 0;
        this.y = 0;
-


Click handler handle has been move to overlayplugin class (decoupling)

jparez · 2024-06-26T08:53:49Z

src/DeviceRenderer.js

@@ -486,10 +475,6 @@ module.exports = class DeviceRenderer {
                this.mouseEvents.addMouseCallbacks();
            }



decoupling see above

jparez · 2024-06-26T08:54:57Z

src/plugins/FingerPrint.js

@@ -127,7 +127,7 @@ module.exports = class FingerPrint extends OverlayPlugin {

        // register callback
        this.instance.registerEventCallback('fingerprint', (message) => this.handleFingerprintEvent(message));
-        if (this.instance.store.getState().isWebRTCConnectionReady) {


getState was unecessary for a little store,

jparez · 2024-06-26T08:55:26Z

src/plugins/Fullscreen.js

@@ -20,6 +20,18 @@ module.exports = class Fullscreen {
            this.instance.addListener(document, 'fullscreenchange', this.onFullscreenEvent.bind(this), false);
        }

+        this.instance.apiManager.registerFunction({


Requested from socis

jparez · 2024-06-26T08:55:59Z

src/plugins/KeyboardEvents.js

        this.isListenerAdded = false;
        this.currentlyPressedKeys = new Map();

        this.instance.store.subscribe(({isKeyboardEventsEnabled}) => {
-            this.transmitKeys = isKeyboardEventsEnabled;
+            if (isKeyboardEventsEnabled) {


little refact to decoupling keyboard handle events of deviceRenderer js

jparez · 2024-06-26T08:56:19Z

src/plugins/MediaManager.js

-            this.instance.video.isMuted = true;
-            this.instance.video.muted = true;
+        this.instance.apiManager.registerFunction({
+            name: 'mute',


added category

jparez · 2024-06-26T08:56:48Z

src/plugins/MouseEvents.js

@@ -20,6 +20,7 @@ module.exports = class MouseEvents {

        this.leftButtonPressed = false;
        this.boundEventListener = this.releaseAtPreviousPositionEvent.bind(this);
+        this.removeMouseUpListener = () => {};


removeMouseUplistener will never be undefined

jparez · 2024-06-26T08:57:57Z

src/plugins/util/OverlayPlugin.js

@@ -26,6 +26,13 @@ module.exports = class OverlayPlugin {
                this.closeOverlay();
            }
        });
+
+        // Attach listener for first object created only
+        if (!OverlayPlugin.hasBeenCalled) {


it's a kind of static method for a Class, this way we attach the listener only once

…harm

…tener

pgivel

This is looking great. Most of my comments are about documentation, since this feature is a bit complicated to understand, I think it benefits from being as documented as possible.

I didn't much review the debug stuff (grid and trace), since I trust you that it works well enough.

src/APIManager.js

src/scss/components/_toolbar.scss

src/store/index.js

src/plugins/KeyboardMapping.js

…ping_paused

[enhanced] focus on clipboardinput when open dialog

e67547a

jparez marked this pull request as draft June 14, 2024 12:12

jparez force-pushed the dev/PLAYER-19-keybinding-post-refacto-dialog branch from 7c18672 to 6a5b918 Compare June 14, 2024 12:41

jparez marked this pull request as ready for review June 25, 2024 16:17

jparez force-pushed the dev/PLAYER-19-keybinding-post-refacto-dialog branch from 61e4dd6 to 1cb706b Compare June 25, 2024 16:27

jparez added 5 commits June 26, 2024 10:09

somes refacto (keyboard start / stop listening), ... and keymapping s…

adf4417

…imple touch

keymapping with multitouch (dpad, swipe, tap)

27efda7

refacto apimanager, adding category + adding fullscren api

bb2733d

set keyboardmapping default at false

0f2f58b

adding toolbar icon and fix reacticate issue

ca11d54

jparez force-pushed the dev/PLAYER-19-keybinding-post-refacto-dialog branch from 7c82442 to ca11d54 Compare June 26, 2024 08:13

jparez commented Jun 26, 2024

View reviewed changes

src/APIManager.js

Copy link

Contributor Author

jparez Jun 26, 2024

Choose a reason for hiding this comment

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

Added a category for API manager exposed fn => window.player.media.mute()

jparez commented Jun 26, 2024

View reviewed changes

jparez requested review from pgivel and JulienBolard June 26, 2024 09:10

refacto keymapping motor. Simple mouse integration but works like a c…

97c8341

…harm

jparez force-pushed the dev/PLAYER-19-keybinding-post-refacto-dialog branch from 4f8f8a9 to 97c8341 Compare June 28, 2024 14:50

jparez added 6 commits June 28, 2024 16:52

update eslint rule

6225eb8

cleanup ebug and test trace

945ec43

refacto enable/disable mouse event + keymaaping has his own mouse lis…

878896d

…tener

light refacto

79a0831

update readme with keymapping and api doc

9636952

keymapping documentation

3e80de7

jparez force-pushed the dev/PLAYER-19-keybinding-post-refacto-dialog branch from cfb37ab to 3e80de7 Compare July 4, 2024 12:00

pgivel requested changes Jul 10, 2024

View reviewed changes

handle pr comments

f9f7af6

jparez force-pushed the dev/PLAYER-19-keybinding-post-refacto-dialog branch from 1a73030 to f9f7af6 Compare July 10, 2024 14:43

pgivel approved these changes Jul 11, 2024

View reviewed changes

chang icon name for consistency : ic_keymapping_disabled => ic_keymap…

9cc3cb4

…ping_paused

jparez force-pushed the dev/PLAYER-19-keybinding-post-refacto-dialog branch from a0e1503 to 9cc3cb4 Compare July 17, 2024 07:20

fix issue with clipboard dialog (events not properly send)

6965a6c

pgivel approved these changes Jul 18, 2024

View reviewed changes

jparez added 2 commits July 22, 2024 17:52

fix wording issue

1d08aa5

fix wording on apiManager's descriptions

a5d6da7

jparez merged commit 28e8b50 into main Jul 25, 2024
1 check passed

jparez deleted the dev/PLAYER-19-keybinding-post-refacto-dialog branch July 25, 2024 12:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PLAYER-19] keybinding (post refacto dialog) #82

[PLAYER-19] keybinding (post refacto dialog) #82

jparez commented Jun 14, 2024 •

edited

Loading

jparez Jun 26, 2024

jparez Jun 26, 2024

jparez Jun 26, 2024

jparez Jun 26, 2024

jparez Jun 26, 2024

jparez Jun 26, 2024

jparez Jun 26, 2024

jparez Jun 26, 2024

jparez Jun 26, 2024

pgivel left a comment

		@@ -486,10 +475,6 @@ module.exports = class DeviceRenderer {
		this.mouseEvents.addMouseCallbacks();
		}

[PLAYER-19] keybinding (post refacto dialog) #82

[PLAYER-19] keybinding (post refacto dialog) #82

Conversation

jparez commented Jun 14, 2024 • edited Loading

Description

Gesture handle

Config

Api added

v1

Type of change

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pgivel left a comment

Choose a reason for hiding this comment

jparez commented Jun 14, 2024 •

edited

Loading