Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix vlite halts native keyboard events when using HTML input Elements #57

Merged

Conversation

bfiessinger
Copy link
Contributor

@bfiessinger bfiessinger commented Oct 20, 2021

When vlitejs is implemented on a page where also input Elements are located you are unable to use these form inputs.
This is especially an issue when trying to type something for example inside a text input. You are not allowed to type spaces, move backwards or forwards in text and so on.

Quick Example:
https://jsfiddle.net/2b0uxmew/4/

bfiessinger added a commit to bfiessinger/vlitejs that referenced this pull request Oct 20, 2021
@bfiessinger bfiessinger requested a review from yoriiis October 22, 2021 07:37
@Jackky90
Copy link

Got the same issue: unable to type text normally even vlite not in viewport. Wondering why there is no option to disable keyboard actions completely? Cuz I don't need them totally in my current projects, for example.

@yoriiis yoriiis merged commit 241bd23 into vlitejs:main Oct 22, 2021
@yoriiis
Copy link
Member

yoriiis commented Oct 22, 2021

@Jackky90 Adding an option is possible, but it concerns a fairly specific case. Normally this merge request corrects the problem.

@Jackky90
Copy link

@Jackky90 Adding an option is possible, but it concerns a fairly specific case. Normally this merge request corrects the problem.

You have just unbind several tags, but that's not a pure solution. There can be any DOM node, listens to space key even a custom div. Also I've tried to create several players on the page and pressed down space -- result is the all players start playing simultaneously, that's not ok, this type of behavior breaks accessibility absolutely. There should be an option to disable key bindings plus, of course, player should listen keydown events only while focused, but not through the entire document.

@Jackky90
Copy link

Jackky90 commented Oct 22, 2021

Have solved for myself by this 'hack' within onReady method

onReady: player =>
{
   document.removeEventListener( 'keydown', player.Vlitejs.onKeydown )
}

Also noticed player doesn't respect the option of disabling fullscreen mode -- it's just hiding the fs button, but still listen to dblclick event. The another dirty hack, I used is
player.Vlitejs.container.removeEventListener( 'dblclick', player.Vlitejs.onDoubleClickOnPlayer )

@yoriiis
Copy link
Member

yoriiis commented Oct 22, 2021

For editable DOM nodes, the current condition covers most cases imo. If there are other use case, an issue can be created.

For the problem related to focus and accessibility, indeed it is a bug, can you open an issue?

@Jackky90
Copy link

For editable DOM nodes, the current condition covers most cases imo. If there are other use case, an issue can be created.

For the problem related to focus and accessibility, indeed it is a bug, can you open an issue?

For both: key bindings and fs mode?

@Jackky90
Copy link

contenteditable check doesn't solve this in total. I repeat: ANY node can be space key accessible. Example: fake focusable div node, that behaves like submit button with js. Also default space key pressing scrolls the page down, but in your case it will always start toggling all the players on the page: really? I think you should just inspect the basic youtube embed accessibility with a keyboard. Just place several embeds on the page and check, how it behaves on keydown events: each player will respond to key pressing only while it's in focus.

@bfiessinger
Copy link
Contributor Author

What about adding two things:

  1. an option to completly disable kb events
  2. a method to toggle kb events

yoriiis added a commit that referenced this pull request Oct 22, 2021
Move keydown event to the player element
Player has the focus on big play button click and on subtitle button click
Remove keydown restriction on specific tags (#57)
Limit keydown action when the player or children's player has the focus
Replace querySelector by cached elements
Add focus on first subtitle button when the subtitle menu is opened
@yoriiis yoriiis mentioned this pull request Oct 22, 2021
@yoriiis
Copy link
Member

yoriiis commented Oct 22, 2021

The main problem is that the "focus" is attached to the document. If the "focus" were attached to each player element, there wouldn't be these conflicts with keyboard events fired outside of the player context.

I started an accessibility optimization in #58.

@Jackky90 It works well with several players on the same page and the accessibility is not broken.
@bfiessinger The option may not be needed if #58 resolve these issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants