Skip to content

JS Editor: add missing block coverage (codegen + runtime + JS→Blocks)#5834

Merged
walterbender merged 1 commit intosugarlabs:masterfrom
zealot-zew:js-editor-blocks-coverage
Mar 14, 2026
Merged

JS Editor: add missing block coverage (codegen + runtime + JS→Blocks)#5834
walterbender merged 1 commit intosugarlabs:masterfrom
zealot-zew:js-editor-blocks-coverage

Conversation

@zealot-zew
Copy link
Contributor

@zealot-zew zealot-zew commented Feb 20, 2026

This PR completes missing block coverage in the JS Editor by adding support in both directions:

  • Blocks → JS: added missing block mappings and runtime API methods
  • JS → Blocks: added AST conversion rules for the same blocks (where applicable)
  • Safely ignores comment blocks during JS generation to avoid crashes

This ensures round-trip compatibility between Blocks and the JS Editor for the added blocks.

Blocks Added / Updated

1. wait

  • Blocks → JS: waitdoWait(seconds)
  • Runtime: GraphicsBlocksAPI.doWait(seconds)
  • JS → Blocks: await mouse.doWait(n)wait

2. erasemedia

  • Blocks → JS: erasemediaclearMedia()
  • Runtime: GraphicsBlocksAPI.clearMedia()
  • JS → Blocks: await mouse.clearMedia()erasemedia

3. beginfill

  • Blocks → JS: beginfillbeginFill()
  • Runtime: PenBlocksAPI.beginFill()
  • JS → Blocks: await mouse.beginFill()beginfill

4. endfill

  • Blocks → JS: endfillendFill()
  • Runtime: PenBlocksAPI.endFill()
  • JS → Blocks: await mouse.endFill()endfill

5. fill (clamp block)

  • Blocks → JS: fillfillShape(flow)
  • Clamp support: inner blocks emitted as an async callback
  • Runtime: already existed
  • JS → Blocks: await mouse.fillShape(async () => { ... })fill

6. hollowline (clamp block)

  • Blocks → JS: hollowlinehollowLine(flow)
  • Clamp support: added
  • Runtime: already existed
  • JS → Blocks: await mouse.hollowLine(async () => { ... })hollowline

7. definemode (clamp block)

  • Blocks → JS: definemodedefineMode(name, flow)
  • Clamp support: already present
  • Runtime: already existed
  • JS → Blocks: await mouse.defineMode("name", async () => { ... })definemode

8. movable

  • Blocks → JS: setter mapping → MOVABLEDO
  • Runtime: already existed (Mouse.MOVABLEDO)
  • JS → Blocks: mouse.MOVABLEDO = true/falsemovable

9. comment

  • Blocks → JS: ignored (no-op)
  • Reason: comments have no runtime effect and don’t exist in the AST
  • Impact: prevents JS generator crashes

Verified In the JS Editor.
image

JS Editor Architeture for Reference:
image

This PR extends the JS Editor in both directions:

Blocks → JS: adds block mappings and missing runtime methods.
JS → Blocks: adds AST conversion rules for the same blocks (where applicable).
@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@zealot-zew
Copy link
Contributor Author

@walterbender @omsuneri can you please review if you get time, also recommend me important blocks to be added to the editor which are missing.

here are the missing blocks list
https://docs.google.com/document/d/1IfFZksLIQqTQeF40G45nUC2eVfqdLVPsJEk-jnNIC2g/edit?tab=t.0

Copy link

@shashank03-dev shashank03-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey, I have a few questions regarding this. I noticed that fillShape(flow) calls doStartFill and awaits the flow(), but it doesn't seem to call an end fill command. Should there be something known as this.runCommand("doEndFill"); after the flow execution to close the shape properly? and did you add specific unit test strings for these new blocks to lock in the coverage, or does the current test setup catch them automatically?

@zealot-zew
Copy link
Contributor Author

@shashank03-dev Thanks for the review, There were no testcases for JS Editor ( i saw a PR open but not sure if it got merged). and i think the flow was never touched in this PR because it already existed before. so it should work.

@shashank03-dev
Copy link

shashank03-dev commented Feb 23, 2026

Thanks for the context, @zealot-zew! I totally understand that this was existing code.

To make fillShape consistent and more robust. Adding a doEndFill call within a try...finally block would ensure the shape closes properly even if the inner flow() throws an error

async fillShape(flow) {
    await this.runCommand("doStartFill");
    try {
        await flow();
    } finally {
        await this.runCommand("doEndFill");
    }
}

we could even do this ,this would be more easy for runtime

@zealot-zew
Copy link
Contributor Author

@walterbender I have updated the Missing blocks list to make sure AI slop blocks are not there. Can you check some them so that i can proceed with adding them.

https://docs.google.com/document/d/1IfFZksLIQqTQeF40G45nUC2eVfqdLVPsJEk-jnNIC2g/edit?tab=t.0

@walterbender walterbender merged commit c0730e1 into sugarlabs:master Mar 14, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants