Fix MakeCode Editor Service Ready State #9837
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…en we change the iframe url. The ref can be updated anytime and doesn't necessarily mean the content of the iframe has changed or refreshed, so we can't simply unset the ready flag whenever that updates (the app has already sent its editorcontentloaded event, so it won't send another to unblock us). Instead, we need to preserve our ready flag unless the actual contents of the iframe changes. An alternative approach would be to send a "ping" any time we try to send a message but we're not ready, and switch to ready if the ping is successful, but that feels like it may be unnecessary at this stage.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes a bug where you couldn't run eval multiple times. (In fact, messaging to the iframe was totally breaking down.)
Before this change, the
makecodeEditorServiceunset itsreadyForMessagesflag anytime the iframe ref was updated. This was done under the assumption that the ref updating also means the iframe would reload, which turns out to be untrue. As a result, we end up settingreadyForMessagestofalseand get stuck because the editor inside the iframe does not send itseditorcontentloadedevent again, since it already did so a while ago.I basically took the simplest approach to fixing this, which is to make the
readyForMessagesreset an intentional action, and only call that when we change the iframe url. It seems to work in my testing and I'm struggling to find a scenario where we'd get into a bad state again, but it's possible I'm missing something.An alternative approach would be to send an "are you ready?" ping any time we want to send a message but
readyForMessagesis false, then switch to ready and clear the queue if the editor responds to that ping successfully (as we do currently when we get aneditorcontentloadedevent)...but that feels like it may be unnecessary at this stage.I've also included a few UI touch-ups for when you are running eval multiple times and/or changing the rubric after eval has run.
Upload target: https://makecode.microbit.org/app/f3f1b4e65ff5b1c8a233beb71cf0024757ea5b4a-aff29c0756