Skip to content

Adding docs for code in the refrence. #7902

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

Open
wants to merge 2 commits into
base: dev-2.0
Choose a base branch
from

Conversation

perminder-17
Copy link
Collaborator

Resolves #7881

Changes:
Added documentation and examples for code function.

PR Checklist

* of the current keyboard layout (QWERTY, Dvorak, AZERTY, etc.) or the character
* that appears in a text field.
*
* The code property returns a plain string (e.g. 'ArrowRight'), you can
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @perminder-17 ,
just a small thing - (e.g. 'ArrowRight') to be (e.g., 'ArrowRight') a comma (,) after the abbreviation, for stating eaxmple and probably a full-stop after it (e.g. 'ArrowRight'), --> (e.g., 'ArrowRight'). You can....

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for catching this. Do you think the PR looks good to you now?

Copy link
Member

@ksen0 ksen0 left a comment

Choose a reason for hiding this comment

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

I've added some comments, please feel free to update based on the feedback where you think it's appropriate. Thank you for adding this!

*
* <div>
* <code>
* // Click on the canvas to begin detecting key presses.
Copy link
Member

Choose a reason for hiding this comment

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

Hi @perminder-17 ! Apologies for the delay on this. What do you think about linking to the code docs (https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/code) at the end of the description - and also adding an example that's inspired by their interactive one? (Or by this one https://www.toptal.com/developers/keycode that we have linked before). Something that shows each of the values of key and code? Here's a sketch I made: https://editor.p5js.org/ksen0/sketches/oxgLK9HIV you're welcoem to use it as a starting point if you think it's a good idea. That sketch is actually completely backwards compatible except for the line with code so maybe it could also be added to the 1.x reference?

Lastly, while reviewing the transition guide I suggested an overview table https://github.com/processing/p5.js-compatibility/pull/27/files#r2156196242 - maybe the 2.x reference could use the info in just the second column, if you think it improves clarity?

*
* Unlike <a href="#/p5/key">key</a>, the `code` property differentiates between
* physical keys that generate the same character—for example, `CtrlLeft` and
* `CtrlRight`—so each can be handled independently.
Copy link
Member

Choose a reason for hiding this comment

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

I think linking to the mozilla code docs at the end of this would be useful?

* The code property returns a plain string (e.g., 'ArrowRight'). You can
* compare it directly with string literals:
* ```js
* if (code === 'ArrowRight') {
Copy link
Member

Choose a reason for hiding this comment

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

I think because keyIsDown is a backwards-compatible pattern with the system variables, we should only show that in the example code, what do you think?

And explain that it's the same as code === 'ArrowRight' and key === 'ArrowRight'

* }
* ```
*
* For extra clarity, you can instead use the predefined key-code constants
Copy link
Member

Choose a reason for hiding this comment

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

I think the way these are introduced and described in the previous docs (https://p5js.org/reference/p5/keyCode/) is really concise, maybe this can be used instead? And not make it sound like an optional/extra, but rather the recommended way to use keyboard events?

* }
*
* function draw() {
* // Update x and y if an arrow key is pressed.
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about using keyIsDown here as well? https://editor.p5js.org/ksen0/sketches/vrMANhunx Main benefit: backwards compatible snippet. Main drawback: makes this reference page less standalone / use another function. Up to you if you think it's a good idea!

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.

[p5.js 2.0 Bug Report]: Add documentation for the code system variable in key handling.
3 participants