google / blockly

The web-based visual programming editor.
https://developers.google.com/blockly/
Apache License 2.0
12.51k stars 3.72k forks source link

fix: Simplify list and text WHERE validation #8575

Closed johnnesky closed 2 months ago

johnnesky commented 2 months ago

The basics

The details

Resolves

Fixes https://github.com/google/blockly/issues/8556

Proposed Changes

I modified the following block type definitions:

Each of these had an input called AT with an attached field called WHERE. The input and its attached field were sometimes removed and recreated in response to the field's validator. This PR instead attaches the field to a separate dummy input, such that the field does not need to be recreated every time the input is recreated.

Reason for Changes

Edits to the field did not always fire a BlockChange event. In particular, if the user is trying to change the field's value to its default value, but the field gets recreated, then the recreated field will already have the default value and thus no change is detected.

Test Coverage

All existing tests pass. Manual testing confirms that BlockChange events are fired as expected.

Additional Information

I can't prove that nobody is depending on the specific hierarchy of inputs and fields in these blocks, although I preserved the existing names of the inputs and fields. I also added new names for new dummy inputs in some cases (so that other inputs could be moved adjacent to them). Should this be considered a breaking change?

Also, there's a bug when undoing/redoing edits to the WHERE field while a child block is already attached to the AT input. Changing the field's value can replace the AT input with a dummy input, which forcibly detaches the child block, but this is not properly recorded in undo history. In particular, if you edited the field and then made additional changes, then undid all those changes and tried to redo them, the additional changes are lost. A similar bug existed before this PR, but I don't think this PR makes the bug any worse, and I suspect this PR is a prerequisite to properly fixing the bug. Possibly related to: https://github.com/google/blockly/issues/7950