polimediaupv / paella-core

Paella Player core library
Educational Community License v2.0
20 stars 15 forks source link

Keyboard shortcuts should not show/enable shortcuts for functions that are not enabled #340

Closed karendolan closed 8 months ago

karendolan commented 11 months ago

Shortcut items are showing on the shortcut UI that are not enabled. For example the volume and fullscreen control buttons have code to check if volume and fullscreen are allowed. The buttons do not show up in the player if their function is not allowed.

The default shortcuts, from paella-core, are where the keyboardShortcutsHelp, from paella-basic-plugins, gets the list of keys to show as shortcuts in the shortcut help UI, but no easy way to see if those shortcuts are useable in the player in that device.

https://github.com/polimediaupv/paella-core/blob/main/src/js/plugins/es.upv.paella.defaultShortcuts.js#L225-L229

Is there a way to make the default shortcut keys dynamic so that shortcuts for functions that are not enabled are not returned in the getKeys() function?

ferserc1 commented 8 months ago

There can be two interpretations here:

The first case should be contemplated in the keyboard shortcut plugin. For example, on an iPad with a keyboard connected it would show that the up arrow and down arrow key allow to change the audio volume, when it is not possible. The problem is that the default shortcut plugin does not contemplate this scenario. This is a bug, and I have created a ticket to fix it (https://github.com/polimediaupv/paella-core/issues/350).

In the second case, the keyboard shortcut plugins have no relation with the button plugins, and as they are completely separate, there is no way to make it so that if you disable a button, the shortcut is also disabled. The shortcuts are defined in the en.upv.paella.defaultShortcuts plugin, inside paella-core, while the rest of the buttons are independent and are in other repositories. The shortcuts that are defined in a shortcuts plugin can be queried by an API, and in this way the button plugin that displays the help is able to know which keyboard shortcuts are defined. Note that the defaultShortcuts configuration also include other parameters that are duplicated in other plugins, such are the valid playback rates and the skip forward and backwards time:

"es.upv.paella.defaultShortcuts": {
    "enabled": true,
    "validPlaybackRates": [0.75, 2, 1, 1.5],
    "skipBackwards": 10,
    "skipForward": 10,
    "order": 1
},

So, this case is not so simple. Here what would have to be done is to modify the defaultShortcuts plugin to allow to enable and disable actions through the configuration, although currently I can't find any reason to remove these shortcuts from the player. It is understandable that you do not want to display the step forward and step backwards buttons in the user interface for reasons of space in the button bar, but it does not make sense to relate this to the keyboard shortcuts, which can still be active.

You may want to define a custom keyboard shortcut plugin to cover other scenarios. For example, if you have implemented some quiz system, it may be desirable to disable the step forward or step backwards shortcut. The default paella-core keyboard shortcut plugin is not intended to take these cases into account, and therefore it has been defined as a plugin, so that the developer is left free to implement more specific scenarios.

In any case, defining a custom keyboard shortcuts plugin is very simple and you can check the documentation here:

https://paellaplayer.upv.es/#/doc/key_shortcuts.md