Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Show warning message in publish dialog when code contains errors.

This should fix #70.
  • Loading branch information...
commit 119fd7a802e763501c08193052aa1da31c3a6860 1 parent f12549f
Atul Varma toolness authored
Showing with 19 additions and 0 deletions.
  1. +10 −0 css/modals.css
  2. +4 −0 index.html
  3. +5 −0 js/fc/ui/modals.js
10 css/modals.css
View
@@ -184,6 +184,16 @@
color: #236819;
}
+#confirm-dialog .error-warning {
+ display: none;
+}
+
+#confirm-dialog.has-errors .error-warning {
+ display: block;
+ padding-bottom: 1em;
+ color: red;
+}
+
.confirmation-buttons {
text-align: center;
margin-top: 1em;
4 index.html
View
@@ -98,6 +98,10 @@
<span id="confirm-publication" class="confirmation-button yes-button">Yes</span>
<span id="cancel-publication" class="confirmation-button no-button">No</span>
</div>
+ <div class="error-warning">
+ <strong>Warning:</strong> Your code appears to contain
+ errors, and the published page may not appear as you intend.
+ </div>
</div>
<!-- CONFIRM PUBLICATION -->
</div>
5 js/fc/ui/modals.js
View
@@ -26,6 +26,11 @@ define(function() {
$("#publish-button").toggleClass("enabled", isEnabled);
});
+ codeMirror.on("reparse", function(event) {
+ var hasErrors = event.error ? true : false;
Mike Kamermans Owner
Pomax added a note

this looks very much like "var x = true ? true : false;" redudancy. What about "var hasErrors = !!event.error"?

Atul Varma Owner
toolness added a note

Yeah, I guess this is a matter of code style, since if one is familiar with the double-negative idiom it's understandable, but if they're not, it reads like a confusing double-negative. I don't think either is particularly readable, but I think the longer version is slightly more readable... maybe we can just leave it in there for now and discuss it in a style guide later?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ $("#confirm-dialog").toggleClass("has-errors", hasErrors);
+ });
+
$("#publish-button").click(function(){
if ($(this).hasClass("enabled")) $("#confirm-dialog").show();
});
Mike Kamermans

this looks very much like "var x = true ? true : false;" redudancy. What about "var hasErrors = !!event.error"?

Atul Varma

Yeah, I guess this is a matter of code style, since if one is familiar with the double-negative idiom it's understandable, but if they're not, it reads like a confusing double-negative. I don't think either is particularly readable, but I think the longer version is slightly more readable... maybe we can just leave it in there for now and discuss it in a style guide later?

Please sign in to comment.
Something went wrong with that request. Please try again.