fix: Reenable support for tabbing between fields. by gonfunko · Pull Request #9049 · RaspberryPiFoundation/blockly

gonfunko · 2025-05-14T18:05:57Z

The basics

I validated my changes

The details

Resolves

Proposed Changes

This PR re-adds support for tabbing between editable fields to correct a regression. This is a bit complicated because, in Zelos, navigation is to blocks rather than fields for simple reporters, but it appears to work correctly in both Zelos and non-Zelos renderers.

rachel-fenichel · 2025-05-14T18:12:48Z

Can get tests added as part of #8915

BenHenning · 2025-05-14T18:57:54Z

+ WidgetDiv.hideIfOwner(this);
+ dropDownDiv.hideWithoutAnimation();
+ target.showEditor();


Does this correctly focus the new field, too?

Yes, ultimately it calls showInlineEditor() (on non-mobile) which calls focus() on the input.

## The basics - [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change) ## The details ### Resolves FixesRaspberryPiFoundation/blockly-keyboard-experimentation#578 Fixes part of #8915 ### Proposed Changes Ensures fields update focus to the next field when tabbing between field editors. The old behavior can be observed in [#578](RaspberryPiFoundation/blockly-keyboard-experimentation#578) and the new behavior can be observed here: [Screen recording 2025-06-25 1.39.28 PM.webm](https://github.com/user-attachments/assets/e00fcb55-5c20-4d5c-81a8-be9049cc0d7e) ### Reason for Changes Having focus reset back to the original field editor that was opened is an unexpected experience for users. This approach is better. Note that there are some separate changes added here, as well: - Connections and fields now check if their block IDs contain their indicator prefixes since this will all-but-guarantee focus breaks for those nodes. This is an excellent example of why #9171 is needed. - Some minor naming updates for `FieldInput`: it was incorrectly implying that key events are sent for changes to the `input` element used by the field editor, but they are actually `InputEvent`s per https://developer.mozilla.org/en-US/docs/Web/API/Element/input_event. ### Test Coverage New tests were added for field editing in general (since this seems to be missing), including tabbing support to ensure the fixes originally introduced in #9049. One new test has been added specifically for verifying that focus updates with tabbing. This has been verified to fail with the fix removed (as have all tabbing tests with the tabbing code from #9049 removed). Some specific notes for the test changes: - There's a slight test dependency inversion happening here. `FieldInput` is being tested with a specific `FieldNumber` class via real block loading. This isn't ideal, but it seems fine given the circumstances (otherwise a lot of extra setup would be necessary for the tests). - The workspace actually needs to be made visible during tests in order for focus to work correctly (though it's reset at the end of each test, but this may cause some flickering while the tests are running). - It's the case that a bunch of tests were actually setting up blocks incorrectly (i.e. not defining a must-have `id` property which caused some issues with the new field and connection ID validation checks). These tests have been corrected, but it's worth noting that the blocks are likely still technically wrong since they are not conforming to their TypeScript contracts. - Similar to the previous point, one test was incorrectly setting the first ID to be returned by the ID generator as `undefined` since (presumably due to a copy-and-paste error when the test was introduced) it was referencing a `TEST_BLOCK_ID` property that hadn't been defined for that test suite. This has been corrected as, without it, there are failures due to the new validation checks. - For the connection database checks, a new ID is generated instead of fixing the block ID to ensure that it's always unique even if called multiple times (otherwise a block ID would need to be piped through from the calling tests, or an invalid situation would need to be introduced where multiple blocks shared an ID; the former seemed unnecessary and the latter seemed nonideal). - There are distinct Geras/Zelos tests to validate the case where a full-block field should have its parent block, rather than the field itself, focused on tabbing. See this conversation for more context: #9173 (comment). ### Documentation No documentation changes should be needed here. ### Additional Information Nothing to add.

gonfunko · 2025-05-14T18:05:57Z

The basics

I validated my changes

The details

Resolves

Fixes#8821

Proposed Changes

This PR re-adds support for tabbing between editable fields to correct a regression. This is a bit complicated because, in Zelos, navigation is to blocks rather than fields for simple reporters, but it appears to work correctly in both Zelos and non-Zelos renderers.

rachel-fenichel · 2025-05-14T18:12:48Z

Can get tests added as part of #8915

BenHenning · 2025-05-14T18:57:54Z

+ WidgetDiv.hideIfOwner(this);
+ dropDownDiv.hideWithoutAnimation();
+ target.showEditor();


Does this correctly focus the new field, too?

Yes, ultimately it calls showInlineEditor() (on non-mobile) which calls focus() on the input.

