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.

Sign up for GitHub

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

fixing-keyboard.js docs #7689

Merged
merged 2 commits into from
Apr 8, 2025
Merged

fixing-keyboard.js docs #7689

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fa31afa
fixing-keyboard.js docs
perminder-17 Mar 29, 2025
c93d7a7
updating the docs
perminder-17 Mar 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 34 additions & 30 deletions src/events/keyboard.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,32 +196,36 @@ function keyboard(p5, fn){
fn.key = '';

/**
* A `Number` system variable that contains the code of the last key typed.
* A `Number` system variable that contains the code of the last key pressed.
*
* All keys have a `keyCode`. For example, the `a` key has the `keyCode` 65.
* The `keyCode` variable is helpful for checking whether a special key has
* been typed. For example, the following conditional checks whether the enter
* key has been typed:
* Every key has a numeric key code. For example, the letter `a` key has the key code 65.
* Use this key code to determine which key was pressed by comparing it to the numeric value
* of the desired key.
*
* For example, to detect when the Enter key is pressed:
*
* ```js
* if (keyCode === 13) {
* // Code to run if the enter key was pressed.
* if (keyCode === 13) { // Enter key
* // Code to run if the Enter key was pressed.
* }
* ```
*
* The same code can be written more clearly using the system variable `ENTER`
* which has a value of 13:
* Alternatively, you can use the <a href="#/p5/key">key</a> function to directly compare the key value:
*
* ```js
* if (keyCode === ENTER) {
* // Code to run if the enter key was pressed.
* if (key === 'Enter') { // Enter key
* // Code to run if the Enter key was pressed.
* }
* ```
*
* The system variables `BACKSPACE`, `DELETE`, `ENTER`, `RETURN`, `TAB`,
* `ESCAPE`, `SHIFT`, `CONTROL`, `OPTION`, `ALT`, `UP_ARROW`, `DOWN_ARROW`,
* `LEFT_ARROW`, and `RIGHT_ARROW` are all helpful shorthands the key codes of
* special keys. Key codes can be found on websites such as
* Use the following numeric codes for the arrow keys:
*
* Up Arrow: 38
* Down Arrow: 40
* Left Arrow: 37
* Right Arrow: 39
*
* More key codes can be found at websites such as
* <a href="http://keycode.info/">keycode.info</a>.
*
* @property {Integer} keyCode
Expand Down Expand Up @@ -273,13 +277,13 @@ function keyboard(p5, fn){
* function draw() {
* // Update x and y if an arrow key is pressed.
* if (keyIsPressed === true) {
* if (keyCode === UP_ARROW) {
* if (keyCode === 38) { // Up arrow key
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if a select(key) would be better or will it cause confusion for some users? We could simplify the if else logic, what do you think?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

// Update x and y if an arrow key is pressed.
if (keyIsPressed === true) {
  switch (keyCode) {
    case 38: // Up arrow key
      y -= 1;
      break;
    case 40: // Down arrow key
      y += 1;
      break;
    case 37: // Left arrow key
      x -= 1;
      break;
    case 39: // Right arrow key
      x += 1;
      break;
  }
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok leaving this one as is for now since after a quick search, we don't use switch/case in other reference examples so far, so while it can be simplified, it might be a little easier to understand for now if we use the more familiar if/else` construct.

* y -= 1;
* } else if (keyCode === DOWN_ARROW) {
* } else if (keyCode === 40) { // Down arrow key
* y += 1;
* } else if (keyCode === LEFT_ARROW) {
* } else if (keyCode === 37) { // Left arrow key
* x -= 1;
* } else if (keyCode === RIGHT_ARROW) {
* } else if (keyCode === 39) { // Right arrow key
* x += 1;
* }
* }
Expand Down Expand Up @@ -317,7 +321,7 @@ function keyboard(p5, fn){
* // Code to run.
* }
*
* if (keyCode === ENTER) {
* if (keyCode === 13) { // Enter key
* // Code to run.
* }
* }
Expand Down Expand Up @@ -443,9 +447,9 @@ function keyboard(p5, fn){
*
* // Toggle the background color when the user presses an arrow key.
* function keyPressed() {
* if (keyCode === LEFT_ARROW) {
* if (keyCode === 37) { // Left arrow key
* value = 255;
* } else if (keyCode === RIGHT_ARROW) {
* } else if (keyCode === 39) { // Right arrow key
* value = 0;
* }
* // Uncomment to prevent any default behavior.
Expand Down Expand Up @@ -497,7 +501,7 @@ function keyboard(p5, fn){
* // Code to run.
* }
*
* if (keyCode === ENTER) {
* if (keyCode === 13) { // Enter key
* // Code to run.
* }
* }
Expand Down Expand Up @@ -620,9 +624,9 @@ function keyboard(p5, fn){
*
* // Toggle the background color when the user releases an arrow key.
* function keyReleased() {
* if (keyCode === LEFT_ARROW) {
* if (keyCode === 37) { // Left arrow key
* value = 255;
* } else if (keyCode === RIGHT_ARROW) {
* } else if (keyCode === 39) { // Right arrow key
* value = 0;
* }
* // Uncomment to prevent any default behavior.
Expand Down Expand Up @@ -683,7 +687,7 @@ function keyboard(p5, fn){
* }
*
* // Check for "c" using keyCode.
* if (keyCode === 67) {
* if (keyCode === 67) { // 67 is the ASCII code for 'c'
* // Code to run.
* }
* }
Expand Down Expand Up @@ -899,19 +903,19 @@ function keyboard(p5, fn){
*
* function draw() {
* // Update x and y if an arrow key is pressed.
* if (keyIsDown(37) === true) {
* if (keyIsDown('ArrowLeft') === true) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while this is correct and same from the old version, I think its a pattern we should discourage, maybe add a note at the begining that keyIsDown() returns true and false, and we don't need to explicitly compare with the truth value.

Copy link
Member

@ksen0 ksen0 Apr 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would support simplifying this as you suggest.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I generally also agree, but also wanted to flag that this is consistent with what the current documentation style guide has, so we might want to also update that if we're considering changing the pattern we want to use.

Copy link
Contributor Author

@perminder-17 perminder-17 Apr 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it’s valuable to maintain the explicit comparison with === true because it ensures both clarity and consistency with our current style guide. By explicitly checking for the boolean true, we remove any ambiguity around type coercion, making the code more readable and easier to reason about, especially for newer contributors who might be less familiar with how JavaScript treats truthy and falsy values. Additionally, if the style guide specifically calls out this convention, it’s simpler and more consistent to uphold it in the code rather than change the guide itself. This keeps our documentation and code aligned without introducing extra work or confusion about whether keyIsDown is always guaranteed to return a strict boolean.

Currently we could keep this as it is and in future if it comes up, I would be more than happy to change the style-guide and replace by not comparing with truth values? Let me know your thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think at this point it's ok to leave as-is because probably other things are priority if that's ok with everyone else. But maybe we can make an issue to discuss changing the style guide + updating other docs?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, its a matter of style, and if the explicit type comparison to true makes sense in this context lets keep it.

* x -= 1;
* }
*
* if (keyIsDown(39) === true) {
* if (keyIsDown('ArrowRight') === true) {
* x += 1;
* }
*
* if (keyIsDown(38) === true) {
* if (keyIsDown('ArrowUp') === true) {
* y -= 1;
* }
*
* if (keyIsDown(40) === true) {
* if (keyIsDown('ArrowDown') === true) {
* y += 1;
* }
*
Expand Down