Skip to content

Replaced deprecated keyCode functionality and docs with KeyboardEvent.code & KeyboardEvent.key also updates the keyIsDown function to accept alphanumerics as parameters #7472

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

Merged
merged 12 commits into from
Feb 4, 2025
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
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
72 changes: 42 additions & 30 deletions src/core/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -879,92 +879,104 @@ export const MITER = 'miter';
* @final
*/
export const AUTO = 'auto';

// INPUT
/**
* @typedef {18} ALT
* @typedef {'AltLeft' | 'AltRight'} ALT
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason why the typedef is both of these while the constant is just one?

* @property {ALT} ALT
* @final
*/
// INPUT
export const ALT = 18;
export const ALT = 'AltLeft';

/**
* @typedef {8} BACKSPACE
* @typedef {'Backspace'} BACKSPACE
* @property {BACKSPACE} BACKSPACE
* @final
*/
export const BACKSPACE = 8;
export const BACKSPACE = 'Backspace';

/**
* @typedef {17} CONTROL
* @typedef {'ControlLeft' | 'ControlRight'} CONTROL
* @property {CONTROL} CONTROL
* @final
*/
export const CONTROL = 17;
export const CONTROL = 'ControlLeft';
Copy link
Contributor

Choose a reason for hiding this comment

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

Each constant can only be defined as one thing, so we might need to make two constants instead of a single one

Copy link
Author

@Vaivaswat2244 Vaivaswat2244 Jan 30, 2025

Choose a reason for hiding this comment

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

@davepagurek So, I'm not entirely sure what to do for this.
Should I make two constants one for key and code, like...

export const CONTROL_CODE = 'Control';
export const CONTROL_KEY = 'ControlLeft';

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking originally having a const CONTROL_LEFT and CONTROL_RIGHT, but on second thought, the use case before of having a single CONTROL implies that it doesn't matter which. So it probably makes most sense to just export const CONTROL = 'Control so that it ends up using the key instead of the keyCode so that it would check for both.

I think this might mean we need to update isCode so that isCode('Control') returns false though. And for everything we do for Control, we would also do for Alt and Shift, since they have a left/right version too.


/**
* @typedef {46} DELETE
* @typedef {'Delete'} DELETE
* @property {DELETE} DELETE
* @final
*/
export const DELETE = 46;
export const DELETE = 'Delete';

/**
* @typedef {40} DOWN_ARROW
* @typedef {'ArrowDown'} DOWN_ARROW
* @property {DOWN_ARROW} DOWN_ARROW
* @final
*/
export const DOWN_ARROW = 40;
export const DOWN_ARROW = 'ArrowDown';

/**
* @typedef {13} ENTER
* @typedef {'Enter'} ENTER
* @property {ENTER} ENTER
* @final
*/
export const ENTER = 13;
export const ENTER = 'Enter';

/**
* @typedef {27} ESCAPE
* @typedef {'Escape'} ESCAPE
* @property {ESCAPE} ESCAPE
* @final
*/
export const ESCAPE = 27;
export const ESCAPE = 'Escape';

/**
* @typedef {37} LEFT_ARROW
* @typedef {'ArrowLeft'} LEFT_ARROW
* @property {LEFT_ARROW} LEFT_ARROW
* @final
*/
export const LEFT_ARROW = 37;
export const LEFT_ARROW = 'ArrowLeft';

/**
* @typedef {18} OPTION
* @typedef {'AltLeft' | 'AltRight'} OPTION
* @property {OPTION} OPTION
* @final
*/
export const OPTION = 18;
export const OPTION = 'AltLeft';

/**
* @typedef {13} RETURN
* @typedef {'Enter'} RETURN
* @property {RETURN} RETURN
* @final
*/
export const RETURN = 13;
export const RETURN = 'Enter';

/**
* @typedef {39} RIGHT_ARROW
* @typedef {'ArrowRight'} RIGHT_ARROW
* @property {RIGHT_ARROW} RIGHT_ARROW
* @final
*/
export const RIGHT_ARROW = 39;
export const RIGHT_ARROW = 'ArrowRight';