## The basics - [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change) ## The details ### Resolves FixesRaspberryPiFoundation/blockly-keyboard-experimentation#578 Fixes part of #8915 ### Proposed Changes Ensures fields update focus to the next field when tabbing between field editors. The old behavior can be observed in [#578](RaspberryPiFoundation/blockly-keyboard-experimentation#578) and the new behavior can be observed here: [Screen recording 2025-06-25 1.39.28 PM.webm](https://github.com/user-attachments/assets/e00fcb55-5c20-4d5c-81a8-be9049cc0d7e) ### Reason for Changes Having focus reset back to the original field editor that was opened is an unexpected experience for users. This approach is better. Note that there are some separate changes added here, as well: - Connections and fields now check if their block IDs contain their indicator prefixes since this will all-but-guarantee focus breaks for those nodes. This is an excellent example of why #9171 is needed. - Some minor naming updates for `FieldInput`: it was incorrectly implying that key events are sent for changes to the `input` element used by the field editor, but they are actually `InputEvent`s per https://developer.mozilla.org/en-US/docs/Web/API/Element/input_event. ### Test Coverage New tests were added for field editing in general (since this seems to be missing), including tabbing support to ensure the fixes originally introduced in #9049. One new test has been added specifically for verifying that focus updates with tabbing. This has been verified to fail with the fix removed (as have all tabbing tests with the tabbing code from #9049 removed). Some specific notes for the test changes: - There's a slight test dependency inversion happening here. `FieldInput` is being tested with a specific `FieldNumber` class via real block loading. This isn't ideal, but it seems fine given the circumstances (otherwise a lot of extra setup would be necessary for the tests). - The workspace actually needs to be made visible during tests in order for focus to work correctly (though it's reset at the end of each test, but this may cause some flickering while the tests are running). - It's the case that a bunch of tests were actually setting up blocks incorrectly (i.e. not defining a must-have `id` property which caused some issues with the new field and connection ID validation checks). These tests have been corrected, but it's worth noting that the blocks are likely still technically wrong since they are not conforming to their TypeScript contracts. - Similar to the previous point, one test was incorrectly setting the first ID to be returned by the ID generator as `undefined` since (presumably due to a copy-and-paste error when the test was introduced) it was referencing a `TEST_BLOCK_ID` property that hadn't been defined for that test suite. This has been corrected as, without it, there are failures due to the new validation checks. - For the connection database checks, a new ID is generated instead of fixing the block ID to ensure that it's always unique even if called multiple times (otherwise a block ID would need to be piped through from the calling tests, or an invalid situation would need to be introduced where multiple blocks shared an ID; the former seemed unnecessary and the latter seemed nonideal). - There are distinct Geras/Zelos tests to validate the case where a full-block field should have its parent block, rather than the field itself, focused on tabbing. See this conversation for more context: #9173 (comment). ### Documentation No documentation changes should be needed here. ### Additional Information Nothing to add.

gonfunko · 2025-05-14T18:05:57Z

The basics

I validated my changes

The details

Resolves

Fixes#8821

Proposed Changes

This PR re-adds support for tabbing between editable fields to correct a regression. This is a bit complicated because, in Zelos, navigation is to blocks rather than fields for simple reporters, but it appears to work correctly in both Zelos and non-Zelos renderers.

rachel-fenichel · 2025-05-14T18:12:48Z

Can get tests added as part of #8915

BenHenning · 2025-05-14T18:57:54Z

+ WidgetDiv.hideIfOwner(this);
+ dropDownDiv.hideWithoutAnimation();
+ target.showEditor();


Does this correctly focus the new field, too?

Yes, ultimately it calls showInlineEditor() (on non-mobile) which calls focus() on the input.

## The basics - [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change) ## The details ### Resolves FixesRaspberryPiFoundation/blockly-keyboard-experimentation#578 Fixes part of #8915 ### Proposed Changes Ensures fields update focus to the next field when tabbing between field editors. The old behavior can be observed in [#578](RaspberryPiFoundation/blockly-keyboard-experimentation#578) and the new behavior can be observed here: [Screen recording 2025-06-25 1.39.28 PM.webm](https://github.com/user-attachments/assets/e00fcb55-5c20-4d5c-81a8-be9049cc0d7e) ### Reason for Changes Having focus reset back to the original field editor that was opened is an unexpected experience for users. This approach is better. Note that there are some separate changes added here, as well: - Connections and fields now check if their block IDs contain their indicator prefixes since this will all-but-guarantee focus breaks for those nodes. This is an excellent example of why #9171 is needed. - Some minor naming updates for `FieldInput`: it was incorrectly implying that key events are sent for changes to the `input` element used by the field editor, but they are actually `InputEvent`s per https://developer.mozilla.org/en-US/docs/Web/API/Element/input_event. ### Test Coverage New tests were added for field editing in general (since this seems to be missing), including tabbing support to ensure the fixes originally introduced in #9049. One new test has been added specifically for verifying that focus updates with tabbing. This has been verified to fail with the fix removed (as have all tabbing tests with the tabbing code from #9049 removed). Some specific notes for the test changes: - There's a slight test dependency inversion happening here. `FieldInput` is being tested with a specific `FieldNumber` class via real block loading. This isn't ideal, but it seems fine given the circumstances (otherwise a lot of extra setup would be necessary for the tests). - The workspace actually needs to be made visible during tests in order for focus to work correctly (though it's reset at the end of each test, but this may cause some flickering while the tests are running). - It's the case that a bunch of tests were actually setting up blocks incorrectly (i.e. not defining a must-have `id` property which caused some issues with the new field and connection ID validation checks). These tests have been corrected, but it's worth noting that the blocks are likely still technically wrong since they are not conforming to their TypeScript contracts. - Similar to the previous point, one test was incorrectly setting the first ID to be returned by the ID generator as `undefined` since (presumably due to a copy-and-paste error when the test was introduced) it was referencing a `TEST_BLOCK_ID` property that hadn't been defined for that test suite. This has been corrected as, without it, there are failures due to the new validation checks. - For the connection database checks, a new ID is generated instead of fixing the block ID to ensure that it's always unique even if called multiple times (otherwise a block ID would need to be piped through from the calling tests, or an invalid situation would need to be introduced where multiple blocks shared an ID; the former seemed unnecessary and the latter seemed nonideal). - There are distinct Geras/Zelos tests to validate the case where a full-block field should have its parent block, rather than the field itself, focused on tabbing. See this conversation for more context: #9173 (comment). ### Documentation No documentation changes should be needed here. ### Additional Information Nothing to add.

