Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error report now only appears when DropDownDiv not visible #55

Closed
wants to merge 1 commit into from

Conversation

FurryR
Copy link

@FurryR FurryR commented Apr 24, 2024

Resolves

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

  • Chrome
  • Firefox
  • Edge

Chromebook

  • Chrome

iPad

  • Safari

Android Tablet

  • Chrome

Note: Tests not required for this pull request

@FurryR
Copy link
Author

FurryR commented Apr 24, 2024

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
Copy link

how exactly does this achieve its target goal?

@RedMan13
Copy link

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

@FurryR
Copy link
Author

FurryR commented Apr 24, 2024

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
Copy link

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

@FurryR
Copy link
Author

FurryR commented Apr 24, 2024

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
Copy link

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
Copy link
Author

FurryR commented Apr 25, 2024

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
Copy link

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

@RedMan13 RedMan13 closed this Apr 25, 2024
@FurryR
Copy link
Author

FurryR commented Apr 26, 2024

@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
Copy link

RedMan13 commented Apr 26, 2024

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
Copy link
Author

FurryR commented Apr 27, 2024

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...?

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.

Error reportValue should check if there is already drop down content
2 participants