[Finishes #99188090] Examples on close#3501
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
maybe short note here about what params it takes / what it returns?
4422c31 to
399baae
Compare
Contributor
There was a problem hiding this comment.
Wondering what might be a good name for this to make its behavior a little more clear.
The callback kind of fills two roles, (1) return true/false if it should subsume the closing responsibility, (2) execute custom logic on a failure's close.
One option would be to, s/this.appsHijackedDialogClose_/this.customFailureCloseHandler_, and ifthis.customFailureCloseHandler_` is null, close normally on fail. Then the existence handles responsibility #1, and the callback has one less job.
399baae to
bc376ab
Compare
Contributor
There was a problem hiding this comment.
Is there a reason this isn't this.hideIfOpen()?
Contributor
Author
There was a problem hiding this comment.
Heh. Probably not. I think my first approach had me sending params to hideIfOpen.
Contributor
|
LGTM overall, just a few nits. I like the flipping of example evaluation result string meaning. |
b2dfc2c to
99c5650
Compare
Bjvanminnen
added a commit
that referenced
this pull request
Aug 12, 2015
[Finishes #99188090] Examples on close
deploy-code-org
added a commit
that referenced
this pull request
Aug 12, 2015
commit 836a717 Merge: 1788d65 0cde9b9 Author: Bjvanminnen <Bjvanminnen@gmail.com> Date: Wed Aug 12 15:06:04 2015 -0700 Merge pull request #3252 from code-dot-org/hackedLevels custom type for play lab ninja cat commit 1788d65 Merge: 65b860e 7f669b3 Author: Brian Jordan <bcjordan@gmail.com> Date: Wed Aug 12 15:04:13 2015 -0700 Merge pull request #3512 from code-dot-org/fix_example_buttons_ie11 [Finishes #101164984] Fix pressing example add/run buttons on IE11. commit 0cde9b9 Author: Brent Van Minnen <bjvanminnen@gmail.com> Date: Tue Jul 28 10:07:44 2015 -0700 custom type for play lab ninja cat commit 7f669b3 Author: Brian Jordan <bcjordan@gmail.com> Date: Wed Aug 12 15:03:42 2015 -0700 [Finishes #101164984] Fix example add/run buttons on IE11. commit 65b860e Merge: fef04cd 99c5650 Author: Bjvanminnen <Bjvanminnen@gmail.com> Date: Wed Aug 12 15:00:49 2015 -0700 Merge pull request #3501 from code-dot-org/examplesOnClose [Finishes #99188090] Examples on close commit fef04cd Merge: 5fce434 550c909 Author: philbogle <phil@code.org> Date: Wed Aug 12 14:59:45 2015 -0700 Merge pull request #3511 from code-dot-org/revert-3492-netsim-viz-performance-cleanup Revert "NetSim: Viz performance cleanup" commit 550c909 Author: philbogle <phil@code.org> Date: Wed Aug 12 14:59:01 2015 -0700 Revert "NetSim: Viz performance cleanup" commit 5fce434 Merge: 6677a1d e6d07ba Author: philbogle <phil@code.org> Date: Wed Aug 12 14:57:14 2015 -0700 Merge pull request #3492 from code-dot-org/netsim-viz-performance-cleanup NetSim: Viz performance cleanup commit 6677a1d Author: Continuous Integration <dev@code.org> Date: Wed Aug 12 21:47:41 2015 +0000 Automatically built. commit a4aefe4 Merge: 3848999 89a2f53 Author: Brian Jordan <bcjordan@gmail.com> Date: Wed Aug 12 14:33:20 2015 -0700 Merge pull request #3508 from code-dot-org/ignore_clicks_drawn_ui Ignore clicks on SVG-drawn UI elements to allow panning through them. commit 89a2f53 Author: Brian Jordan <bcjordan@gmail.com> Date: Wed Aug 12 14:31:06 2015 -0700 Rename svgIgnoreMouse [ci skip] commit 96647d1 Author: Brian Jordan <bcjordan@gmail.com> Date: Wed Aug 12 14:20:51 2015 -0700 Ignore clicks on SVG-drawn UI elements to allow panning through them. - Rename callText to callResult text to be more accurate (result also uses it) - Add helper for adding pointer events for SVG elements commit 99c5650 Author: Brent Van Minnen <bjvanminnen@gmail.com> Date: Wed Aug 12 14:37:17 2015 -0700 finish bad rebase commit 3bf61c6 Merge: a4aefe4 0300263 Author: Tanya Parker <tanya@code.org> Date: Wed Aug 12 14:36:46 2015 -0700 Merge pull request #3509 from code-dot-org/strayunderscore no text decoration on buttons commit 9d6c0bc Author: Brent Van Minnen <bjvanminnen@gmail.com> Date: Wed Aug 12 14:34:21 2015 -0700 fix UI test commit 56c2f28 Author: Brent Van Minnen <bjvanminnen@gmail.com> Date: Wed Aug 12 13:33:17 2015 -0700 code review feedback commit bc16af5 Author: Brent Van Minnen <bjvanminnen@gmail.com> Date: Wed Aug 12 12:49:14 2015 -0700 consolidate some common code
joshlory
pushed a commit
that referenced
this pull request
Aug 13, 2015
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.
Run examples on close, and alert if any are wrong.
This dialog is pretty similar to the confirm dialog we give you when you click start over, so I made a single showSimpleDialog that is used for both.
I also did some bug fixing with how examples on run were working in Eval.
I also did some refactoring of how examples work in general. The biggest change was to make it so that our test example functions return either an error string or null on success. This allows us to consolidate our success string to be in a single place (in blockly-core).