/**
* @typedef {16} SHIFT
* @typedef {'ShiftLeft' | 'ShiftRight'} SHIFT
* @property {SHIFT} SHIFT
* @final
*/
export const SHIFT = 16;
export const SHIFT = 'ShiftLeft';

/**
* @typedef {9} TAB
* @typedef {'Tab'} TAB
* @property {TAB} TAB
* @final
*/
export const TAB = 9;
export const TAB = 'Tab';

/**
* @typedef {38} UP_ARROW
* @typedef {'ArrowUp'} UP_ARROW
* @property {UP_ARROW} UP_ARROW
* @final
*/
export const UP_ARROW = 38;
export const UP_ARROW = 'ArrowUp';

// RENDERING
/**
Expand Down
1 change: 1 addition & 0 deletions src/core/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,7 @@ class p5 {

this._styles = [];
this._downKeys = {}; //Holds the key codes of currently pressed keys
this._downKeyCodes = {};
}
}

Expand Down
87 changes: 58 additions & 29 deletions src/events/keyboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,21 @@
* @for p5
* @requires core
*/

export function isCode(input) {
const leftRightKeys = [
'AltLeft', 'AltRight',
'ShiftLeft', 'ShiftRight',
'ControlLeft', 'ControlRight',
'MetaLeft', 'MetaRight',
];
if (leftRightKeys.includes(input)) {
return false;
}
if (typeof input !== 'string') {
return false;
}
return input.length > 1;
}
function keyboard(p5, fn){
/**
* A `Boolean` system variable that's `true` if any key is currently pressed
Expand Down Expand Up @@ -95,7 +109,9 @@ function keyboard(p5, fn){
* </code>
* </div>
*/

fn.keyIsPressed = false;
fn.code = null;

/**
* A `String` system variable that contains the value of the last key typed.
Expand Down Expand Up @@ -439,23 +455,16 @@ function keyboard(p5, fn){
* </div>
*/
fn._onkeydown = function(e) {
if (e.repeat) {
// Ignore repeated key events when holding down a key
if (this._downKeys[e.code]) {
return;
}

this.keyIsPressed = true;
this.keyCode = e.which;
this._downKeys[e.which] = true;
this.key = e.key || String.fromCharCode(e.which) || e.which;

// Track keys pressed with meta key
if (e.metaKey) {
if (!this._metaKeys) {
this._metaKeys = [];
}
this._metaKeys.push(e.which);
}
this.key = e.key;
this.code = e.code;
this._downKeyCodes[e.code] = true;
this._downKeys[e.key] = true;

const context = this._isGlobal ? window : this;
if (typeof context.keyPressed === 'function' && !e.charCode) {
Expand Down Expand Up @@ -623,18 +632,20 @@ function keyboard(p5, fn){
* </div>
*/
fn._onkeyup = function(e) {
this._downKeys[e.which] = false;
this.keyIsPressed = false;
this._lastKeyCodeTyped = null;
delete this._downKeyCodes[e.code];
delete this._downKeys[e.key];

if (e.key === 'Meta') { // Meta key codes
// When meta key is released, clear all keys pressed with it
if (this._metaKeys) {
this._metaKeys.forEach(key => {
this._downKeys[key] = false;
});
this._metaKeys = [];
}

if (!this._areDownKeys()) {
this.keyIsPressed = false;
this.key = '';
this.code = null;
} else {
// If other keys are still pressed, update code to the last pressed key
const lastPressedCode = Object.keys(this._downKeyCodes).pop();
this.code = lastPressedCode;
const lastPressedKey = Object.keys(this._downKeys).pop();
this.key = lastPressedKey;
}

const context = this._isGlobal ? window : this;
Expand Down Expand Up @@ -821,7 +832,7 @@ function keyboard(p5, fn){
* <a href="https://keycode.info" target="_blank">keycode.info</a>.
*
* @method keyIsDown
* @param {Number} code key to check.
* @param {Number|String} code key to check.
* @return {Boolean} whether the key is down or not.
*
* @example
Expand Down Expand Up @@ -913,11 +924,29 @@ function keyboard(p5, fn){
* </code>
* </div>
*/
fn.keyIsDown = function(code) {
// p5._validateParameters('keyIsDown', arguments);
return this._downKeys[code] || false;
};
function isCode(input) {
const leftRightKeys = [
Copy link
Contributor

Choose a reason for hiding this comment

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

Just noticed that we're defining isCode twice -- I think we can remove this one, and just use the exported one above.

Copy link
Author

Choose a reason for hiding this comment

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

Oh yeah, sure.

'AltLeft', 'AltRight',
'ShiftLeft', 'ShiftRight',
'ControlLeft', 'ControlRight',
'MetaLeft', 'MetaRight',
];
if (leftRightKeys.includes(input)) {
return false;
}
if (typeof input !== 'string') {
return false;
}
return input.length > 1;
}

fn.keyIsDown = function(input) {
if (isCode(input)) {
return this._downKeyCodes[input] || this._downKeys[input] || false;
} else {
return this._downKeys[input] || this._downKeyCodes[input] || false;
}
}
/**
* The _areDownKeys function returns a boolean true if any keys pressed
* and a false if no keys are currently pressed.
Expand Down
65 changes: 58 additions & 7 deletions test/unit/events/keyboard.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import p5 from '../../../src/app.js';
import { isCode } from '../../../src/events/keyboard.js';
import { parallelSketches } from '../../js/p5_helpers';

suite('Keyboard Events', function() {
Expand Down Expand Up @@ -164,18 +165,68 @@ suite('Keyboard Events', function() {
});
});

suite('isCode', function() {
test('returns false for non-string inputs', function() {
assert.isFalse(isCode(null));
assert.isFalse(isCode(undefined));
assert.isFalse(isCode(123));
assert.isFalse(isCode({}));
assert.isFalse(isCode([]));
});

test('returns false for single non-digit and digit characters', function() {
assert.isFalse(isCode('a'));
assert.isFalse(isCode('Z'));
assert.isFalse(isCode('1'));
assert.isFalse(isCode('2'));
assert.isFalse(isCode(' '));
});

test('returns true for multi-character strings', function() {
assert.isTrue(isCode('Enter'));
assert.isTrue(isCode('ArrowUp'));
assert.isTrue(isCode('Shift'));
assert.isTrue(isCode('Control'));
assert.isTrue(isCode('ab'));
});

test('returns false for strings for letright keys', function() {
assert.isFalse(isCode('AltLeft'));
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 we want AltLeft to return true actually, and just Alt to return false. I'm testing out the demo on MDN here: https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/code#try_it_out

Hitting the alt key, it outputs: KeyboardEvent: key='Alt' | code='AltLeft'

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, that's right.

Copy link
Author

Choose a reason for hiding this comment

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

fixed this, returning false for keys and true for codes

assert.isFalse(isCode('ShiftRight'));
});

test('handles edge cases correctly', function() {
assert.isFalse(isCode('')); // empty string
assert.isTrue(isCode('11')); // multi-digit number
assert.isTrue(isCode('1a')); // digit + letter
});
});

suite('p5.prototype.keyIsDown', function() {
test('keyIsDown should return a boolean', function() {
assert.isBoolean(myp5.keyIsDown(65));
assert.isBoolean(myp5.keyIsDown('a'));
assert.isBoolean(myp5.keyIsDown('Enter'));
});

test('keyIsDown should return true if key is down', function() {
window.dispatchEvent(new KeyboardEvent('keydown', { keyCode: 35 }));
assert.strictEqual(myp5.keyIsDown(35), true);
});

test.todo('keyIsDown should return false if key is not down', function() {
assert.strictEqual(myp5.keyIsDown(35), false);
// Test single character keys
window.dispatchEvent(new KeyboardEvent('keydown', { key: 'a' }));
assert.strictEqual(myp5.keyIsDown('a'), true);

// Test digit keys
window.dispatchEvent(new KeyboardEvent('keydown', { key: '1', code: 'Digit1' }));
assert.strictEqual(myp5.keyIsDown('1'), true);

// Test special keys
window.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', code: 'Enter' }));
assert.strictEqual(myp5.keyIsDown('Enter'), true);
});

test('keyIsDown should return false if key is not down', function() {
// Ensure key is not down
window.dispatchEvent(new KeyboardEvent('keyup'));
assert.strictEqual(myp5.keyIsDown('z'), false);

});
});
});