gonfunko · 2025-05-14T18:05:57Z

The basics

I validated my changes

The details

Resolves

Fixes#8821

Proposed Changes

This PR re-adds support for tabbing between editable fields to correct a regression. This is a bit complicated because, in Zelos, navigation is to blocks rather than fields for simple reporters, but it appears to work correctly in both Zelos and non-Zelos renderers.

rachel-fenichel · 2025-05-14T18:12:48Z

Can get tests added as part of #8915

BenHenning · 2025-05-14T18:57:54Z

+ WidgetDiv.hideIfOwner(this);
+ dropDownDiv.hideWithoutAnimation();
+ target.showEditor();


Does this correctly focus the new field, too?

Yes, ultimately it calls showInlineEditor() (on non-mobile) which calls focus() on the input.

## The basics - [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change) ## The details ### Resolves FixesRaspberryPiFoundation/blockly-keyboard-experimentation#578 Fixes part of #8915 ### Proposed Changes Ensures fields update focus to the next field when tabbing between field editors. The old behavior can be observed in [#578](RaspberryPiFoundation/blockly-keyboard-experimentation#578) and the new behavior can be observed here: [Screen recording 2025-06-25 1.39.28 PM.webm](https://github.com/user-attachments/assets/e00fcb55-5c20-4d5c-81a8-be9049cc0d7e) ### Reason for Changes Having focus reset back to the original field editor that was opened is an unexpected experience for users. This approach is better. Note that there are some separate changes added here, as well: - Connections and fields now check if their block IDs contain their indicator prefixes since this will all-but-guarantee focus breaks for those nodes. This is an excellent example of why #9171 is needed. - Some minor naming updates for `FieldInput`: it was incorrectly implying that key events are sent for changes to the `input` element used by the field editor, but they are actually `InputEvent`s per https://developer.mozilla.org/en-US/docs/Web/API/Element/input_event. ### Test Coverage New tests were added for field editing in general (since this seems to be missing), including tabbing support to ensure the fixes originally introduced in #9049. One new test has been added specifically for verifying that focus updates with tabbing. This has been verified to fail with the fix removed (as have all tabbing tests with the tabbing code from #9049 removed). Some specific notes for the test changes: - There's a slight test dependency inversion happening here. `FieldInput` is being tested with a specific `FieldNumber` class via real block loading. This isn't ideal, but it seems fine given the circumstances (otherwise a lot of extra setup would be necessary for the tests). - The workspace actually needs to be made visible during tests in order for focus to work correctly (though it's reset at the end of each test, but this may cause some flickering while the tests are running). - It's the case that a bunch of tests were actually setting up blocks incorrectly (i.e. not defining a must-have `id` property which caused some issues with the new field and connection ID validation checks). These tests have been corrected, but it's worth noting that the blocks are likely still technically wrong since they are not conforming to their TypeScript contracts. - Similar to the previous point, one test was incorrectly setting the first ID to be returned by the ID generator as `undefined` since (presumably due to a copy-and-paste error when the test was introduced) it was referencing a `TEST_BLOCK_ID` property that hadn't been defined for that test suite. This has been corrected as, without it, there are failures due to the new validation checks. - For the connection database checks, a new ID is generated instead of fixing the block ID to ensure that it's always unique even if called multiple times (otherwise a block ID would need to be piped through from the calling tests, or an invalid situation would need to be introduced where multiple blocks shared an ID; the former seemed unnecessary and the latter seemed nonideal). - There are distinct Geras/Zelos tests to validate the case where a full-block field should have its parent block, rather than the field itself, focused on tabbing. See this conversation for more context: #9173 (comment). ### Documentation No documentation changes should be needed here. ### Additional Information Nothing to add.

gonfunko · 2025-05-14T18:05:57Z

The basics

I validated my changes

The details

Resolves

Fixes#8821

Proposed Changes

