Error report now only appears when DropDownDiv not visible

[FurryR]

Resolves

• Fixes #53.

Proposed Changes

Error report now only appears when DropDownDiv not visible.

Reason for Changes

It allows universal custom report block or advanced DropDownDiv usage.

Test Coverage

N/A

Browser Coverage

Check the OS/browser combinations tested (At least 2)

Mac

• [ ] Chrome
• [ ] Firefox
• [ ] Safari

Windows

• [x] Chrome
• [x] Firefox
• [x] Edge

Chromebook

• [ ] Chrome

iPad

• [ ] Safari

Android Tablet

• [ ] Chrome

Note: Tests not required for this pull request

[FurryR]

Attachment:

Extension for test:

class Test {
  getInfo() {
    return {
      id: 'test',
      name: 'test',
      blocks: [
        { blockType: 'command', opcode: 'test', arguments: {}, text: 'test' }
      ]
    }
  }
  test() {
    throw new Error('test')
  }
}
Scratch.extensions.register(new Test());

[RedMan13]

how exactly does this achieve its target goal?

[RedMan13]

ok but on a more serious note, wont this prevent errors from showing if you happen to already have a dropdown open

[FurryR]

ok but on a more serious note, wont this prevent errors from showing if you happen to already have a dropdown open

already had a dropdown open is only happened when extension called Blockly.DropDownDiv before onErrorStackBlock. In other situations Blockly.DropDownDiv.isVisible() will always return false.

[RedMan13]

no that function is called when any error is reported not just an extension

[FurryR]

no that function is called when any error is reported not just an extension

if any error refers to compiler runtime error and builtin block error, it will also be fine. DropDownDiv is cleared before any block execution.

[RedMan13]

what im saying is, if you run some piece of code that takes a while then open a dropdown and that piece of code errors, you wont see it, only the error outline

[FurryR]

what im saying is, if you run some piece of code that takes a while then open a dropdown and that piece of code errors, you wont see it, only the error outline

yea, i think thats intended since you are dealing with DropDownDiv (that means ur already know how to open DevTools console) and its rare (edge) situation.

[RedMan13]

my guy you sound insain dropdowndiv is used by blockly for EVERYTHING that popsup under a block with a little arrow pointing to the block this means that any dropdowns, value reports, errors (and anything else i cant think of :trollface:) will be able to hide the error message, so no, your point isnt correct as the dropdowndiv exists for average users too also the "correct operation" that is enforced normally is to close the currently open dropdown div to show the other one, this causes a conflict with that concept

if you for somereason absolutely require the source code to be patched over and can not do anything at all somehow, then patch it over inside your own code i see no reason for this to be merged as all it adds is more possible issues and no reasonable gain

[FurryR]

@RedMan13 Please reconsider this PR.

If you read Blockly source code more carefully, you will notice that any gesture triggers Blockly.Gesture.prototype.doStart -> Blockly.hideChaff -> Blockly.hideChaffInternal_ -> Blockly.DropDownDiv.hideWithoutAnimation and hides DropDownDiv, and click block is considered as a gesture. Therefore, when onBlockStackError is called, this.ScratchBlocks.DropDownDiv.isVisible() will always return false unless there is already a DropDownDiv created by the extension synchronically.

[RedMan13]

that is not the order i stated, the order i stated was you run the script then open a dropdown, and the block wont be able to throw the error (corrected block to script as i intended to mean any code chunk that can error, not just a single block)

[FurryR]

that is not the order i stated, the order i stated was you run the script then open a dropdown, and the block wont be able to throw the error (corrected block to script as i intended to mean any code chunk that can error, not just a single block)

PenguinMod doesn't even handle asynchronous extension errors, so maybe it's fine...?