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

[p5.js 2.0 RFC Proposal]: expand keyIsDown() to work with characters as arguments #6798

Open
3 of 21 tasks
kjhollen opened this issue Feb 7, 2024 · 5 comments · May be fixed by #6993
Open
3 of 21 tasks

[p5.js 2.0 RFC Proposal]: expand keyIsDown() to work with characters as arguments #6798

kjhollen opened this issue Feb 7, 2024 · 5 comments · May be fixed by #6993

Comments

@kjhollen
Copy link
Member

kjhollen commented Feb 7, 2024

Increasing access

Having some symmetry where you can also do:

keyIsDown('a');

would be useful for teaching beginners.

Which types of changes would be made?

  • Breaking change (Add-on libraries or sketches will work differently even if their code stays the same.)
  • Systemic change (Many features or contributor workflows will be affected.)
  • Overdue change (Modifications will be made that have been desirable for a long time.)
  • Unsure (The community can help to determine the type of change.)

Most appropriate sub-area of p5.js?

  • Accessibility
  • Color
  • Core/Environment/Rendering
  • Data
  • DOM
  • Events
  • Image
  • IO
  • Math
  • Typography
  • Utilities
  • WebGL
  • Build process
  • Unit testing
  • Internationalization
  • Friendly errors
  • Other (specify if possible)

What's the problem?

keyIsDown() currently only works with keycodes. Sometimes this is confusing for my students, because they can use this for things like:

keyIsDown(LEFT_ARROW);
keyIsDown(ENTER);

but if they want to check if an alphanumeric key is down, the code is different:

keyIsPressed && key == 'a'

What's the solution?

keyIsDown() would need an addition that checks the type of the argument and allows passing text/characters. This means p5.js would have some symmetry where you can also do:

keyIsDown('a');

Pros (updated based on community comments)

Example list:

  • Consistency: This proposal increases API consistency by giving users a way to enter the name of an alphanumeric key for keyIsDown();
  • can use keyIsDown() without having to look up keycodes

Cons (updated based on community comments)

  • would this be confusing for cases like keyIsDown(4) where the user might mean the number 4? (note that 3, 8, and 9 are valid keycodes, so we can't just allow all numbers < 10)

Proposal status

Under review

@wwwld1
Copy link

wwwld1 commented Apr 2, 2024

Hi, I'd like to work on this, thank you!

@RuimingShen
Copy link
Contributor

Hi, I'd like to work on this, thank you! I think I solve it.

@wwwld1
Copy link

wwwld1 commented Apr 17, 2024

Sorry I don't know if this is a right pin @limzykenneth, I saw that you are under I/O and I am currently working on the implementation for this. It seems to be a conflict for the const "LEFT_ARROW" with the purposed solutions of using String as a input. I can propose 2 solutions.

  1. We change the validateParameters for keyIsDown to allow both type number and string for the code parameter (this allows to keep the constants DOWN_ARROW, RIGHT_ARROW, etc.
  2. Or we can keep things more consistent, and changing the parameter to allow allow strings, and make users type "DOWN_ARROW", "LEFT_ARROW", etc instead (and then maybe delete the constants for the arrow and other escape keys?).

@wwwld1
Copy link

wwwld1 commented Apr 20, 2024

Hey,

I made a working draft PR above for keyIsDown() to accept alphanumeric String params such as 'w' or 'W'. Regarding the confusing case of the int 4 versus string '4', this implementation includes both where the string parameter e.g. '4' reflects the ASCII value of '4' while the int parameter e.g. 4 reflects the ASCII code 4.

The documentation is also updated to reflect the changes with examples now included for the new changes.

Let me know if any unit tests are necessary for these changes (based on the current tests for keyIsDown() I do not think we need to add any additional tests)

Feedback is much appreciated :)

@wwwld1 wwwld1 linked a pull request Apr 26, 2024 that will close this issue
3 tasks
@mvicky2592
Copy link

@wwwld1 I think keyIsDown should also accept keys with names longer than one character such as "arrowUp", "command", and "space"

@limzykenneth limzykenneth self-assigned this Jun 18, 2024
@limzykenneth limzykenneth linked a pull request Sep 12, 2024 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Implementation
5 participants