Issue #1277

[bryanhoffman]

…p and down arrow keys are pressed

resolves #1277

Link: #1277

[ksen0]

Thanks for finding this bug and reporting it! (In the future @bryanhoffman it would be really helpful if you waited to be assigned to work on an issue before filing a PR. I know this can require a bit more patience, but it also helps reduce duplicate work or avoidable revisions. Thank you for understanding!)

In this case, I'd like to reconsider the approach of adding a windowlistener. I would like to investigate a bit whether this might be a regression (I don't think so? @davepagurek does this seem like the behavior that's always been there?). If you have ideas on how to fix this in the component that displays the code and the sketch (not the sketch code itself), you are welcome to revise this.

Also cc @doradocodes if you have any ideas related here!

[ksen0]

Let's look for a solution that doesn't require changing sketch code itself, if at all possible!

[bryanhoffman]

@ksen0 I'm sorry! I thought what I was doing was helpful, but I'm very new to open source contributions. I don't know what happens on your end when I submit a PR. When I submit a PR, what "duplicate work or avoidable revisions" does it create? I appreciate the feedback, I want to be helpful and learn. Sorry for making a mess!

As for fixing this without altering example code, if I run:

window.addEventListener("keydown", (e) => {
  if (["ArrowUp", "ArrowDown", " "].includes(e.key)) e.preventDefault();
}, { passive: false });

in the console after changing it so it's running in the about:srcdoc context, it fixes the scrolling issue. I've attempted to place the logic in an appropriate place namely the src/components/CodeEmbed/frame.tsx file.

[ksen0]

@bryanhoffman no worries at all! n this case "duplicate work" doesn't apply , but we have the guideline against unassigned PRs (in contributor guidelines) because sometimes people make PRs to an issue someone else is already working on, which mean the unassigned PR has to be closed. When I say "avoidable revisions" I mean if code is written before there's an agreement on what actually the fix should be.

In this example, what could have been done differently (what to do in the future): make the issue. Comment on the issue how you would fix it. A maintainer or steward (scroll down to the table here to see who that would be) would check and reply, assigning you the issue and providing feedback on the approach.

The p5.js project is now over a decade old. The website is newer, but has many complex integrations. Even fixes that seem simple usually aren't and the process suggested in the guidelines is the way it is to try to build in ways to give technical guidance before contributors invest a lot of energy into a solution. I hope that clarifies! And welcome :) !

[ksen0]

It is very important not to make keyboard navigation unusable on the site as a whole: (WCAG 2.1)

cc website accessibility steward @coseeian - in this case, the sketch does something on keyboard arrow up/down. I assume the proposed solution, which ignores arrow up and down, would make keyboard navigation not usable. However, if it is possible to only ignore these when there is a sketch "in focus", would that be ok?

I am actually not sure how complex it would be to only do this when a sketch is "in focus" @bryanhoffman, if the current sketch viewer component has such information

[bryanhoffman]

@ksen0 It should only ignore up and down arrows and the space bar when the sketch is "in focus." If I click the sketch it will start to ignore these keys, but if I then click outside of the sketch, the browser starts to "hear" up, down, and space again. I verified this locally. I think that should be suitable for accessibility concerns.

[coseeian]

I’d like to raise a concern regarding the global application of this change.

Intercepting these keys by default may inadvertently break examples that rely on native browser behavior. For instance, in the DOM Form Elements example, the createSelect() dropdown becomes nearly unusable because the arrow keys are blocked, preventing users from navigating options. This creates a significant accessibility barrier for keyboard-reliant users.

Perhaps we should narrow the scope of this intervention to specific cases that require it (e.g., the Snake Game) while leaving the default iframe behavior untouched.

[bryanhoffman]

@coseeian Here's my first PR where I put the logic into the example code itself: bryanhoffman@ffb29aefd

Do you think this approach is preferrable?

Alternatively, I'm starting to think that this might not be a website issue, but actually something needing ported to the p5.js library when keyIsPressed() or keyPressed() is used. I'm thinking the logic would go approximately here: https://github.com/processing/p5.js/blob/9206f8d542b248d767ecdc652a26de988926952c/src/events/keyboard.js#L443

I'm happy to hear thoughts on this. Thanks to everyone for the help!

[bryanhoffman]

Sorry for back to back comments. I want to expound a little more on these couple of options. The more nuclear option is adding something like:

// Block the default browser scrolling
const reservedKeys = [32, 37, 38, 39, 40]; // Space, Left, Up, Right, Down
if (reservedKeys.includes(e.keyCode)) {
  e.preventDefault();
}

to the p5.prototype._onkeydown function on line 443.

One more alternative is the creation of a disableScrollKeys() function within the p5.js library. So, this would still rely on developers adding this function to their examples and would result in the example code changing but it seems to embody the friendly philosophy of the p5.js project.