This PR re-adds support for tabbing between editable fields to correct a regression. This is a bit complicated because, in Zelos, navigation is to blocks rather than fields for simple reporters, but it appears to work correctly in both Zelos and non-Zelos renderers.

rachel-fenichel · 2025-05-14T18:12:48Z

Can get tests added as part of #8915

BenHenning · 2025-05-14T18:57:54Z

+ WidgetDiv.hideIfOwner(this);
+ dropDownDiv.hideWithoutAnimation();
+ target.showEditor();


Does this correctly focus the new field, too?

Yes, ultimately it calls showInlineEditor() (on non-mobile) which calls focus() on the input.

## The basics - [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change) ## The details ### Resolves FixesRaspberryPiFoundation/blockly-keyboard-experimentation#578 Fixes part of #8915 ### Proposed Changes Ensures fields update focus to the next field when tabbing between field editors. The old behavior can be observed in [#578](RaspberryPiFoundation/blockly-keyboard-experimentation#578) and the new behavior can be observed here: [Screen recording 2025-06-25 1.39.28 PM.webm](https://github.com/user-attachments/assets/e00fcb55-5c20-4d5c-81a8-be9049cc0d7e) ### Reason for Changes Having focus reset back to the original field editor that was opened is an unexpected experience for users. This approach is better. Note that there are some separate changes added here, as well: - Connections and fields now check if their block IDs contain their indicator prefixes since this will all-but-guarantee focus breaks for those nodes. This is an excellent example of why #9171 is needed. - Some minor naming updates for `FieldInput`: it was incorrectly implying that key events are sent for changes to the `input` element used by the field editor, but they are actually `InputEvent`s per https://developer.mozilla.org/en-US/docs/Web/API/Element/input_event. ### Test Coverage New tests were added for field editing in general (since this seems to be missing), including tabbing support to ensure the fixes originally introduced in #9049. One new test has been added specifically for verifying that focus updates with tabbing. This has been verified to fail with the fix removed (as have all tabbing tests with the tabbing code from #9049 removed). Some specific notes for the test changes: - There's a slight test dependency inversion happening here. `FieldInput` is being tested with a specific `FieldNumber` class via real block loading. This isn't ideal, but it seems fine given the circumstances (otherwise a lot of extra setup would be necessary for the tests). - The workspace actually needs to be made visible during tests in order for focus to work correctly (though it's reset at the end of each test, but this may cause some flickering while the tests are running). - It's the case that a bunch of tests were actually setting up blocks incorrectly (i.e. not defining a must-have `id` property which caused some issues with the new field and connection ID validation checks). These tests have been corrected, but it's worth noting that the blocks are likely still technically wrong since they are not conforming to their TypeScript contracts. - Similar to the previous point, one test was incorrectly setting the first ID to be returned by the ID generator as `undefined` since (presumably due to a copy-and-paste error when the test was introduced) it was referencing a `TEST_BLOCK_ID` property that hadn't been defined for that test suite. This has been corrected as, without it, there are failures due to the new validation checks. - For the connection database checks, a new ID is generated instead of fixing the block ID to ensure that it's always unique even if called multiple times (otherwise a block ID would need to be piped through from the calling tests, or an invalid situation would need to be introduced where multiple blocks shared an ID; the former seemed unnecessary and the latter seemed nonideal). - There are distinct Geras/Zelos tests to validate the case where a full-block field should have its parent block, rather than the field itself, focused on tabbing. See this conversation for more context: #9173 (comment). ### Documentation No documentation changes should be needed here. ### Additional Information Nothing to add.

gonfunko · 2025-05-14T18:05:57Z

The basics

I validated my changes

The details

Resolves

Fixes#8821

Proposed Changes

This PR re-adds support for tabbing between editable fields to correct a regression. This is a bit complicated because, in Zelos, navigation is to blocks rather than fields for simple reporters, but it appears to work correctly in both Zelos and non-Zelos renderers.

rachel-fenichel · 2025-05-14T18:12:48Z

Can get tests added as part of #8915

BenHenning · 2025-05-14T18:57:54Z

+ WidgetDiv.hideIfOwner(this);
+ dropDownDiv.hideWithoutAnimation();
+ target.showEditor();


Does this correctly focus the new field, too?

Yes, ultimately it calls showInlineEditor() (on non-mobile) which calls focus() on the input.

## The basics - [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change) ## The details ### Resolves FixesRaspberryPiFoundation/blockly-keyboard-experimentation#578 Fixes part of #8915 ### Proposed Changes Ensures fields update focus to the next field when tabbing between field editors. The old behavior can be observed in [#578](RaspberryPiFoundation/blockly-keyboard-experimentation#578) and the new behavior can be observed here: [Screen recording 2025-06-25 1.39.28 PM.webm](https://github.com/user-attachments/assets/e00fcb55-5c20-4d5c-81a8-be9049cc0d7e) ### Reason for Changes Having focus reset back to the original field editor that was opened is an unexpected experience for users. This approach is better. Note that there are some separate changes added here, as well: - Connections and fields now check if their block IDs contain their indicator prefixes since this will all-but-guarantee focus breaks for those nodes. This is an excellent example of why #9171 is needed. - Some minor naming updates for `FieldInput`: it was incorrectly implying that key events are sent for changes to the `input` element used by the field editor, but they are actually `InputEvent`s per https://developer.mozilla.org/en-US/docs/Web/API/Element/input_event. ### Test Coverage New tests were added for field editing in general (since this seems to be missing), including tabbing support to ensure the fixes originally introduced in #9049. One new test has been added specifically for verifying that focus updates with tabbing. This has been verified to fail with the fix removed (as have all tabbing tests with the tabbing code from #9049 removed). Some specific notes for the test changes: - There's a slight test dependency inversion happening here. `FieldInput` is being tested with a specific `FieldNumber` class via real block loading. This isn't ideal, but it seems fine given the circumstances (otherwise a lot of extra setup would be necessary for the tests). - The workspace actually needs to be made visible during tests in order for focus to work correctly (though it's reset at the end of each test, but this may cause some flickering while the tests are running). - It's the case that a bunch of tests were actually setting up blocks incorrectly (i.e. not defining a must-have `id` property which caused some issues with the new field and connection ID validation checks). These tests have been corrected, but it's worth noting that the blocks are likely still technically wrong since they are not conforming to their TypeScript contracts. - Similar to the previous point, one test was incorrectly setting the first ID to be returned by the ID generator as `undefined` since (presumably due to a copy-and-paste error when the test was introduced) it was referencing a `TEST_BLOCK_ID` property that hadn't been defined for that test suite. This has been corrected as, without it, there are failures due to the new validation checks. - For the connection database checks, a new ID is generated instead of fixing the block ID to ensure that it's always unique even if called multiple times (otherwise a block ID would need to be piped through from the calling tests, or an invalid situation would need to be introduced where multiple blocks shared an ID; the former seemed unnecessary and the latter seemed nonideal). - There are distinct Geras/Zelos tests to validate the case where a full-block field should have its parent block, rather than the field itself, focused on tabbing. See this conversation for more context: #9173 (comment). ### Documentation No documentation changes should be needed here. ### Additional Information Nothing to add.

gonfunko · 2025-05-14T18:05:57Z

The basics

I validated my changes

The details

Resolves

Fixes#8821

Proposed Changes

This PR re-adds support for tabbing between editable fields to correct a regression. This is a bit complicated because, in Zelos, navigation is to blocks rather than fields for simple reporters, but it appears to work correctly in both Zelos and non-Zelos renderers.

rachel-fenichel · 2025-05-14T18:12:48Z

Can get tests added as part of #8915

BenHenning · 2025-05-14T18:57:54Z

+ WidgetDiv.hideIfOwner(this);
+ dropDownDiv.hideWithoutAnimation();
+ target.showEditor();


Does this correctly focus the new field, too?

Yes, ultimately it calls showInlineEditor() (on non-mobile) which calls focus() on the input.

## The basics - [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change) ## The details ### Resolves FixesRaspberryPiFoundation/blockly-keyboard-experimentation#578 Fixes part of #8915 ### Proposed Changes Ensures fields update focus to the next field when tabbing between field editors. The old behavior can be observed in [#578](RaspberryPiFoundation/blockly-keyboard-experimentation#578) and the new behavior can be observed here: [Screen recording 2025-06-25 1.39.28 PM.webm](https://github.com/user-attachments/assets/e00fcb55-5c20-4d5c-81a8-be9049cc0d7e) ### Reason for Changes Having focus reset back to the original field editor that was opened is an unexpected experience for users. This approach is better. Note that there are some separate changes added here, as well: - Connections and fields now check if their block IDs contain their indicator prefixes since this will all-but-guarantee focus breaks for those nodes. This is an excellent example of why #9171 is needed. - Some minor naming updates for `FieldInput`: it was incorrectly implying that key events are sent for changes to the `input` element used by the field editor, but they are actually `InputEvent`s per https://developer.mozilla.org/en-US/docs/Web/API/Element/input_event. ### Test Coverage New tests were added for field editing in general (since this seems to be missing), including tabbing support to ensure the fixes originally introduced in #9049. One new test has been added specifically for verifying that focus updates with tabbing. This has been verified to fail with the fix removed (as have all tabbing tests with the tabbing code from #9049 removed). Some specific notes for the test changes: - There's a slight test dependency inversion happening here. `FieldInput` is being tested with a specific `FieldNumber` class via real block loading. This isn't ideal, but it seems fine given the circumstances (otherwise a lot of extra setup would be necessary for the tests). - The workspace actually needs to be made visible during tests in order for focus to work correctly (though it's reset at the end of each test, but this may cause some flickering while the tests are running). - It's the case that a bunch of tests were actually setting up blocks incorrectly (i.e. not defining a must-have `id` property which caused some issues with the new field and connection ID validation checks). These tests have been corrected, but it's worth noting that the blocks are likely still technically wrong since they are not conforming to their TypeScript contracts. - Similar to the previous point, one test was incorrectly setting the first ID to be returned by the ID generator as `undefined` since (presumably due to a copy-and-paste error when the test was introduced) it was referencing a `TEST_BLOCK_ID` property that hadn't been defined for that test suite. This has been corrected as, without it, there are failures due to the new validation checks. - For the connection database checks, a new ID is generated instead of fixing the block ID to ensure that it's always unique even if called multiple times (otherwise a block ID would need to be piped through from the calling tests, or an invalid situation would need to be introduced where multiple blocks shared an ID; the former seemed unnecessary and the latter seemed nonideal). - There are distinct Geras/Zelos tests to validate the case where a full-block field should have its parent block, rather than the field itself, focused on tabbing. See this conversation for more context: #9173 (comment). ### Documentation No documentation changes should be needed here. ### Additional Information Nothing to add.

gonfunko · 2025-05-14T18:05:57Z

The basics

I validated my changes

The details

Resolves

Fixes#8821

Proposed Changes

This PR re-adds support for tabbing between editable fields to correct a regression. This is a bit complicated because, in Zelos, navigation is to blocks rather than fields for simple reporters, but it appears to work correctly in both Zelos and non-Zelos renderers.

rachel-fenichel · 2025-05-14T18:12:48Z

Can get tests added as part of #8915

BenHenning · 2025-05-14T18:57:54Z

+ WidgetDiv.hideIfOwner(this);
+ dropDownDiv.hideWithoutAnimation();
+ target.showEditor();


Does this correctly focus the new field, too?

Yes, ultimately it calls showInlineEditor() (on non-mobile) which calls focus() on the input.

## The basics - [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change) ## The details ### Resolves FixesRaspberryPiFoundation/blockly-keyboard-experimentation#578 Fixes part of #8915 ### Proposed Changes Ensures fields update focus to the next field when tabbing between field editors. The old behavior can be observed in [#578](RaspberryPiFoundation/blockly-keyboard-experimentation#578) and the new behavior can be observed here: [Screen recording 2025-06-25 1.39.28 PM.webm](https://github.com/user-attachments/assets/e00fcb55-5c20-4d5c-81a8-be9049cc0d7e) ### Reason for Changes Having focus reset back to the original field editor that was opened is an unexpected experience for users. This approach is better. Note that there are some separate changes added here, as well: - Connections and fields now check if their block IDs contain their indicator prefixes since this will all-but-guarantee focus breaks for those nodes. This is an excellent example of why #9171 is needed. - Some minor naming updates for `FieldInput`: it was incorrectly implying that key events are sent for changes to the `input` element used by the field editor, but they are actually `InputEvent`s per https://developer.mozilla.org/en-US/docs/Web/API/Element/input_event. ### Test Coverage New tests were added for field editing in general (since this seems to be missing), including tabbing support to ensure the fixes originally introduced in #9049. One new test has been added specifically for verifying that focus updates with tabbing. This has been verified to fail with the fix removed (as have all tabbing tests with the tabbing code from #9049 removed). Some specific notes for the test changes: - There's a slight test dependency inversion happening here. `FieldInput` is being tested with a specific `FieldNumber` class via real block loading. This isn't ideal, but it seems fine given the circumstances (otherwise a lot of extra setup would be necessary for the tests). - The workspace actually needs to be made visible during tests in order for focus to work correctly (though it's reset at the end of each test, but this may cause some flickering while the tests are running). - It's the case that a bunch of tests were actually setting up blocks incorrectly (i.e. not defining a must-have `id` property which caused some issues with the new field and connection ID validation checks). These tests have been corrected, but it's worth noting that the blocks are likely still technically wrong since they are not conforming to their TypeScript contracts. - Similar to the previous point, one test was incorrectly setting the first ID to be returned by the ID generator as `undefined` since (presumably due to a copy-and-paste error when the test was introduced) it was referencing a `TEST_BLOCK_ID` property that hadn't been defined for that test suite. This has been corrected as, without it, there are failures due to the new validation checks. - For the connection database checks, a new ID is generated instead of fixing the block ID to ensure that it's always unique even if called multiple times (otherwise a block ID would need to be piped through from the calling tests, or an invalid situation would need to be introduced where multiple blocks shared an ID; the former seemed unnecessary and the latter seemed nonideal). - There are distinct Geras/Zelos tests to validate the case where a full-block field should have its parent block, rather than the field itself, focused on tabbing. See this conversation for more context: #9173 (comment). ### Documentation No documentation changes should be needed here. ### Additional Information Nothing to add.

fix: Reenable support for tabbing between fields.
2d2af66

gonfunko requested a review from a team as a code owner May 14, 2025 18:05

gonfunko requested a review from rachel-fenichel May 14, 2025 18:05

github-actionsBot assigned rachel-fenichel May 14, 2025

github-actionsBot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels May 14, 2025

rachel-fenichel reviewed May 14, 2025
View reviewed changes

Comment threadcore/field_input.ts Outdated

rachel-fenichel approved these changes May 14, 2025
View reviewed changes

