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

Add slider keyboard functionality #2827

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jadeddelta
Copy link
Contributor

Hi all, this PR was made from the suggestion in #2570, by adding three fields enable_keys, keys_adjust, and keys_panning in order to allow a participant to use the keyboard to pan through a slider.

This is a draft for now as I'd like to make sure that everything looks good before I can potentially add this functionality to the rest of the slider response plugins.

@changeset-bot
Copy link

changeset-bot bot commented Oct 18, 2022

⚠️ No Changeset found

Latest commit: cd53450

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@jodeleeuw
Copy link
Member

🎉

what about using pluginAPI.getKeyboardResponse({persist:true}) to handle the keyboard listener? It's not necessary, but makes it more consistent with other keyboard events.

@jadeddelta
Copy link
Contributor Author

@jodeleeuw all done! interesting thing to note though is i had to use ALL_KEYS for both listeners, if i tried to specify which keys were allowed by concatenating the two key field arrays, it would throw an error.

@jodeleeuw
Copy link
Member

Hmm it should be possible to use one listener and register only the set of valid keys. What error was being generated?

@jadeddelta
Copy link
Contributor Author

this kind of error, since keys_adjust and keys_panning can technically be any or none character (indicated by a string), it throws a hissy fit when you try to concatenate a string | string[] with another string | string[]
image

@nikbpetrov
Copy link
Contributor

Just one thing to flag is that it is not entirely clear to me how the 'panning' would work. Using the jsfiddle example from #2570, it's unclear to me why the keys 3 and 5 pressed in succession should lock to 30 and 50, instead of 35. The keys_panning argument suggests (I have not looked at the code itself) that the 1-6 keys would move the slider to the values of the slider equidistantly, i.e. if there are 12 ticks on the slider, the key 1 would move the slider to the value fo 2, the key 3 to the value of 6 and so on - this, it feels to me, is far from an intuitive use of a slider; instead, I would expect participants to be able to type in their value, e.g. press 3 and 5 and set the value to 35.

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.

None yet

3 participants