Skip to content

fix: restore passive focus on full-block fields in Zelos - #10559

Open
minwookshin wants to merge 4 commits into
RaspberryPiFoundation:mainfrom
minwookshin:fix/full-block-field-passive-focus
Open

minwookshin wants to merge 4 commits into
RaspberryPiFoundation:mainfrom
minwookshin:fix/full-block-field-passive-focus

Conversation

@minwookshin

Copy link
Copy Markdown

Fixed the missing passive focus outline on full-block fields in Zelos. Ran regression tests and reviewed the changes.

Fixes #10504.

AI assistance: OpenAI Codex (implementation and regression tests).

@minwookshin
minwookshin requested a review from a team as a code owner October 4, 2026 02:28
@minwookshin
minwookshin requested a review from mikeharv October 4, 2026 02:28
@github-actions github-actions Bot added the PR: fix Fixes a bug label Oct 4, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Welcome! It looks like this is your first pull request in Blockly, so here are a couple of tips:

  • You can find tips about contributing to Blockly and how to validate your changes on our developer site.
  • We use conventional commits to make versioning the package easier. Make sure your commit message is in the proper format or learn how to fix it.
  • If any of the other checks on this PR fail, you can click on them to learn why. It might be that your change caused a test failure, or that you need to double-check the style guide.
    Thank you for opening this PR! A member of the Blockly team will review it soon.

@mikeharv mikeharv left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi Minwook! Thanks for submitting this fix. The change itself works, but I'm requesting a couple small changes before we can merge this.

  • Use dom.addClass instead of this.fieldGroup_.classList.add(...). The rest of Field uses the helper already, including in initView() a few lines below.
  • The AI generated tests are a little too padded for what this change is. I think we can cut the test down to a single full-block case. It would be good if we could do this without depending on the exact 5px 3px values.

Please re-tag me for review when you'd like me to take another look.

@mikeharv mikeharv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for making those changes. Instead of creating a brand new test file, could we add this new test to a new or existing suite in keyboard_navigation_test.ts?

@minwookshin

Copy link
Copy Markdown
Author

Moved the test to keyboard_navigation_test.ts and reran the tests. @mikeharv, could you take another look?

This branch has not been deployed

No deployments
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.

Passive focus not displayed on full-block fields in Zelos

2 participants