refactor: Reduce code duplication.
78ae843

gonfunko merged commit 7a7fad4 into RaspberryPiFoundation:rc/v12.0.0May 14, 2025

gonfunko deleted the field-tab branch May 14, 2025 18:23

BenHenning reviewed May 14, 2025
View reviewed changes

BenHenning mentioned this pull request May 15, 2025
Shift+Tab after keyboard navigating to a block doesn’t open flyout as expected #8965
Closed
1 task

BenHenning mentioned this pull request May 28, 2025
feat: add keyboard navigation controller #8924
Merged
1 task

BenHenning mentioned this pull request Jun 26, 2025
fix: Ensure focus changes when tabbing fields #9173
Merged
1 task

fix: Reenable support for tabbing between fields.
2d2af66

gonfunko requested a review from a team as a code owner May 14, 2025 18:05

gonfunko requested a review from rachel-fenichel May 14, 2025 18:05

github-actionsBot assigned rachel-fenichel May 14, 2025

github-actionsBot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels May 14, 2025

rachel-fenichel reviewed May 14, 2025
View reviewed changes

Comment threadcore/field_input.ts Outdated

rachel-fenichel approved these changes May 14, 2025
View reviewed changes

refactor: Reduce code duplication.
78ae843

gonfunko merged commit 7a7fad4 into RaspberryPiFoundation:rc/v12.0.0May 14, 2025

gonfunko deleted the field-tab branch May 14, 2025 18:23

BenHenning reviewed May 14, 2025
View reviewed changes

BenHenning mentioned this pull request May 15, 2025
Shift+Tab after keyboard navigating to a block doesn’t open flyout as expected #8965
Closed
1 task

BenHenning mentioned this pull request May 28, 2025
feat: add keyboard navigation controller #8924
Merged
1 task

BenHenning mentioned this pull request Jun 26, 2025
fix: Ensure focus changes when tabbing fields #9173
Merged
1 task

fix: Reenable support for tabbing between fields.
2d2af66

gonfunko requested a review from a team as a code owner May 14, 2025 18:05

gonfunko requested a review from rachel-fenichel May 14, 2025 18:05

github-actionsBot assigned rachel-fenichel May 14, 2025

github-actionsBot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels May 14, 2025

rachel-fenichel reviewed May 14, 2025
View reviewed changes

Comment threadcore/field_input.ts Outdated

rachel-fenichel approved these changes May 14, 2025
View reviewed changes

refactor: Reduce code duplication.
78ae843

gonfunko merged commit 7a7fad4 into RaspberryPiFoundation:rc/v12.0.0May 14, 2025

gonfunko deleted the field-tab branch May 14, 2025 18:23

BenHenning reviewed May 14, 2025
View reviewed changes

BenHenning mentioned this pull request May 15, 2025
Shift+Tab after keyboard navigating to a block doesn’t open flyout as expected #8965
Closed
1 task

BenHenning mentioned this pull request May 28, 2025
feat: add keyboard navigation controller #8924
Merged
1 task

BenHenning mentioned this pull request Jun 26, 2025
fix: Ensure focus changes when tabbing fields #9173
Merged
1 task

fix: Reenable support for tabbing between fields.
2d2af66

gonfunko requested a review from a team as a code owner May 14, 2025 18:05

gonfunko requested a review from rachel-fenichel May 14, 2025 18:05

github-actionsBot assigned rachel-fenichel May 14, 2025

github-actionsBot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels May 14, 2025

rachel-fenichel reviewed May 14, 2025
View reviewed changes

Comment threadcore/field_input.ts Outdated

rachel-fenichel approved these changes May 14, 2025
View reviewed changes

refactor: Reduce code duplication.
78ae843

gonfunko merged commit 7a7fad4 into RaspberryPiFoundation:rc/v12.0.0May 14, 2025

gonfunko deleted the field-tab branch May 14, 2025 18:23

BenHenning reviewed May 14, 2025
View reviewed changes

BenHenning mentioned this pull request May 15, 2025
Shift+Tab after keyboard navigating to a block doesn’t open flyout as expected #8965
Closed
1 task

BenHenning mentioned this pull request May 28, 2025
feat: add keyboard navigation controller #8924
Merged
1 task

BenHenning mentioned this pull request Jun 26, 2025
fix: Ensure focus changes when tabbing fields #9173
Merged
1 task

fix: Reenable support for tabbing between fields.
2d2af66

gonfunko requested a review from a team as a code owner May 14, 2025 18:05

gonfunko requested a review from rachel-fenichel May 14, 2025 18:05

github-actionsBot assigned rachel-fenichel May 14, 2025

github-actionsBot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels May 14, 2025

rachel-fenichel reviewed May 14, 2025
View reviewed changes

Comment threadcore/field_input.ts Outdated

rachel-fenichel approved these changes May 14, 2025
View reviewed changes

refactor: Reduce code duplication.
78ae843

gonfunko merged commit 7a7fad4 into RaspberryPiFoundation:rc/v12.0.0May 14, 2025

