Add Blockly.readOnly check to function editor - #4459
Merged
Conversation
The function editor attempts to give itself left padding based on the existence of either the toolbox or the flyout, neither of which exist in readOnly mode, causing the whole thing to crash.
Contributor
There was a problem hiding this comment.
Brent added a similar check here yesterday https://github.com/code-dot-org/code-dot-org/pull/4395/files — instead of checking for readOnly here, it might make sense to check for isFlyout, in which case we can also probably drop the explanation comment
Contributor
Author
There was a problem hiding this comment.
so,
var left = Blockly.hasCategories ?
goog.dom.getElementByClass('blocklyToolboxDiv').getBoundingClientRect().width :
Blockly.isFlyout ? goog.dom.getElementByClass('blocklyFlyoutBackground').getBoundingClientRect().width : 0;Is that really clearer?
Contributor
There was a problem hiding this comment.
Oops, sorry I missed this response! Reads a tiny bit clearer, more importantly the point of truth we care about from this context is "is there a flyout", versus "is this read only [implicitly that means there is not a flyout, so ignore its size]"
Could also imagine a helper e.g. Blockly.divToMainBlockspaceDistance that encapsulates this.
We can always change later on if we end up revisiting something related to this
Contributor
Author
There was a problem hiding this comment.
Ahh, I see what you mean. I'll make a note to clean this up.
Contributor
|
LGTM! Just one suggestion |
Hamms
added a commit
that referenced
this pull request
Oct 14, 2015
Add Blockly.readOnly check to function editor
deploy-code-org
added a commit
that referenced
this pull request
Oct 14, 2015
commit 9764e23 Merge: b37971a 4842714 Author: Elijah Hamovitz <elijahhamovitz@gmail.com> Date: Tue Oct 13 18:00:05 2015 -0700 Merge pull request #4459 from code-dot-org/readonly-function-editor Add Blockly.readOnly check to function editor commit b37971a Author: Josh Lory <josh.lory@code.org> Date: Tue Oct 13 17:46:30 2015 -0700 Cucumber doesn't like inline comments
deploy-code-org
added a commit
that referenced
this pull request
Oct 14, 2015
commit 91bd150 Merge: 3acdc4f 1db88fa Author: Trevor Berg <trevor@code.org> Date: Tue Oct 13 21:10:06 2015 -0700 Merge pull request #4531 from code-dot-org/fix-debug-icon Get rid of hacks for debugger hide/show state commit 3acdc4f Merge: 867ec7f 1c7c553 Author: Trevor Berg <trevor@code.org> Date: Tue Oct 13 21:08:46 2015 -0700 Merge pull request #4548 from code-dot-org/xframe Rewrite documentation urls to forward on the embedded flag commit 867ec7f Author: Continuous Integration <dev@code.org> Date: Wed Oct 14 01:05:32 2015 +0000 Automatically built. commit 9764e23 Merge: b37971a 4842714 Author: Elijah Hamovitz <elijahhamovitz@gmail.com> Date: Tue Oct 13 18:00:05 2015 -0700 Merge pull request #4459 from code-dot-org/readonly-function-editor Add Blockly.readOnly check to function editor commit b37971a Author: Josh Lory <josh.lory@code.org> Date: Tue Oct 13 17:46:30 2015 -0700 Cucumber doesn't like inline comments
deploy-code-org
added a commit
that referenced
this pull request
Oct 14, 2015
commit d1b8e20 Merge: 30a0c41 6be1903 Author: Trevor Berg <trevor@code.org> Date: Tue Oct 13 21:25:20 2015 -0700 Merge pull request #4533 from code-dot-org/autocomplete-tooltips-rebase Add the ability to not show droplet tooltips on a per level basis commit 30a0c41 Merge: 60ed967 04ac1cd Author: davidsbailey <davidsbailey@users.noreply.github.com> Date: Tue Oct 13 21:24:41 2015 -0700 Merge pull request #4547 from code-dot-org/fix-id-dropdowns Fix id dropdowns commit 60ed967 Author: Continuous Integration <dev@code.org> Date: Wed Oct 14 04:19:55 2015 +0000 Automatically built. commit 91bd150 Merge: 3acdc4f 1db88fa Author: Trevor Berg <trevor@code.org> Date: Tue Oct 13 21:10:06 2015 -0700 Merge pull request #4531 from code-dot-org/fix-debug-icon Get rid of hacks for debugger hide/show state commit 3acdc4f Merge: 867ec7f 1c7c553 Author: Trevor Berg <trevor@code.org> Date: Tue Oct 13 21:08:46 2015 -0700 Merge pull request #4548 from code-dot-org/xframe Rewrite documentation urls to forward on the embedded flag commit 867ec7f Author: Continuous Integration <dev@code.org> Date: Wed Oct 14 01:05:32 2015 +0000 Automatically built. commit 9764e23 Merge: b37971a 4842714 Author: Elijah Hamovitz <elijahhamovitz@gmail.com> Date: Tue Oct 13 18:00:05 2015 -0700 Merge pull request #4459 from code-dot-org/readonly-function-editor Add Blockly.readOnly check to function editor commit b37971a Author: Josh Lory <josh.lory@code.org> Date: Tue Oct 13 17:46:30 2015 -0700 Cucumber doesn't like inline comments commit 91bd150 Merge: 3acdc4f 1db88fa Author: Trevor Berg <trevor@code.org> Date: Tue Oct 13 21:10:06 2015 -0700 Merge pull request #4531 from code-dot-org/fix-debug-icon Get rid of hacks for debugger hide/show state commit 3acdc4f Merge: 867ec7f 1c7c553 Author: Trevor Berg <trevor@code.org> Date: Tue Oct 13 21:08:46 2015 -0700 Merge pull request #4548 from code-dot-org/xframe Rewrite documentation urls to forward on the embedded flag
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The function editor attempts to give itself left padding based on the
existence of either the toolbox or the flyout, neither of which exist in
readOnly mode, causing the whole thing to crash.