Use registry for creating the no-category flyout #4659
Conversation
| throw Error('Missing require for Blockly.HorizontalFlyout'); | ||
| } | ||
| this.flyout_ = new Blockly.HorizontalFlyout(workspaceOptions); | ||
| var HorizontalFlyout = Blockly.registry.getClassFromOptions( |
There was a problem hiding this comment.
This no longer throws a clear error for the missing require. Are we okay with that?
There was a problem hiding this comment.
That’s true, so it’s not exactly the right check to check if Blockly.HorizontalFlyout exists any more since it can be some other object that is registered, but we can check if getClassFromOptions returns nothing and throw.
I would rather deal with this in the registry. It is currently logging a warning. This sounds like something that applies anywhere we request a class from the registry (the whole warn vs throw dilemma). If we want to throw or warn on a case by case basis then we can add another param to the registry call to request a throw if it yields nothing. I would just rather not have to always check if exists and throw.
There was a problem hiding this comment.
I like having it optionally throw on failure. At the call site we often know whether a failure would be catastrophic--like here--and a centralized throw would be way better.
There was a problem hiding this comment.
Ok I’ll add that functionality to the registry, and make it throw from this call site
The basics
The details
Resolves
Fixes 6 strict missing requires errors.
Proposed Changes
Use the registry for creating the vertical / horizontal flyout on a no-category workspace.
Behavior Before Change
Behavior After Change
Reason for Changes
Test Coverage
Documentation
Additional Information
@rachel-fenichel FYI