gonfunko deleted the field-tab branch May 14, 2025 18:23

BenHenning reviewed May 14, 2025
View reviewed changes

BenHenning mentioned this pull request May 15, 2025
Shift+Tab after keyboard navigating to a block doesn’t open flyout as expected #8965
Closed
1 task

BenHenning mentioned this pull request May 28, 2025
feat: add keyboard navigation controller #8924
Merged
1 task

BenHenning mentioned this pull request Jun 26, 2025
fix: Ensure focus changes when tabbing fields #9173
Merged
1 task

fix: Reenable support for tabbing between fields.
2d2af66

gonfunko requested a review from a team as a code owner May 14, 2025 18:05

gonfunko requested a review from rachel-fenichel May 14, 2025 18:05

github-actionsBot assigned rachel-fenichel May 14, 2025

github-actionsBot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels May 14, 2025

rachel-fenichel reviewed May 14, 2025
View reviewed changes

Comment threadcore/field_input.ts Outdated

rachel-fenichel approved these changes May 14, 2025
View reviewed changes

refactor: Reduce code duplication.
78ae843

gonfunko merged commit 7a7fad4 into RaspberryPiFoundation:rc/v12.0.0May 14, 2025

gonfunko deleted the field-tab branch May 14, 2025 18:23

BenHenning reviewed May 14, 2025
View reviewed changes

BenHenning mentioned this pull request May 15, 2025
Shift+Tab after keyboard navigating to a block doesn’t open flyout as expected #8965
Closed
1 task

BenHenning mentioned this pull request May 28, 2025
feat: add keyboard navigation controller #8924
Merged
1 task

BenHenning mentioned this pull request Jun 26, 2025
fix: Ensure focus changes when tabbing fields #9173
Merged
1 task

fix: Reenable support for tabbing between fields.
2d2af66

gonfunko requested a review from a team as a code owner May 14, 2025 18:05

gonfunko requested a review from rachel-fenichel May 14, 2025 18:05

github-actionsBot assigned rachel-fenichel May 14, 2025

github-actionsBot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels May 14, 2025

rachel-fenichel reviewed May 14, 2025
View reviewed changes

Comment threadcore/field_input.ts Outdated

rachel-fenichel approved these changes May 14, 2025
View reviewed changes

refactor: Reduce code duplication.
78ae843

gonfunko merged commit 7a7fad4 into RaspberryPiFoundation:rc/v12.0.0May 14, 2025

gonfunko deleted the field-tab branch May 14, 2025 18:23

BenHenning reviewed May 14, 2025
View reviewed changes

BenHenning mentioned this pull request May 15, 2025
Shift+Tab after keyboard navigating to a block doesn’t open flyout as expected #8965
Closed
1 task

BenHenning mentioned this pull request May 28, 2025
feat: add keyboard navigation controller #8924
Merged
1 task

BenHenning mentioned this pull request Jun 26, 2025
fix: Ensure focus changes when tabbing fields #9173
Merged
1 task

fix: Reenable support for tabbing between fields.
2d2af66

gonfunko requested a review from a team as a code owner May 14, 2025 18:05

gonfunko requested a review from rachel-fenichel May 14, 2025 18:05

github-actionsBot assigned rachel-fenichel May 14, 2025

github-actionsBot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels May 14, 2025

rachel-fenichel reviewed May 14, 2025
View reviewed changes

Comment threadcore/field_input.ts Outdated

rachel-fenichel approved these changes May 14, 2025
View reviewed changes

refactor: Reduce code duplication.
78ae843

gonfunko merged commit 7a7fad4 into RaspberryPiFoundation:rc/v12.0.0May 14, 2025

gonfunko deleted the field-tab branch May 14, 2025 18:23

BenHenning reviewed May 14, 2025
View reviewed changes

BenHenning mentioned this pull request May 15, 2025
Shift+Tab after keyboard navigating to a block doesn’t open flyout as expected #8965
Closed
1 task

BenHenning mentioned this pull request May 28, 2025
feat: add keyboard navigation controller #8924
Merged
1 task

BenHenning mentioned this pull request Jun 26, 2025
fix: Ensure focus changes when tabbing fields #9173
Merged
1 task

Uh oh!

Conversation

gonfunko commented May 14, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The basics

The details

Resolves

Proposed Changes

Uh oh!

Uh oh!

rachel-fenichel commented May 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

gonfunko commented May 14, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The basics

The details

Resolves

Proposed Changes

Uh oh!

Uh oh!

rachel-fenichel commented May 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

gonfunko commented May 14, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The basics

The details

Resolves

Proposed Changes

Uh oh!

Uh oh!

rachel-fenichel commented May 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

gonfunko commented May 14, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The basics

The details

Resolves

Proposed Changes

gonfunko commented May 14, 2025•
edited
Loading

gonfunko commented May 14, 2025•
edited
Loading

gonfunko commented May 14, 2025•
edited
Loading

gonfunko commented May 14, 2025•
edited
Loading

gonfunko commented May 14, 2025•
edited
Loading