microsoft / pxt-microbit

A Blocks / JavaScript code editor for the micro:bit built on Microsoft MakeCode
https://makecode.microbit.org
Other
708 stars 588 forks source link

iframe/controller regression in beta vs live when updating fixedInstances #5896

Closed microbit-robert closed 1 week ago

microbit-robert commented 1 week ago

Describe the bug When using the importproject iframe message to update an embedded MakeCode project, changes to fixedInstances (e.g., updating the fixedInstance name) aren't propagated/recognised in the block dropdowns. If the import updates main.blocks to use a modified fixedInstance, the following warning is observed in the console Cannot set the dropdown's value to an unavailable option. Block type: *block_type*, Field name: this, Value: *new_fixedInstance_value*. The block dropdown then defaults to the first available value. The user can subsequently select the updated fixedInstance value from the dropdown when interacting with the block.

The first import of a project that uses fixedInstances in block dropdowns works as expected. It is only subsequent updates that seem to fail in the manner described above.

This bug is only present in the beta version of the MakeCode editor.

We currently use this API in micro:bit classroom and will also be using it in the micro:bit machine learning tool. The latter will use the pxt-microbit-ml extension which utilizes fixedInstances which is where we first noticed this issue.

CC @microbit-matt-hillsdon

To Reproduce Steps to reproduce the behavior:

  1. Unzip controller.zip and find controller-beta.html and controller-live.html. These are edited from controller.html in the pxt repo and allow for the easy importing of two projects that use fixedInstances (a cutdown version of the fixed instances example in the MakeCode playground tool). controller-beta.html is using the beta version of the editor, controller-live.html is using the live version.
  2. Open or serve controller-beta.html
  3. Click on the "import project 1" button and observe that the project successfully updates
  4. Click on the "import project 2" button and observe the block dropdown defaulting to "D0" instead of "D12". The warning described above will also be present in the console.
  5. Open or serve controller-live.html
  6. Repeat steps 3 and 4 above, and observe the block dropdown now successfully updates to "D12"

Expected behavior The correct behaviour noted in step 6 of the reproduction above should also be maintained in the beta version of the editor.

Screenshots Not relevant.

micro:bit version (please complete the following information): Not relevant.

Desktop (please complete the following information):

Smartphone (please complete the following information): Not relevant.

Additional context Add any other context about the problem here.

abchatra commented 1 week ago

@riknoll this will break classroom, so likely ship blocking. Lets fix this before shipping the payload.

microbit-carlos commented 1 week ago

@abchatra will this be a blocker for the release? Do you think it will still ship on schedule or might be delayed?

abchatra commented 1 week ago

This is blocker in my opinion. Unless you folks think otherwise we will hold on the release.

microbit-matt-hillsdon commented 1 week ago

We've continued to investigate the impact of this issue.

We found that we can trigger the bug by making edits in MakeCode (not in an iframe) that correspond to the differences in the two projects we provided. So it's possible to trigger without the iframe/controller mode.

Even though the ML tool and micro:bit classroom both update projects in a MakeCode iframe via importproject we've found we can't trigger the issue in micro:bit classroom.

It turns out that the difference is that in micro:bit classroom the project always has a header field. In ML tool the project only has a text field (like the simple example in controller.html). We can arrange for a header in the ML tool scenario and work around the behaviour change in that way. It's not clear to us why the header makes a difference or whether it's a reliable approach.

As this issue seems to involve some out of sync state/blocks definitions I'm not sure if it could have other impacts than the one we've noticed, so it would be good to investigate it. But I think from a practical point of view it doesn't seem to be an issue for micro:bit classroom or ML tool (where the MakeCode integration is unreleased) if we switch to use a header.

For future use, you test in micro:bit classroom with https://classroom.microbit.org?editorVersion=beta (but as mentioned we can't reproduce it there).