Fixes: Keybindings for the videoplayer are not successfully overriden #828
Conversation
|
Hello! Apologies for taking so long to look at this. Could you please update this branch to reflect upstream? There's a lot of out od date packages now and it's preventing me from looking into this further. Thank you! |
|
Hello @PrestonN! No worries, it was the holiday season when I first did the PR. I have rebased my branch and pushed the changes. The same issues still persist as detailed above. I did not implement the new keybindings that were added this past month ('D' ,'esc') or the |
| // // Fast Forward by 5 seconds | ||
| // event.preventDefault() | ||
| // this.changeDurationBySeconds(5) | ||
| // break | ||
| // case 49: | ||
| // // 1 Key | ||
| // // Jump to 10% in the video | ||
| // event.preventDefault() | ||
| // this.changeDurationByPercentage(0.1) | ||
| // break | ||
| // case 50: | ||
| // // 2 Key | ||
| // // Jump to 20% in the video | ||
| // event.preventDefault() | ||
| // this.changeDurationByPercentage(0.2) | ||
| // break | ||
| // case 51: | ||
| // // 3 Key | ||
| // // Jump to 30% in the video | ||
| // event.preventDefault() | ||
| // this.changeDurationByPercentage(0.3) | ||
| // break | ||
| // case 52: | ||
| // // 4 Key | ||
| // // Jump to 40% in the video | ||
| // event.preventDefault() | ||
| // this.changeDurationByPercentage(0.4) | ||
| // break | ||
| // case 53: | ||
| // // 5 Key | ||
| // // Jump to 50% in the video | ||
| // event.preventDefault() | ||
| // this.changeDurationByPercentage(0.5) | ||
| // break | ||
| // case 54: | ||
| // // 6 Key | ||
| // // Jump to 60% in the video | ||
| // event.preventDefault() | ||
| // this.changeDurationByPercentage(0.6) | ||
| // break | ||
| // case 55: | ||
| // // 7 Key | ||
| // // Jump to 70% in the video | ||
| // event.preventDefault() | ||
| // this.changeDurationByPercentage(0.7) | ||
| // break | ||
| // case 56: | ||
| // // 8 Key | ||
| // // Jump to 80% in the video | ||
| // event.preventDefault() | ||
| // this.changeDurationByPercentage(0.8) | ||
| // break | ||
| // case 57: | ||
| // // 9 Key | ||
| // // Jump to 90% in the video | ||
| // event.preventDefault() | ||
| // this.changeDurationByPercentage(0.9) | ||
| // break | ||
| // case 48: | ||
| // // 0 Key | ||
| // // Jump to 0% in the video (The beginning) | ||
| // event.preventDefault() | ||
| // this.changeDurationByPercentage(0) | ||
| // break | ||
| // case 188: | ||
| // // , Key | ||
| // // Return to previous frame | ||
| // this.framebyframe(-1) | ||
| // break | ||
| // case 190: | ||
| // // . Key | ||
| // // Advance to next frame | ||
| // this.framebyframe(1) | ||
| // break | ||
| // case 68: | ||
| // // D Key | ||
| // // Toggle Picture in Picture Mode | ||
| // if (!this.player.isInPictureInPicture()) { | ||
| // this.player.requestPictureInPicture() | ||
| // } else if (this.player.isInPictureInPicture()) { | ||
| // this.player.exitPictureInPicture() | ||
| // } | ||
| // break | ||
| // case 27: | ||
| // // esc Key | ||
| // // Exit full window | ||
| // event.preventDefault() | ||
| // this.exitFullWindow() | ||
| // break | ||
| // case 83: | ||
| // // S Key | ||
| // // Toggle Full Window Mode | ||
| // this.toggleFullWindow() | ||
| // break | ||
| // } | ||
| // } | ||
| // ...mapActions([ | ||
| // 'calculateColorLuminance' | ||
| // ]) | ||
| // } |
There was a problem hiding this comment.
I would prefer these lines removed if we don't need them, we can always revert using git if we need to restore them.
There was a problem hiding this comment.
No comments on the proposed solution since I am not familiar with the codebase... Here are my 2cents for fixing this issue :)
Funnily enough, if you hit the f key the full-screen comes on and keyboard shortcuts work. Its only when you use your mouse to go full-screen that the shortcuts stop working.
Its as if the player (or whatever has the shortcuts bind to it) does not contain the full screen button, and thus it goes out of focus. I wonder if bringing in hierarchically the full-screen button inside the player would fix the problem once and for all?
|
Hi @TDDR would u please update us on this otherwise we may consider closing this request. |
|
Hey there! It's been a while since there's been any activity here. Since we haven't had any updates in a while, I'll go ahead and close this PR so that we can clean up our PR list. If by chance you decide to come back and continue this PR, please let us know and we can re-open this. Thanks for understanding. |
Fixes: Keybindings for the videoplayer are not successfully overriden
Pull Request Type
Related issue
Fixes: #637
Description
This PR is intended to fix the auto focus issue when changing to full screen so that hotkeys will still work without having to manually give focus back to the player. I have it mostly working but there are still a few bugs that I was hopping to get a hand with.
Testing (for code that is not small enough to be easily understandable)
Issues -
alwaysCaptureHotkeysproperty kind of works, If you click on fullscreen, then let theuserInactiveevent happen, focus will return to the player, allowing the use of hotkeys without having to manually click the player again. I know this is not the optimal behavior, but I'm not sure if the behavior I'm getting is because of the warning that came with using it, namelyNote: This feature may break accessibility, and cause unexpected behavior. Or if the behavior is because I am not capturing the correct elements on this lineDesktop (please complete the following information):
Additional context
I have replicated most of the old hotkey functionality. The only difference, aside from the bugs, is that I bundled
l and right arrrowandj and left arrow. the old functionality was thatj and lwould change the playtime by10secand the arrows would only change it by5sec. They both change it by 5 seconds now but I can change it to the old way if requested.