Skip to content

Conversation

@gonfunko
Copy link
Contributor

@gonfunko gonfunko commented Jul 7, 2025

The basics

The details

This PR addresses two points of feedback raised in RaspberryPiFoundation/blockly-keyboard-experimentation#625:

  • The workspace comment textarea has its tabindex set to -1, preventing tabbing into it. It remains accessible via keyboard navigation.
  • When workspace comments are focused, they will be scrolled into view on the workspace.

@gonfunko gonfunko requested a review from a team as a code owner July 7, 2025 21:19
@gonfunko gonfunko requested a review from maribethb July 7, 2025 21:19
@github-actions github-actions bot added the PR: fix Fixes a bug label Jul 7, 2025
Copy link
Contributor

@maribethb maribethb left a comment

Choose a reason for hiding this comment

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

lgtm when tests pass. though i don't think the current approach to scrolling into view is very nice, that is a long term problem to fix.

do you think we need an interface for "Scroll-into-view-able"?

or should these components do this automatically in their "onNodeFocus" methods? is there a time when a node is focused that we wouldn't want to scroll them into view?

these are just rhetorical questions for now. i'll file an issue for them.

@gonfunko
Copy link
Contributor Author

gonfunko commented Jul 7, 2025

Thanks! I don't think the test failures are actually related to this PR - they pass locally, and nothing in this would make Blockly.getMainWorkspace() not be a function.

@maribethb
Copy link
Contributor

Thanks! I don't think the test failures are actually related to this PR - they pass locally, and nothing in this would make Blockly.getMainWorkspace() not be a function.

fair, i didn't investigate what the failures actually were

@gonfunko gonfunko merged commit e3d17be into develop Jul 7, 2025
18 of 21 checks passed
@gonfunko gonfunko deleted the workspace-comment-tabindex branch July 7, 2025 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: fix Fixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants