FreeTube icon indicating copy to clipboard operation
FreeTube copied to clipboard

[Feature Request]: Disable spacebar activation for buttons except through keyboard navigation, & vice versa

Open kommunarr opened this issue 10 months ago • 18 comments

Guidelines

  • [X] I have searched the issue tracker for open and closed issues that are similar to the feature request I want to file, without success.
  • [X] I have searched the documentation for information that matches the description of the feature request I want to file, without success.
  • [X] This issue contains only one feature request.

Problem Description

As someone who often un-favorites a video and then clicks the spacebar to continue the video, I experience an undesirable and unexpected outcome: 1a) the video unpauses (good), but 1b) the video is re-favorited (bad). This occurs because the focus is on the favorite button, which is toggled by hitting either the Enter key or the spacebar. The same happens for the video player controls; try muting the video by clicking on the Mute button, then hitting the spacebar. Not only is 2a) the video unmuted (bad), but 2b) the video doesn't even unpause (also bad).

To compare with a website like YouTube: with clicking the like button and then pressing the spacebar, 1a) the video unpauses (good), and 1b) the video is not un-liked (good). With clicking the Mute button and then pressing the spacebar, 2a) the video is not unmuted (good), and 2b) the video unpauses (good).

Vice versa, if you're navigating through the controls using keyboard navigation, hitting the Spacebar on FreeTube both 3a) pauses/unpauses the video (bad) and 3b) toggles the focused button (good). You only want the focused control to be being affected. For the video player, though, our current behavior is good in this respect: navigating to the Mute button through keyboard controls and presses the spacebar 4a) mutes the video without pausing/unpausing the video, and 4b) successive presses of the spacebar continue to only toggle the Mute button without pausing/unpausing the video. As expected, YT is perfect here as well.

One last important behavior is 5) that focus is still applied to a control that is clicked (verified by clicking a button, then pressing the Enter key, which should still toggle the button), which FreeTube does (good), as well as YouTube. I don't know the selected method that YouTube has implemented to achieve this one while maintaining the rest.

In summary, here's where we're at: 1a) good 1b) bad 2a) bad 2b) bad 3a) bad 3b) good 4a) good 4b) good 5) good

Proposed Solution

Behavior-wise, 1b, 2a, 2b, and 3a need to be fixed while still preserving 1a, 3b, 4a, 4b, and 5. Technical-wise, there are a few different ways you could do this, although I'll admit there are probably better ways that I don't know about. Here is what I was able to find online:

  • https://stackoverflow.com/questions/70831240/when-pressing-on-spacebar-the-previous-clicked-button-gets-clicked
  • https://stackoverflow.com/questions/38603936/limit-spacebar-trigger-to-button

One promising route is to identify if it's a A) "keyboard navigation focus" or B) "it's focused because it was clicked" kind of focus and have the spacebar trigger only the button for Case A & only the videoplayer for case B. The only problem is that I have no idea how to determine that programmatically. Edit: There are some answers in this article. You can track the click event in JS on the /watch route and disable the selected button's spacebar event listener until focus changes (handling 1a through 2b), while setting event.stopPropagation() on the main controls (i.e., ft-icon-button and the video player buttons) to handle behaviors 3a through 5. Alternatively, you can listen for focus-visible events somehow and selectively temporarily disable the spacebar event listeners that way.

Alternatives Considered

Behavior-wise, I don't think there are any good alternatives. 1a through 5 as YT does it are all the best behaviors for optimal usability, intuitiveness, and accessibility. Technical-wise, there are probably other ways to do this that I'm missing, which all are fine as long as they work (and are not absurdly convoluted, of course).

Issue Labels

ease of use improvement, improvement to existing feature

Additional Information

I use the Mute button and Favorite button as examples, but these behavioral problems affect many other controls as well. A really common & pesky one is that clicking the Fullscreen button and then pressing Spacebar toggles Fullscreen rather than pausing/unpausing the video. Fullscreen is a somewhat unique case in YouTube, though, in that focus is applied to the full video player & removed from the button when going from non-fullscreen to fullscreen.

Overall, this issue is one that's hard to place your finger on and isn't too hard to fix, but it has massive effects on the UX of FreeTube, à la the Pareto principle.

kommunarr avatar Oct 12 '23 03:10 kommunarr

Duplicate of #637

absidue avatar Oct 12 '23 08:10 absidue

Good catch on the dupe. That said, I don't think anyone there laid out the causes, specific harmful behaviors, and what behaviors we should be preserving with the fix. Up to us if we want to move this all to the other issue or keep both open separately.

kommunarr avatar Oct 12 '23 12:10 kommunarr

If this one is more detailed, the other one can be closed if favour of this.

absidue avatar Oct 12 '23 16:10 absidue

Does https://github.com/FreeTubeApp/FreeTube/issues/766 also belong here? If so if this issue covers both issues than i would be in favor of keeping this open and closing the others

Yes, that's 2a and 2b.

kommunarr avatar Oct 13 '23 03:10 kommunarr

@efb4f5ff-1298-471a-8973-3d47447115dc

observation:

when you click the play/pause icon in the bottom left of the video viewport, it grabs focus and now keyboard shortcuts such as left/right arrows no longer work.

