fix: preserve dropdown values that are not in the current options list - #3548
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes Scratch sprite-targeting dropdowns so they preserve and display stored values even when those values aren’t present in the current runtime-generated options list (e.g., when the currently edited sprite is excluded).
Changes:
- Override
ScratchFieldDropdown.doClassValidation_to accept any string value (even if not in current options). - Override
ScratchFieldDropdown.getText_to display the raw stored value when no option matches. - Add browser regression tests covering out-of-options persistence/display and in-options “happy path” rendering.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/fields/scratch_field_dropdown.ts |
Relaxes dropdown validation and adds fallback display behavior for unmatched stored values. |
tests/browser/scratch_field_dropdown.test.ts |
Adds browser-level regression coverage for preserving/displaying values absent from the current option list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@cwillisf |
...oof. Yep, that's pretty bad! :( Looks like that overlaps with the last item over here: scratchfoundation/scratch-editor#533 I'm focusing on functionality and data loss issues first; I might not be able to get to cosmetic issues like that one until next week. Sorry! |
3e6771a to
33a2d8c
Compare
@cwillisf And for functionality maybe double click a comment to jump to a block? And maybe on the note of drop downs right now, when you right click a variable, you can switch it so maybe you can switch like a looks reporter or switch a block with the right click! I like this update, just buggy. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Scratch's sprite-targeting menus (motion_goto_menu, motion_pointtowards_menu, sensing_touchingobject_menu, and similar) are populated at runtime from the project's sprite list, with the currently editing sprite deliberately excluded. When a block is copied from another sprite into the sprite it now targets, the stored field value is absent from the option list for that editing context. Blockly's default FieldDropdown validator rejects such values, so the field silently reverts to the first option — the displayed text shows the default even though scratch-vm runs the block against the real stored value. Override doClassValidation_ so ScratchFieldDropdown accepts any string, and override getText_ so the display falls back to the raw value when no option matches. Matched-option cases still delegate to the base so image and HTMLElement option types render correctly. Fixes https://scratch.mit.edu/discuss/topic/878311/
Are you the only dev working on this? 😬 |
I’ve noticed there is different devs for each respiratory like WWW, GUI, Blocks |
I would make a PR to fix it but I have no idea if this repo uses CSS (Im not a very big person in JS and TS..) |
|
We tend to divide work by topic rather than repository. Some topics tend to be mostly in one repository, while other topics span across multiple repositories. |
Oh? That's interesting work flow. |
33a2d8c to
5314b87
Compare
|
So what would this do if I can ask. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This avoids dropdown menus being reset when you drag a block from one sprite to another. It's "light" data loss, but it's still data loss. |
Resolves
Proposed Changes
Scratch's sprite-targeting menus (
motion_goto_menu,motion_pointtowards_menu,sensing_touchingobject_menu, and similar) are populated at runtime from the project's sprite list, with the currently editing sprite deliberately excluded. When a block is copied from another sprite into the sprite it now targets, the stored field value is absent from the option list for that editing context. Blockly's defaultFieldDropdownvalidator rejects such values, so the field silently reverts to the first option — the displayed text shows the default even thoughscratch-vmruns the block against the real stored value.Override
doClassValidation_soScratchFieldDropdownaccepts any string, and overridegetText_so the display falls back to the raw value when no option matches. Matched-option cases still delegate to the base, so image andHTMLElementoption types render correctly.Test Coverage
Tests added