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

Look into overloaded rendered field on block #4288

Closed
alschmiedt opened this issue Sep 17, 2020 · 2 comments
Closed

Look into overloaded rendered field on block #4288

alschmiedt opened this issue Sep 17, 2020 · 2 comments
Assignees
Labels
component: rendering issue: feature request Describes a new feature and why it should be added

Comments

@alschmiedt
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Creating a field that forces a re render in the initModel method causes an error when mutating the block. (See example in #3458). This specific instance is fixed, but it is still plausible that someone could create a field that forces a re render which would cause this problem. To recreate Set this.doValueUpdate_ back to setValue.

The reason is that when we mutate the block, we are setting the rendered property on the block to false in an attempt to hold off rendering until the block has been fully created (not doing so will cause #4175). However, setting rendered = false causes the fields init methods to not get called.

When the block is finally ready to be rendered it will go through and call the init methods on all the fields. Since the init methods have not been called before when we reach a field that forces a re render of the entire block(in the example in #4175 this is the field_variable) the fields that have not yet been initialized will throw an error.

Describe the solution you'd like
All this points to our blocks rendered property being overloaded as described in this comment and at the bottom of this comment. By setting some kind of flag for when we are making batch changes to a block we can avoid this issue. However, there are quite a few other issues that are around this that we should look into before deciding.

Describe alternatives you've considered

Additional context

@alschmiedt alschmiedt added issue: feature request Describes a new feature and why it should be added issue: triage Issues awaiting triage by a Blockly team member and removed issue: triage Issues awaiting triage by a Blockly team member labels Sep 17, 2020
@alschmiedt alschmiedt added this to the 2020_q4_release milestone Sep 17, 2020
@BeksOmega
Copy link
Collaborator

Thanks for the issue! I think this is a good summary :D #1676 Also discusses some possible meanings / uses of rendered that might be useful for deciding how to approach this.

@BeksOmega
Copy link
Collaborator

After v11 we will use rendered to check whether a block is a Block instance or a BlockSvg instance. initialized will be used to check if initSvg or (inclusive) initModel has been called.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: rendering issue: feature request Describes a new feature and why it should be added
Projects
None yet
Development

No branches or pull requests

2 participants