space still works by coincidence: i think space just clicks the focused play/pause button, so it emulates the normal space shortcut.

the initial play icon at center of screen also suffers from this i think. i suppose many other gadgets at bottom of the video viewport too. maybe focus grabbing and thus keyboard navigation for these widgets should just be disabled: being auto-hide, they are clearly meant to be used with the mouse.

Lanchon avatar Oct 19 '23 19:10 Lanchon

i've been using freetube since they blocked ad blockers, the main problem for me is now this left/right arrow not working, please fix asap!

tigros avatar Oct 20 '23 03:10 tigros

i've been using freetube since they blocked ad blockers, the main problem for me is now this left/right arrow not working, please fix asap!

they work! just click on the video once before using the keys

Lanchon avatar Oct 20 '23 04:10 Lanchon

i know that, if i click the navigation bar to position then the arrows don't work, i have to click the the video pause/play to get them to work again, it's bit of a pain! Should be the arrows work always no exceptions! thank you.

tigros avatar Oct 20 '23 04:10 tigros

This issue was bothering me too, so I've been working on it for the past few days, and I think I've come up with a solution that checks all the boxes.

The only option I found to have a complete solution was indeed to have a way to detect whether an element received focus from a keyboard or a pointer. To do this, I used some code from Keyboard Focus.

With this in place, we can just:

  • Emit a click event when a button receives a space key event only if it was focused using a keyboard.
  • Toggle play/pause when the keyboard shortcut handler receives a space key event only if the target element was focused using a pointer.

Points 2a) and 2b) required additional measures because of this video.js issue. The issue is that video.js stops the propagation of all keydown events when one of its components is focused (including the buttons of the control bar). So our keyboard shortcut handler, registered at the document level, never receives those events in that case. The best workaround I found was to register our keyboard shortcut handler directly on each each control bar button of the player.

I'll make a pull request for this changes. In the meantime, if you want to test if everything is working as expected, you can checkout my branch here.

Here is a recap of the expected behavior :

  1. When you click on a button outside the player (e.g. quick bookmark) using a pointing device, then press the space bar: a) The video should pause/unpause b) The button should not be triggered again

  2. When you click on a button inside the player (e.g. mute) using a pointing device, then press the space bar: a) The video should pause/unpause b) The button should not be triggered again

  3. When you focus a button outside the player using a keyboard, then press the space bar: a) The video should not pause/unpause b) The button should be triggered

  4. When you focus a button inside the player using a keyboard, then press the space bar: a) The video should not pause/unpause b) The button should be triggered

  5. In any case, the control should remain focused (the enter key should still toggle the button)

Damus765 avatar May 11 '24 15:05 Damus765

Hi @Damus765

Thank you so much for looking into it, however as we are currently in the works of switching to a different player (see pull request linked to this issue https://github.com/FreeTubeApp/FreeTube/pull/4978) that doesn't have the problem that this issue is about. I do still appreciate you putting so much work into it.

absidue avatar May 11 '24 16:05 absidue

Hi @Damus765

Thank you so much for looking into it, however as we are currently in the works of switching to a different player (see pull request linked to this issue #4978) that doesn't have the problem that this issue is about. I do still appreciate you putting so much work into it.

I tested your shaka-migration branch. And it only partially addresses this issue. It only fixes points 2.a and 2.b, as they are related to video.js (but regresses points 4.a and 4.b in the meantime). All the other buttons outside of the player are still affected by this issue.

In fact, I cherry-picked my commit on top of your branch and everything seems to work fine. I only had to make a small change to the keyboardShortcutHandler in ft-shaka-video-player.js. You can find my shaka-migration branch here to test it.

Of course, if you don't plan to release another version before the switch to Shaka Player, we could wait until the migration is complete. But if you do, I think it could be nice to have this issue fixed.

You tell me if I should make the PR now or wait.

Damus765 avatar May 11 '24 17:05 Damus765

tried the shaka version works great! space/arrows work all the time, wouldn't care about what button is keyboard focused.

just my opinion.

great job!

so no chance for win7 support? is there new stuff in electron you really need or would old version still work? maybe a separate win7 release.

tigros avatar May 12 '24 04:05 tigros

so no chance for win7 support? is there new stuff in electron you really need or would old version still work? maybe a separate win7 release.

As mentioned many times in the release discussion, no. We are a small team we don't have the time or the ability to test every single change on an abandoned operating system with a severely outdated version of Electron, that has known security issues like remote code execution just from downloading an image. Also yes we use stuff from newer Electron versions.

This is completely unofficial and the FreeTube team provides zero support for it, but some users mentioned that using FreeTube with https://github.com/vxiiduu/VxKex works on Windows 7.

absidue avatar May 12 '24 07:05 absidue

As far as I'm concerned, the main point in this issue is 1b). I find it confusing when the button I just clicked is triggered again when I just want to play/pause the video. But of course, better handling of keyboard navigation is also important.

To avoid confusion in the Skaka Player migration, I will wait until #4978 is merged before doing a PR to fix this issue.

Edit: Fixed link to PR.

Damus765 avatar May 13 '24 14:05 Damus765

To avoid confusion in the Skaka Player migration, I will wait until #4137 is merged before doing a PR to fix this issue.

@Damus765 i think you wanted to link a PR instead of this issue