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

use bootstrap modal in file edit on discard #914

Merged
merged 2 commits into from
Jun 10, 2014

Conversation

eugenk
Copy link
Member

@eugenk eugenk commented Jun 3, 2014

fixes #828

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling c7804e9 on 828-use_bootstrap_modal_in_file_edit into cff4154 on staging.

@@ -41,7 +44,10 @@ $ ->
false

editorDiscard = (e) ->
discard() if editor.doc.isClean() or confirmDiscard()
if editor.doc.isClean()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally think it would be cleaner to extract the bootstrap-modal functionality into the discard-function. For example
like this:

diff --git a/app/assets/javascripts/codemirror-custom.js.coffee b/app/assets/javascripts/codemirror-custom.js.coffee
index abe791e..7a30142 100644
--- a/app/assets/javascripts/codemirror-custom.js.coffee
+++ b/app/assets/javascripts/codemirror-custom.js.coffee
@@ -44,10 +44,7 @@ $ ->
       false

     editorDiscard = (e) ->
-      if editor.doc.isClean()
-        discard()
-      else
-        discard_modal.modal('show')
+      discard(editor.doc.isClean())
       false

     requireCommitMessage = (e) ->
@@ -58,18 +55,21 @@ $ ->
       else
         true

-    discard = ->
-      message_textarea.value = ""
-      editor.setOption "readOnly", true
-      editor.setValue original_editor_content
-      message_group.removeClass hasErrorClass
-      alert_error.hide()
-      $(".show-when-editing").hide()
-      $(".hide-when-editing").show()
-      editorPreventFocus()
-      editor.off "focus", editorSetFocus
-      editor.off "blur", editorSetBlur
-      discard_modal.modal('hide')
+    discard = (use_modal) ->
+      if use_modal == false
+        discard_modal.modal('show')
+      else
+        message_textarea.value = ""
+        editor.setOption "readOnly", true
+        editor.setValue original_editor_content
+        message_group.removeClass hasErrorClass
+        alert_error.hide()
+        $(".show-when-editing").hide()
+        $(".hide-when-editing").show()
+        editorPreventFocus()
+        editor.off "focus", editorSetFocus
+        editor.off "blur", editorSetBlur
+        discard_modal.modal('hide')
       return

     enableEditing = (e) ->

@coveralls
Copy link

Coverage Status

Coverage increased (+0.11%) when pulling 7d7136a on 828-use_bootstrap_modal_in_file_edit into cff4154 on staging.

@0robustus1
Copy link
Contributor

👍

eugenk added a commit that referenced this pull request Jun 10, 2014
…edit

use bootstrap modal in file edit on discard
@eugenk eugenk merged commit 9fd8345 into staging Jun 10, 2014
@eugenk eugenk deleted the 828-use_bootstrap_modal_in_file_edit branch June 10, 2014 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

use bootstrap modal in file edit
3 participants