You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by benkeen <gi...@git.apache.org> on 2014/12/16 21:51:17 UTC

[GitHub] couchdb-fauxton pull request: [WIP] Updating edit doc page

GitHub user benkeen opened a pull request:

    https://github.com/apache/couchdb-fauxton/pull/204

    [WIP] Updating edit doc page

    Updates to the document editor page. Should be ready shortly.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/benkeen/couchdb-fauxton updating-edit-doc-page

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/couchdb-fauxton/pull/204.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #204
    
----
commit f5e3d2339f993df06ad0d8c05a5443d5ad768c33
Author: Benjamin Keen <be...@gmail.com>
Date:   2014-12-11T00:25:57Z

    This commit contains two things:
    - Before #dashboard-content had padding CSS applied directly to it.
    The problem was that that was too high up in the DOM to allow us to
    overwrite it on a page basis (needed for the new doc editor page, which
    will be flush to the edges).
    - Started on the header changes for doc editor page

commit 82ba2ba215b3e30a6c12e0df9341caa058b6931b
Author: Benjamin Keen <be...@gmail.com>
Date:   2014-12-11T01:22:24Z

    Editor now full height. Header fixed pos.

commit fa6eb6ffe75761a34470e59e9896d68d8ca16f41
Author: Benjamin Keen <be...@gmail.com>
Date:   2014-12-11T18:47:08Z

    fixed header now placed properly; editor updated

commit 3256e76dfdcbb3661513eb2d7784753129052f12
Author: Benjamin Keen <be...@gmail.com>
Date:   2014-12-11T20:01:37Z

    editor focus unload; style fixes

commit 644ba058a1d8525cfde589de542488d904e83ba4
Author: Benjamin Keen <be...@gmail.com>
Date:   2014-12-11T21:30:42Z

    confirmation modal

commit cafaff9abdf853ffb04b09f5ee2466579c169410
Author: Benjamin Keen <be...@gmail.com>
Date:   2014-12-12T00:35:26Z

    autoScrollEditorIntoView ace settings added

commit 842f260719a73ad77941ca236a7ed10f91bbd2fe
Author: Benjamin Keen <be...@gmail.com>
Date:   2014-12-12T01:54:22Z

    Fixin' the bloomin' gutter width

commit 3dc80c9b73ae55c25319127e852eba10e1f75838
Author: Benjamin Keen <be...@gmail.com>
Date:   2014-12-12T17:25:36Z

    sync

commit a45a71a800e608a60cb9b46ad48faec2e4c88b42
Author: Benjamin Keen <be...@gmail.com>
Date:   2014-12-12T19:42:06Z

    media queries for button row

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22097301
  
    --- Diff: app/addons/documents/views-doceditor.js ---
    @@ -398,77 +307,88 @@ function (app, FauxtonAPI, Components, Documents, Databases, prettify) {
                 msg: 'Save failed: ' + responseText,
                 type: 'error',
                 fade: false,
    -            clear: true,
    -            selector: '#doc .errors-container'
    +            clear: true
               });
             });
           } else if(this.model.validationError && this.model.validationError === 'Cannot change a documents id.') {
    -          FauxtonAPI.addNotification({
    -            msg: 'Cannot save: ' + 'Cannot change a documents _id, try Duplicate doc instead!',
    -            type: 'error',
    -            selector: '#doc .errors-container',
    -            clear:  true
    -          });
    +        FauxtonAPI.addNotification({
    +          msg: 'Cannot save. Cannot change a documents _id, try Clone Document instead!',
    +          type: 'error',
    +          clear:  true
    +        });
             delete this.model.validationError;
           } else {
             FauxtonAPI.addNotification({
    -          msg: 'Please fix the JSON errors and try again.',
    +          msg: 'Please fix the JSON errors and try saving again.',
               type: 'error',
    -          selector: '#doc .errors-container',
               clear:  true
             });
           }
         },
     
         getDocFromEditor: function () {
    -      var json;
    -
           if (!this.hasValidCode()) {
             return false;
           }
    -
    -      json = JSON.parse(this.editor.getValue());
    -
    +      var json = JSON.parse(this.editor.getValue());
           this.model.clear().set(json, {validate: true});
           if (this.model.validationError) {
             return false;
           }
    -
           return this.model;
         },
     
    -    hasValidCode: function () {
    -      var errors = this.editor.getAnnotations();
    -      return errors.length === 0;
    +    beforeRender: function () {
    +      var that = this;
    +
    +      this.uploadModal = this.setView('#upload-modal', new this.components.uploadModal({ model: this.model }));
    +      this.duplicateModal = this.setView('#duplicate-modal', new this.components.duplicateDocModal({ model: this.model }));
    +      this.confirmDeleteModal = this.setView('#delete-doc-modal', new this.components.confirmModal({
    +        text: 'Are you sure you want to delete this document?',
    +        action: that.deleteDocument
    --- End diff --
    
    if you change this to `_.bind(this.deleteDocument, this)` then we can remove the `_.bindAll(this)` in the initialise.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22095840
  
    --- Diff: app/addons/documents/views-doceditor.js ---
    @@ -207,184 +205,95 @@ function (app, FauxtonAPI, Components, Documents, Databases, prettify) {
           'click button.delete': 'destroy',
           'click button.duplicate': 'duplicate',
           'click button.upload': 'upload',
    -      'click button.js-back': 'goback',
    -      'click button.string-edit': 'stringEditing'
    +      'click a.js-back': 'onClickGoBack'
         },
     
         disableLoader: true,
     
         initialize: function (options) {
           this.database = options.database;
    -      _.bindAll(this);
    -    },
     
    -    goback: function () {
    -      var lastPages = FauxtonAPI.router.lastPages;
    +      // to allow dependency injection
    +      this.components = _.extend({
    --- End diff --
    
    I don't quite follow how this allows for dependency injection? Also I think the _.extend might be unnecessary. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#issuecomment-69128552
  
    @benkeen strings color is too dark. It should be lighter, like save button is. JSON and other code syntax highlighting scheme should be radical and looks alien among the main design.
    
    P.S. I'm also very sceptical on dark theme trend for code blocks and views rendering. since it turns Fauxnton design into completely dark style and top header start looking too bright and starts attracting more attention while it should not.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#issuecomment-67619871
  
    @benkeen this is looking awesome. Just some small nits on the code. Also as mentioned earlier we will need the string edit modal back. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22266552
  
    --- Diff: app/addons/documents/views-doceditor.js ---
    @@ -207,184 +205,95 @@ function (app, FauxtonAPI, Components, Documents, Databases, prettify) {
           'click button.delete': 'destroy',
           'click button.duplicate': 'duplicate',
           'click button.upload': 'upload',
    -      'click button.js-back': 'goback',
    -      'click button.string-edit': 'stringEditing'
    +      'click a.js-back': 'onClickGoBack'
         },
     
         disableLoader: true,
     
         initialize: function (options) {
           this.database = options.database;
    -      _.bindAll(this);
    -    },
     
    -    goback: function () {
    -      var lastPages = FauxtonAPI.router.lastPages;
    +      // to allow dependency injection
    +      this.components = _.extend({
    +        confirmModal: Components.ConfirmationModal,
    +        uploadModal: Views.UploadModal,
    +        duplicateDocModal: Views.DuplicateDocModal
    +      }, options);
     
    -      // we copy/pasted the url into the browser or came from
    -      // creating a document with "/new" in the end of the path
    -      if (lastPages.length < 2 || /\/new$/.test(lastPages[0])) {
    -        FauxtonAPI.navigate(this.database.url('index') + '?limit=100');
    -      } else {
    -        window.history.back();
    -      }
    +      _.bindAll(this);
    --- End diff --
    
    Good call! Nope. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#issuecomment-69126039
  
    Hey @seanbarclay, see attached: 
    ![screen shot 2015-01-07 at 6 09 18 pm](https://cloud.githubusercontent.com/assets/512116/5656869/3cb1fb88-9698-11e4-922f-2f6a90debbb2.png)
    
    Can I leave the JSON properties to be a different colour from the values, even though technically both are strings? See screenshot. But maybe white's not the best colour for them.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22274228
  
    --- Diff: app/addons/documents/resources.js ---
    @@ -675,6 +675,21 @@ function(app, FauxtonAPI, PagingCollection) {
       });
     
     
    +  function getPreviousPage (database) {
    --- End diff --
    
    this is a duplicate to `Helpers.getPreviousPage` in `app/addons/documents/helpers.js`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#issuecomment-69814231
  
    Thanks, @michellephung :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22274269
  
    --- Diff: app/addons/documents/views-doceditor.js ---
    @@ -398,77 +308,89 @@ function (app, FauxtonAPI, Components, Documents, Databases, prettify) {
                 msg: 'Save failed: ' + responseText,
                 type: 'error',
                 fade: false,
    -            clear: true,
    -            selector: '#doc .errors-container'
    +            clear: true
               });
             });
           } else if(this.model.validationError && this.model.validationError === 'Cannot change a documents id.') {
    -          FauxtonAPI.addNotification({
    -            msg: 'Cannot save: ' + 'Cannot change a documents _id, try Duplicate doc instead!',
    -            type: 'error',
    -            selector: '#doc .errors-container',
    -            clear:  true
    -          });
    +        FauxtonAPI.addNotification({
    +          msg: 'Cannot save. Cannot change a documents _id, try Clone Document instead!',
    +          type: 'error',
    +          clear:  true
    +        });
             delete this.model.validationError;
           } else {
             FauxtonAPI.addNotification({
    -          msg: 'Please fix the JSON errors and try again.',
    +          msg: 'Please fix the JSON errors and try saving again.',
               type: 'error',
    -          selector: '#doc .errors-container',
               clear:  true
             });
           }
         },
     
         getDocFromEditor: function () {
    -      var json;
    -
           if (!this.hasValidCode()) {
             return false;
           }
    -
    -      json = JSON.parse(this.editor.getValue());
    -
    +      var json = JSON.parse(this.editor.getValue());
           this.model.clear().set(json, {validate: true});
           if (this.model.validationError) {
             return false;
           }
    -
           return this.model;
         },
     
    -    hasValidCode: function () {
    -      var errors = this.editor.getAnnotations();
    -      return errors.length === 0;
    +    beforeRender: function () {
    +      this.uploadModal = this.setView('#upload-modal', new this.components.uploadModal({ model: this.model }));
    +      this.duplicateModal = this.setView('#duplicate-modal', new this.components.duplicateDocModal({ model: this.model }));
    +      this.confirmDeleteModal = this.setView('#delete-doc-modal', new this.components.confirmModal({
    +        text: 'Are you sure you want to delete this document?',
    +        action: this.deleteDocument
    +      }));
    +
    +      // ensures it's initialized only once
    +      this.stringEditModal = this.stringEditModal || this.setView('#string-edit-modal', new Views.StringEditModal());
         },
     
    -    serialize: function () {
    -      return {
    -        doc: this.model,
    -        attachments: this.getAttachments()
    -      };
    +    updateValues: function () {
    +      if (this.model.changedAttributes()) {
    +        FauxtonAPI.addNotification({
    +          msg: 'Document saved successfully.',
    +          type: 'success',
    +          clear: true
    +        });
    +        this.editor.setValue(this.model.prettyJSON());
    +      }
         },
     
    -    getAttachments: function () {
    -      var attachments = this.model.get('_attachments');
    +    establish: function () {
    +      var promise = this.model.fetch(),
    +          deferred = $.Deferred(),
    +          that = this;
     
    -      if (!attachments) { return false; }
    +      promise.then(function () {
    +        deferred.resolve();
    +      }, function (xhr, reason, msg) {
    +        if (xhr.status === 404) {
    +          FauxtonAPI.addNotification({
    +            msg: 'The document does not exist',
    +            type: 'error',
    +            clear: true
    +          });
    +          that.goBack();
    --- End diff --
    
    we use `.bind` or `_.bind` and try to avoid `that = this`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22266711
  
    --- Diff: app/addons/documents/views-doceditor.js ---
    @@ -505,35 +426,68 @@ function (app, FauxtonAPI, Components, Documents, Databases, prettify) {
                 keyChecked.push('_rev');
               }
     
    -          //check the changedDoc has all the required standard keys
    -          if (_.isEmpty(_.difference(keyChecked, _.keys(changedDoc)))) { return; }
    +          // check the changedDoc has all the required standard keys
    +          if (_.isEmpty(_.difference(keyChecked, _.keys(changedDoc)))) {
    +            return;
    +          }
     
               editor.setReadOnly(true);
               setTimeout(function () { editor.setReadOnly(false) ;}, 400);
    +
               // use extend so that _id stays at the top of the object with displaying the doc
               changedDoc = _.extend({_id: model.id, _rev: model.get('_rev')}, changedDoc);
               editor.setValue(JSON.stringify(changedDoc, null, '  '));
               FauxtonAPI.addNotification({
                 type: 'error',
    -            msg: 'Cannot remove a documents Id or Revision.',
    -            clear:  true
    +            msg: 'Cannot remove a document\'s id or revision.',
    +            clear: true
               });
             });
     
    -        var showHideEditDocString = _.bind(this.showHideEditDocString, this);
    -
    -        this.listenTo(editor.editor, 'changeSelection', function (event) {
    -          showHideEditDocString(event);
    -        });
    -        this.listenTo(editor.editor.session, 'changeBackMarker', function (event) {
    -          showHideEditDocString(event);
    -        });
    +        // place focus on the editor
    +        editor.editor.focus();
     
           }.bind(this));
    +
    +      // this sucks having to reference #dashboard, but when user clicks in the region below the editor I want it
    +      // to focus on the final line. It feels natural that way
    +      $('.scrollable').on('click', function(e) {
    +        var clickedInEditor = $(e.target).closest('#editor-container');
    +        if (clickedInEditor.length === 0) {
    +          editor.editor.focus();
    +          var session = editor.editor.getSession();
    +          var count = session.getLength();
    +          editor.editor.gotoLine(count, session.getLine(count-1).length);
    +        }
    +      });
    +    },
    +
    +    serialize: function () {
    +      return {
    +        doc: this.model,
    +        attachments: this.getAttachments()
    +      };
    +    },
    +
    +    getAttachments: function () {
    +      var attachments = this.model.get('_attachments');
    +      if (!attachments) { return false; }
    +
    +      return _.map(attachments, function (att, key) {
    +        return {
    +          fileName: key,
    +          size: att.length,
    +          contentType: att.content_type,
    +          url: this.model.url() + '/' + app.utils.safeURLName(key)
    +        };
    +      }, this);
         },
     
         cleanup: function () {
    -      if (this.editor) this.editor.remove();
    +      if (this.editor) {
    --- End diff --
    
    True! I don't usually use that syntax. I kinda prefer the if-version myself - seems more explicit somehow, despite its verbosity. Updated!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by seanbarclay <gi...@git.apache.org>.
Github user seanbarclay commented on the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#issuecomment-69387245
  
    The reason behind the dark code editor was to bring in some consistency and familiarity with modern developer code editors (e.g. SublimeText), which leverage a dark, high contrast palette. It helps to differentiate the functionality a bit between normal UI controls and code. The docs and the editor were designed to be dark to be consistent. However, they were also designed with a lot of functionality that hasn't been baked in yet (e.g.. table views). I designed a lot of this close to a year ago and it's just starting to make it's way in , which is a good sign... but there's still a long way to go. 
    
    Also, a few developers that I tested with preferred the dark theme vs. a white or lighter theme. It seemed like a safe bet. 
    
    That being said, we can definitely continue to tweak the contrast on the colors to pop or be toned back. The colors we submitted are meant to help developers differentiate between keys (white) and values such as strings (teal), numbers (light blue) and boolean (orange). I'm not opposed to shifting colors to help with contrast and readability. 
    
    I'll take a look at lightening up that green and post an update soon. 
    Cheers


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by michellephung <gi...@git.apache.org>.
Github user michellephung commented on the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#issuecomment-67393163
  
    Cool things going on here.
    
    One thing, if there's time, when scrolling on the gray area below the editor, looks like it would scroll, but doesn't. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22274513
  
    --- Diff: app/addons/documents/views-doceditor.js ---
    @@ -529,13 +454,160 @@ function (app, FauxtonAPI, Components, Documents, Databases, prettify) {
               showHideEditDocString(event);
             });
     
    +        // place focus on the editor
    +        editor.editor.focus();
    +
           }.bind(this));
    +
    +      // this sucks having to reference #dashboard, but when user clicks in the region below the editor I want it
    +      // to focus on the final line. It feels natural that way
    +      $('.scrollable').on('click', function(e) {
    +        var clickedInEditor = $(e.target).closest('#editor-container');
    +        if (clickedInEditor.length === 0) {
    +          editor.editor.focus();
    +          var session = editor.editor.getSession();
    +          var count = session.getLength();
    +          editor.editor.gotoLine(count, session.getLine(count-1).length);
    +        }
    +      });
    +    },
    +
    +    serialize: function () {
    +      return {
    +        doc: this.model,
    +        attachments: this.getAttachments()
    +      };
    +    },
    +
    +    getAttachments: function () {
    +      var attachments = this.model.get('_attachments');
    +      if (!attachments) { return false; }
    +
    +      return _.map(attachments, function (att, key) {
    +        return {
    +          fileName: key,
    +          size: att.length,
    +          contentType: att.content_type,
    +          url: this.model.url() + '/' + app.utils.safeURLName(key)
    +        };
    +      }, this);
    +    },
    +
    +    determineStringEditMatch: function (event) {
    +      var selStart = this.editor.getSelectionStart().row;
    +      var selEnd = this.editor.getSelectionEnd().row;
    +
    +      // one JS(ON) string can't span more than one line - we edit one string, so ensure we don't select several lines
    +      if (selStart >=0 && selEnd >= 0 && selStart === selEnd && this.editor.isRowExpanded(selStart)) {
    +        var editLine = this.editor.getLine(selStart),
    +            editMatch = editLine.match(/^([ \t]*)(["|'][a-zA-Z0-9_]*["|']: )?(["|'].*["|'],?[ \t]*)$/);
    +
    +        if (editMatch) {
    +          return editMatch;
    +        }
    +      }
    +      return null;
    +    },
    +
    +    showHideEditDocString: function (event) {
    --- End diff --
    
    I'd removed the string editing code, then put it back in. Probably wasn't in the same spot. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22095909
  
    --- Diff: app/addons/documents/views-doceditor.js ---
    @@ -398,77 +307,88 @@ function (app, FauxtonAPI, Components, Documents, Databases, prettify) {
                 msg: 'Save failed: ' + responseText,
                 type: 'error',
                 fade: false,
    -            clear: true,
    -            selector: '#doc .errors-container'
    +            clear: true
               });
             });
           } else if(this.model.validationError && this.model.validationError === 'Cannot change a documents id.') {
    -          FauxtonAPI.addNotification({
    -            msg: 'Cannot save: ' + 'Cannot change a documents _id, try Duplicate doc instead!',
    -            type: 'error',
    -            selector: '#doc .errors-container',
    -            clear:  true
    -          });
    +        FauxtonAPI.addNotification({
    +          msg: 'Cannot save. Cannot change a documents _id, try Clone Document instead!',
    +          type: 'error',
    +          clear:  true
    +        });
             delete this.model.validationError;
           } else {
             FauxtonAPI.addNotification({
    -          msg: 'Please fix the JSON errors and try again.',
    +          msg: 'Please fix the JSON errors and try saving again.',
               type: 'error',
    -          selector: '#doc .errors-container',
               clear:  true
             });
           }
         },
     
         getDocFromEditor: function () {
    -      var json;
    -
           if (!this.hasValidCode()) {
             return false;
           }
    -
    -      json = JSON.parse(this.editor.getValue());
    -
    +      var json = JSON.parse(this.editor.getValue());
           this.model.clear().set(json, {validate: true});
           if (this.model.validationError) {
             return false;
           }
    -
           return this.model;
         },
     
    -    hasValidCode: function () {
    -      var errors = this.editor.getAnnotations();
    -      return errors.length === 0;
    +    beforeRender: function () {
    +      var that = this;
    --- End diff --
    
    I don't think the `that` is required here.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22274212
  
    --- Diff: app/addons/documents/views-doceditor.js ---
    @@ -505,23 +428,25 @@ function (app, FauxtonAPI, Components, Documents, Databases, prettify) {
                 keyChecked.push('_rev');
               }
     
    -          //check the changedDoc has all the required standard keys
    -          if (_.isEmpty(_.difference(keyChecked, _.keys(changedDoc)))) { return; }
    +          // check the changedDoc has all the required standard keys
    +          if (_.isEmpty(_.difference(keyChecked, _.keys(changedDoc)))) {
    +            return;
    +          }
     
               editor.setReadOnly(true);
               setTimeout(function () { editor.setReadOnly(false) ;}, 400);
    +
               // use extend so that _id stays at the top of the object with displaying the doc
               changedDoc = _.extend({_id: model.id, _rev: model.get('_rev')}, changedDoc);
               editor.setValue(JSON.stringify(changedDoc, null, '  '));
               FauxtonAPI.addNotification({
                 type: 'error',
    -            msg: 'Cannot remove a documents Id or Revision.',
    -            clear:  true
    +            msg: 'Cannot remove a document\'s id or revision.',
    --- End diff --
    
    `msg: "Cannot remove a document's id or revision."`
    
    is easier to read and write


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22275190
  
    --- Diff: app/addons/documents/views-doceditor.js ---
    @@ -529,13 +454,160 @@ function (app, FauxtonAPI, Components, Documents, Databases, prettify) {
               showHideEditDocString(event);
             });
     
    +        // place focus on the editor
    +        editor.editor.focus();
    +
           }.bind(this));
    +
    +      // this sucks having to reference #dashboard, but when user clicks in the region below the editor I want it
    +      // to focus on the final line. It feels natural that way
    +      $('.scrollable').on('click', function(e) {
    --- End diff --
    
    Dude, this is excellent. That code really bugged me. When I originally worked on it, there was nothing in the View that expanded the full height of the page so it wasn't possible. After the CSS header/layout refactor that changed. I'll add it now.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#issuecomment-69426713
  
    Squashed + rebased with master.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#issuecomment-67584120
  
    Hey @garrensmith, if you have time, mind looking over this one? It's waiting on some colour fixes for the editor but I was hoping to get it merged over Christmas. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22274435
  
    --- Diff: app/addons/documents/views-doceditor.js ---
    @@ -529,13 +454,160 @@ function (app, FauxtonAPI, Components, Documents, Databases, prettify) {
               showHideEditDocString(event);
             });
     
    +        // place focus on the editor
    +        editor.editor.focus();
    +
           }.bind(this));
    +
    +      // this sucks having to reference #dashboard, but when user clicks in the region below the editor I want it
    +      // to focus on the final line. It feels natural that way
    +      $('.scrollable').on('click', function(e) {
    +        var clickedInEditor = $(e.target).closest('#editor-container');
    +        if (clickedInEditor.length === 0) {
    +          editor.editor.focus();
    +          var session = editor.editor.getSession();
    +          var count = session.getLength();
    +          editor.editor.gotoLine(count, session.getLine(count-1).length);
    +        }
    +      });
    +    },
    +
    +    serialize: function () {
    +      return {
    +        doc: this.model,
    +        attachments: this.getAttachments()
    +      };
    +    },
    +
    +    getAttachments: function () {
    +      var attachments = this.model.get('_attachments');
    +      if (!attachments) { return false; }
    +
    +      return _.map(attachments, function (att, key) {
    +        return {
    +          fileName: key,
    +          size: att.length,
    +          contentType: att.content_type,
    +          url: this.model.url() + '/' + app.utils.safeURLName(key)
    +        };
    +      }, this);
    +    },
    +
    +    determineStringEditMatch: function (event) {
    +      var selStart = this.editor.getSelectionStart().row;
    +      var selEnd = this.editor.getSelectionEnd().row;
    +
    +      // one JS(ON) string can't span more than one line - we edit one string, so ensure we don't select several lines
    +      if (selStart >=0 && selEnd >= 0 && selStart === selEnd && this.editor.isRowExpanded(selStart)) {
    +        var editLine = this.editor.getLine(selStart),
    +            editMatch = editLine.match(/^([ \t]*)(["|'][a-zA-Z0-9_]*["|']: )?(["|'].*["|'],?[ \t]*)$/);
    +
    +        if (editMatch) {
    +          return editMatch;
    +        }
    +      }
    +      return null;
    +    },
    +
    +    showHideEditDocString: function (event) {
    +      this.$('button.string-edit').attr('disabled', 'true');
    +      if (!this.hasValidCode()) {
    +        return false;
    +      }
    +      var editMatch = this.determineStringEditMatch(event);
    +      if (editMatch) {
    +        this.$('button.string-edit').removeAttr('disabled');
    +        /* remove the following line (along with CSS) to go back to the toolbar: take the offset top of the editor, go down as many lines as we are positioned including fold and adjust by two pixels as the button is slightly larger than a line */
    +        var positionFromTop = (this.$('#editor-container').offset().top - 2 + this.editor.getRowHeight() * this.editor.documentToScreenRow(this.editor.getSelectionStart().row)) - 62;
    +        this.$('button.string-edit').css('top', positionFromTop + 'px');
    +        return true;
    +      }
    +      return false;
    +    },
    +
    +    stringEditing: function (event) {
    +      event.preventDefault();
    +      if (!this.hasValidCode()) {
    +        return;
    +      }
    +      var editMatch = this.determineStringEditMatch(event);
    +      if (editMatch) {
    +        var indent = editMatch[1] || '',
    +          hashKey = editMatch[2] || '',
    +          editText = editMatch[3],
    +          comma = '';
    +        if (editText.substring(editText.length - 1) === ',') {
    +          editText = editText.substring(0, editText.length - 1);
    +          comma = ',';
    +        }
    +        this.stringEditModal.openWin(this.editor, indent, hashKey, editText, comma);
    +      }
         },
     
         cleanup: function () {
    -      if (this.editor) this.editor.remove();
    +      this.editor && this.editor.remove();
    +      $('#dashboard').off('click');
         }
       });
     
    +
    +  Views.StringEditModal = Components.ModalView.extend({
    +    template: 'addons/documents/templates/string_edit_modal',
    +
    +    initialize: function () {
    +      _.bindAll(this);
    --- End diff --
    
    do we need `_.bindAll`? I think we want to get rid of it


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22274363
  
    --- Diff: app/addons/documents/views-doceditor.js ---
    @@ -398,77 +308,89 @@ function (app, FauxtonAPI, Components, Documents, Databases, prettify) {
                 msg: 'Save failed: ' + responseText,
                 type: 'error',
                 fade: false,
    -            clear: true,
    -            selector: '#doc .errors-container'
    +            clear: true
               });
             });
           } else if(this.model.validationError && this.model.validationError === 'Cannot change a documents id.') {
    -          FauxtonAPI.addNotification({
    -            msg: 'Cannot save: ' + 'Cannot change a documents _id, try Duplicate doc instead!',
    -            type: 'error',
    -            selector: '#doc .errors-container',
    -            clear:  true
    -          });
    +        FauxtonAPI.addNotification({
    +          msg: 'Cannot save. Cannot change a documents _id, try Clone Document instead!',
    +          type: 'error',
    +          clear:  true
    +        });
             delete this.model.validationError;
           } else {
             FauxtonAPI.addNotification({
    -          msg: 'Please fix the JSON errors and try again.',
    +          msg: 'Please fix the JSON errors and try saving again.',
               type: 'error',
    -          selector: '#doc .errors-container',
               clear:  true
             });
           }
         },
     
         getDocFromEditor: function () {
    -      var json;
    -
           if (!this.hasValidCode()) {
             return false;
           }
    -
    -      json = JSON.parse(this.editor.getValue());
    -
    +      var json = JSON.parse(this.editor.getValue());
           this.model.clear().set(json, {validate: true});
           if (this.model.validationError) {
             return false;
           }
    -
           return this.model;
         },
     
    -    hasValidCode: function () {
    -      var errors = this.editor.getAnnotations();
    -      return errors.length === 0;
    +    beforeRender: function () {
    +      this.uploadModal = this.setView('#upload-modal', new this.components.uploadModal({ model: this.model }));
    +      this.duplicateModal = this.setView('#duplicate-modal', new this.components.duplicateDocModal({ model: this.model }));
    +      this.confirmDeleteModal = this.setView('#delete-doc-modal', new this.components.confirmModal({
    +        text: 'Are you sure you want to delete this document?',
    +        action: this.deleteDocument
    +      }));
    +
    +      // ensures it's initialized only once
    +      this.stringEditModal = this.stringEditModal || this.setView('#string-edit-modal', new Views.StringEditModal());
         },
     
    -    serialize: function () {
    -      return {
    -        doc: this.model,
    -        attachments: this.getAttachments()
    -      };
    +    updateValues: function () {
    +      if (this.model.changedAttributes()) {
    +        FauxtonAPI.addNotification({
    +          msg: 'Document saved successfully.',
    +          type: 'success',
    +          clear: true
    +        });
    +        this.editor.setValue(this.model.prettyJSON());
    +      }
         },
     
    -    getAttachments: function () {
    -      var attachments = this.model.get('_attachments');
    +    establish: function () {
    +      var promise = this.model.fetch(),
    +          deferred = $.Deferred(),
    +          that = this;
     
    -      if (!attachments) { return false; }
    +      promise.then(function () {
    +        deferred.resolve();
    +      }, function (xhr, reason, msg) {
    +        if (xhr.status === 404) {
    +          FauxtonAPI.addNotification({
    +            msg: 'The document does not exist',
    +            type: 'error',
    +            clear: true
    +          });
    +          that.goBack();
    +        }
    +        deferred.reject();
    +     });
     
    -      return _.map(attachments, function (att, key) {
    -        return {
    -          fileName: key,
    -          size: att.length,
    -          contentType: att.content_type,
    -          url: this.model.url() + '/' + app.utils.safeURLName(key)
    -        };
    -      }, this);
    +      return deferred;
         },
     
    -    afterRender: function () {
    -      var saveDoc = this.saveDoc,
    -          editor,
    -          model;
    +    hasValidCode: function () {
    +      var errors = this.editor.getAnnotations();
    +      return errors.length === 0;
    +    },
     
    +    afterRender: function () {
    +      _.bind(this.saveDoc, this);
    --- End diff --
    
    that is not how binding context with `this` works
    
    You can either use the native way to bind the context of this:
    
    ```javascript
              exec: function (editor) {
                this.saveDoc();
              }.bind(this),
    ```
    
    or use underscore/lodash (http://underscorejs.org/#bind):
    
    ```javascript
          var saveDoc = _.bind(this.saveDoc, this);
          this.listenTo(this.model, 'sync', this.updateValues);
    
          this.editor = new Components.Editor({
            editorId: 'editor-container',
            forceMissingId: true,
            commands: [{
              name: 'save',
              bindKey: {win: 'Ctrl-S',  mac: 'Ctrl-S'},
              exec: function (editor) {
                saveDoc();
              },
              readOnly: true // false if this command should not apply in readOnly mode
            }]
          });
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22274508
  
    --- Diff: app/addons/documents/views-doceditor.js ---
    @@ -486,16 +408,17 @@ function (app, FauxtonAPI, Components, Documents, Databases, prettify) {
     
           this.editor.render();
     
    -      editor = this.editor;
    -      model = this.model;
    -      //only start listening to editor once it has been rendered
    +      var editor = this.editor;
    +      var model = this.model;
    --- End diff --
    
    I just find the code harder to read that way.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/couchdb-fauxton/pull/204


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22274475
  
    --- Diff: app/addons/documents/views-doceditor.js ---
    @@ -529,13 +454,160 @@ function (app, FauxtonAPI, Components, Documents, Databases, prettify) {
               showHideEditDocString(event);
             });
     
    +        // place focus on the editor
    +        editor.editor.focus();
    +
           }.bind(this));
    +
    +      // this sucks having to reference #dashboard, but when user clicks in the region below the editor I want it
    +      // to focus on the final line. It feels natural that way
    +      $('.scrollable').on('click', function(e) {
    +        var clickedInEditor = $(e.target).closest('#editor-container');
    +        if (clickedInEditor.length === 0) {
    +          editor.editor.focus();
    +          var session = editor.editor.getSession();
    +          var count = session.getLength();
    +          editor.editor.gotoLine(count, session.getLine(count-1).length);
    +        }
    +      });
    +    },
    +
    +    serialize: function () {
    +      return {
    +        doc: this.model,
    +        attachments: this.getAttachments()
    +      };
    +    },
    +
    +    getAttachments: function () {
    +      var attachments = this.model.get('_attachments');
    +      if (!attachments) { return false; }
    +
    +      return _.map(attachments, function (att, key) {
    +        return {
    +          fileName: key,
    +          size: att.length,
    +          contentType: att.content_type,
    +          url: this.model.url() + '/' + app.utils.safeURLName(key)
    +        };
    +      }, this);
    +    },
    +
    +    determineStringEditMatch: function (event) {
    +      var selStart = this.editor.getSelectionStart().row;
    +      var selEnd = this.editor.getSelectionEnd().row;
    +
    +      // one JS(ON) string can't span more than one line - we edit one string, so ensure we don't select several lines
    +      if (selStart >=0 && selEnd >= 0 && selStart === selEnd && this.editor.isRowExpanded(selStart)) {
    +        var editLine = this.editor.getLine(selStart),
    +            editMatch = editLine.match(/^([ \t]*)(["|'][a-zA-Z0-9_]*["|']: )?(["|'].*["|'],?[ \t]*)$/);
    +
    +        if (editMatch) {
    +          return editMatch;
    +        }
    +      }
    +      return null;
    +    },
    +
    +    showHideEditDocString: function (event) {
    --- End diff --
    
    What is the reason to move all that code around in this diff? It makes it very hard to review for me


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Updating edit doc page

Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#issuecomment-67379015
  
    Terrific! Nice to meet you, @SoleilPhan - let me know if you have any questions or anything. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22266659
  
    --- Diff: app/addons/documents/views-doceditor.js ---
    @@ -398,77 +307,88 @@ function (app, FauxtonAPI, Components, Documents, Databases, prettify) {
                 msg: 'Save failed: ' + responseText,
                 type: 'error',
                 fade: false,
    -            clear: true,
    -            selector: '#doc .errors-container'
    +            clear: true
               });
             });
           } else if(this.model.validationError && this.model.validationError === 'Cannot change a documents id.') {
    -          FauxtonAPI.addNotification({
    -            msg: 'Cannot save: ' + 'Cannot change a documents _id, try Duplicate doc instead!',
    -            type: 'error',
    -            selector: '#doc .errors-container',
    -            clear:  true
    -          });
    +        FauxtonAPI.addNotification({
    +          msg: 'Cannot save. Cannot change a documents _id, try Clone Document instead!',
    +          type: 'error',
    +          clear:  true
    +        });
             delete this.model.validationError;
           } else {
             FauxtonAPI.addNotification({
    -          msg: 'Please fix the JSON errors and try again.',
    +          msg: 'Please fix the JSON errors and try saving again.',
               type: 'error',
    -          selector: '#doc .errors-container',
               clear:  true
             });
           }
         },
     
         getDocFromEditor: function () {
    -      var json;
    -
           if (!this.hasValidCode()) {
             return false;
           }
    -
    -      json = JSON.parse(this.editor.getValue());
    -
    +      var json = JSON.parse(this.editor.getValue());
           this.model.clear().set(json, {validate: true});
           if (this.model.validationError) {
             return false;
           }
    -
           return this.model;
         },
     
    -    hasValidCode: function () {
    -      var errors = this.editor.getAnnotations();
    -      return errors.length === 0;
    +    beforeRender: function () {
    +      var that = this;
    +
    +      this.uploadModal = this.setView('#upload-modal', new this.components.uploadModal({ model: this.model }));
    +      this.duplicateModal = this.setView('#duplicate-modal', new this.components.duplicateDocModal({ model: this.model }));
    +      this.confirmDeleteModal = this.setView('#delete-doc-modal', new this.components.confirmModal({
    +        text: 'Are you sure you want to delete this document?',
    +        action: that.deleteDocument
    --- End diff --
    
    Hmm seems like this should work, but I still get errors. For now I'll add the _.bindAll back in, because it's the only method I've found that works. Let's talk in the new year.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22266472
  
    --- Diff: app/addons/documents/templates/code_editor.html ---
    @@ -12,44 +12,64 @@
     the License.
     */%>
     
    -<div id="doc">
    -  <div class="errors-container"></div>
    -   
    -<div id="doc-actions" class="nav">
    -  <div class="span3">
    +<div id="doc-editor-actions-panel">
    +  <div class="doc-actions-left">
         <button class="save-doc btn btn-success save" type="button"><i class="icon fonticon-ok-circled"></i> Save</button>
    -    <button class="btn js-back">Back</button>
    +    <div>
    +      <a href="#" class="js-back cancel-button">Cancel</a>
    +    </div>
       </div>
     
    -  <div class="span7">
    -    <button class="btn string-edit" title="Edit line" disabled="true"><i class="icon icon-edit"></i></button>  
    +  <div class="alignRight">
    +    <button class="btn string-edit" title="Edit line" disabled="true"><i class="icon icon-edit"></i></button>
    +
         <% if (attachments) { %>
    -    <div class="btn-group">
    -      <button class="dropdown-toggle btn" data-bypass="true" data-toggle="dropdown">
    -        View Attachments
    +    <div class="panelSection btn-group">
    +      <button class="panelButton dropdown-toggle btn" data-bypass="true" data-toggle="dropdown" title="View Attachments"
    +        id="view-attachments-menu">
    +        <i class="icon fonticon-picture"></i>
    +        <span>View Attachments</span>
             <span class="caret"></span>
           </button>
    -      <ul class="dropdown-menu">
    +
    +      <ul class="dropdown-menu" role="menu" aria-labelledby="view-attachments-menu">
             <%_.each(attachments, function (att) { %>
             <li>
    -        <a href="<%- att.url %>" target="_blank" data-bypass="true"> <strong> <%- att.fileName %> </strong> -
    -          <span> <%- att.contentType %>, <%- formatSize(att.size)%> </span>
    -        </a>
    +          <a href="<%- att.url %>" target="_blank" data-bypass="true"> <strong> <%- att.fileName %> </strong> -
    +            <span> <%- att.contentType %>, <%- formatSize(att.size)%> </span>
    +          </a>
             </li>
             <% }) %>
           </ul>
         </div>
    -    <% } %> 
    -    <button class="btn upload"><i class="icon icon-circle-arrow-up"></i> Upload Attachment</button>
    -    <button class="btn duplicate"><i class="icon icon-repeat"></i> Clone document</button>
    -    <button class="btn btn-danger delete"><i class="icon icon-trash"></i></button>
    -  </div>
    +    <% } %>
     
    -<div id="upload-modal"> </div>
    -<div id="duplicate-modal"> </div> 
    -<div id="string-edit-modal"> </div> 
    --- End diff --
    
    Yes, but I'll add it back in. The reason was that this was phase 1 - phase 2 was to be adding in a full table-editing-like feature where you can click on a row to edit the JSON data piece by piece; I thought I'd keep the two totally separate, but perhaps that was a bit overzealous.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#issuecomment-69201104
  
    Thanks @kxepal! I personally don't mind the colours, but this is just a first try as @seanbarclay mentioned. Sean, what do you think of the colours & @kxepal's points? I just committed it so you can download the branch and play around with it.
    __________
    
    N.B. so I don't forget, bug found with this ticket:
    1. edit a doc
    2. Clone it
    3. Click back. Some weirdness there. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22274381
  
    --- Diff: app/addons/documents/views-doceditor.js ---
    @@ -486,16 +408,17 @@ function (app, FauxtonAPI, Components, Documents, Databases, prettify) {
     
           this.editor.render();
     
    -      editor = this.editor;
    -      model = this.model;
    -      //only start listening to editor once it has been rendered
    +      var editor = this.editor;
    +      var model = this.model;
    --- End diff --
    
    just curious, is there a special reason to move all variable declarations from the top of the function body into the code?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22274477
  
    --- Diff: app/addons/documents/views-doceditor.js ---
    @@ -529,13 +454,160 @@ function (app, FauxtonAPI, Components, Documents, Databases, prettify) {
               showHideEditDocString(event);
             });
     
    +        // place focus on the editor
    +        editor.editor.focus();
    +
           }.bind(this));
    +
    +      // this sucks having to reference #dashboard, but when user clicks in the region below the editor I want it
    +      // to focus on the final line. It feels natural that way
    +      $('.scrollable').on('click', function(e) {
    +        var clickedInEditor = $(e.target).closest('#editor-container');
    +        if (clickedInEditor.length === 0) {
    +          editor.editor.focus();
    +          var session = editor.editor.getSession();
    +          var count = session.getLength();
    +          editor.editor.gotoLine(count, session.getLine(count-1).length);
    +        }
    +      });
    +    },
    +
    +    serialize: function () {
    +      return {
    +        doc: this.model,
    +        attachments: this.getAttachments()
    +      };
    +    },
    +
    +    getAttachments: function () {
    +      var attachments = this.model.get('_attachments');
    +      if (!attachments) { return false; }
    +
    +      return _.map(attachments, function (att, key) {
    +        return {
    +          fileName: key,
    +          size: att.length,
    +          contentType: att.content_type,
    +          url: this.model.url() + '/' + app.utils.safeURLName(key)
    +        };
    +      }, this);
    +    },
    +
    +    determineStringEditMatch: function (event) {
    +      var selStart = this.editor.getSelectionStart().row;
    +      var selEnd = this.editor.getSelectionEnd().row;
    +
    +      // one JS(ON) string can't span more than one line - we edit one string, so ensure we don't select several lines
    +      if (selStart >=0 && selEnd >= 0 && selStart === selEnd && this.editor.isRowExpanded(selStart)) {
    +        var editLine = this.editor.getLine(selStart),
    +            editMatch = editLine.match(/^([ \t]*)(["|'][a-zA-Z0-9_]*["|']: )?(["|'].*["|'],?[ \t]*)$/);
    +
    +        if (editMatch) {
    +          return editMatch;
    +        }
    +      }
    +      return null;
    +    },
    +
    +    showHideEditDocString: function (event) {
    +      this.$('button.string-edit').attr('disabled', 'true');
    +      if (!this.hasValidCode()) {
    +        return false;
    +      }
    +      var editMatch = this.determineStringEditMatch(event);
    +      if (editMatch) {
    +        this.$('button.string-edit').removeAttr('disabled');
    +        /* remove the following line (along with CSS) to go back to the toolbar: take the offset top of the editor, go down as many lines as we are positioned including fold and adjust by two pixels as the button is slightly larger than a line */
    +        var positionFromTop = (this.$('#editor-container').offset().top - 2 + this.editor.getRowHeight() * this.editor.documentToScreenRow(this.editor.getSelectionStart().row)) - 62;
    +        this.$('button.string-edit').css('top', positionFromTop + 'px');
    +        return true;
    +      }
    +      return false;
    +    },
    +
    +    stringEditing: function (event) {
    +      event.preventDefault();
    +      if (!this.hasValidCode()) {
    +        return;
    +      }
    +      var editMatch = this.determineStringEditMatch(event);
    +      if (editMatch) {
    +        var indent = editMatch[1] || '',
    +          hashKey = editMatch[2] || '',
    +          editText = editMatch[3],
    +          comma = '';
    +        if (editText.substring(editText.length - 1) === ',') {
    +          editText = editText.substring(0, editText.length - 1);
    +          comma = ',';
    +        }
    +        this.stringEditModal.openWin(this.editor, indent, hashKey, editText, comma);
    +      }
         },
     
         cleanup: function () {
    -      if (this.editor) this.editor.remove();
    +      this.editor && this.editor.remove();
    +      $('#dashboard').off('click');
         }
       });
     
    +
    +  Views.StringEditModal = Components.ModalView.extend({
    +    template: 'addons/documents/templates/string_edit_modal',
    +
    +    initialize: function () {
    +      _.bindAll(this);
    +    },
    +
    +    events: {
    +      'click #string-edit-save-btn':'saveString'
    --- End diff --
    
    please add a space


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22275221
  
    --- Diff: app/addons/documents/views-doceditor.js ---
    @@ -398,77 +308,89 @@ function (app, FauxtonAPI, Components, Documents, Databases, prettify) {
                 msg: 'Save failed: ' + responseText,
                 type: 'error',
                 fade: false,
    -            clear: true,
    -            selector: '#doc .errors-container'
    +            clear: true
               });
             });
           } else if(this.model.validationError && this.model.validationError === 'Cannot change a documents id.') {
    -          FauxtonAPI.addNotification({
    -            msg: 'Cannot save: ' + 'Cannot change a documents _id, try Duplicate doc instead!',
    -            type: 'error',
    -            selector: '#doc .errors-container',
    -            clear:  true
    -          });
    +        FauxtonAPI.addNotification({
    +          msg: 'Cannot save. Cannot change a documents _id, try Clone Document instead!',
    +          type: 'error',
    +          clear:  true
    +        });
             delete this.model.validationError;
           } else {
             FauxtonAPI.addNotification({
    -          msg: 'Please fix the JSON errors and try again.',
    +          msg: 'Please fix the JSON errors and try saving again.',
               type: 'error',
    -          selector: '#doc .errors-container',
               clear:  true
             });
           }
         },
     
         getDocFromEditor: function () {
    -      var json;
    -
           if (!this.hasValidCode()) {
             return false;
           }
    -
    -      json = JSON.parse(this.editor.getValue());
    -
    +      var json = JSON.parse(this.editor.getValue());
           this.model.clear().set(json, {validate: true});
           if (this.model.validationError) {
             return false;
           }
    -
           return this.model;
         },
     
    -    hasValidCode: function () {
    -      var errors = this.editor.getAnnotations();
    -      return errors.length === 0;
    +    beforeRender: function () {
    +      this.uploadModal = this.setView('#upload-modal', new this.components.uploadModal({ model: this.model }));
    +      this.duplicateModal = this.setView('#duplicate-modal', new this.components.duplicateDocModal({ model: this.model }));
    +      this.confirmDeleteModal = this.setView('#delete-doc-modal', new this.components.confirmModal({
    +        text: 'Are you sure you want to delete this document?',
    +        action: this.deleteDocument
    +      }));
    +
    +      // ensures it's initialized only once
    +      this.stringEditModal = this.stringEditModal || this.setView('#string-edit-modal', new Views.StringEditModal());
         },
     
    -    serialize: function () {
    -      return {
    -        doc: this.model,
    -        attachments: this.getAttachments()
    -      };
    +    updateValues: function () {
    +      if (this.model.changedAttributes()) {
    +        FauxtonAPI.addNotification({
    +          msg: 'Document saved successfully.',
    +          type: 'success',
    +          clear: true
    +        });
    +        this.editor.setValue(this.model.prettyJSON());
    +      }
         },
     
    -    getAttachments: function () {
    -      var attachments = this.model.get('_attachments');
    +    establish: function () {
    +      var promise = this.model.fetch(),
    +          deferred = $.Deferred(),
    +          that = this;
     
    -      if (!attachments) { return false; }
    +      promise.then(function () {
    +        deferred.resolve();
    +      }, function (xhr, reason, msg) {
    +        if (xhr.status === 404) {
    +          FauxtonAPI.addNotification({
    +            msg: 'The document does not exist',
    +            type: 'error',
    +            clear: true
    +          });
    +          that.goBack();
    --- End diff --
    
    Sure. This was old code. I'll patch it up.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22274502
  
    --- Diff: app/addons/documents/routes-doc-editor.js ---
    @@ -12,22 +12,22 @@
     
     define([
       "app",
    -
       "api",
     
       // Modules
    -  //views
    +  "addons/documents/helpers",
       "addons/documents/views",
       "addons/documents/views-doceditor",
       "addons/databases/base"
     ],
     
    -function(app, FauxtonAPI, Documents, DocEditor, Databases) {
    +function(app, FauxtonAPI, Helpers, Documents, DocEditor, Databases) {
    +
     
       var DocEditorRouteObject = FauxtonAPI.RouteObject.extend({
    -    layout: "one_pane",
    +    layout: 'doc_editor',
    --- End diff --
    
    This is both a style + a layout template change. But yes, I'll try to do that. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#issuecomment-67614678
  
    @benkeen this looks great, really nice to have a large editor page. It seems you have removed the string edit modal. This is a really popular feature. Can you add it back in.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by michellephung <gi...@git.apache.org>.
Github user michellephung commented on the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#issuecomment-69395578
  
    @benkeen 
    no worries, its just a nice to have :D


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#issuecomment-69392835
  
    @seanbarclay The reasons behind dark theme are clear. My point was on overall design. If you're uses dark theme for code editor, you also tries to reach two other goals: turn other editor UI into the same dark theme or minimize amount of bright panels. Same problem here: we have nice dark editor for JSON code, but our attention get distracted by large light grey panels above and below it. For view browsing things getting worse since there we have a big burger of dark-light elements (dark left global sidebar, light database sidebar, dark view results table, light pagination and tabs). Design should be consistent whenever light or dark theme is used. And when it is consistent, it looks awesome.
    
    Just my 5 cents (:


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22274509
  
    --- Diff: app/addons/documents/views-doceditor.js ---
    @@ -529,13 +454,160 @@ function (app, FauxtonAPI, Components, Documents, Databases, prettify) {
               showHideEditDocString(event);
             });
     
    +        // place focus on the editor
    +        editor.editor.focus();
    +
           }.bind(this));
    +
    +      // this sucks having to reference #dashboard, but when user clicks in the region below the editor I want it
    +      // to focus on the final line. It feels natural that way
    +      $('.scrollable').on('click', function(e) {
    +        var clickedInEditor = $(e.target).closest('#editor-container');
    +        if (clickedInEditor.length === 0) {
    +          editor.editor.focus();
    +          var session = editor.editor.getSession();
    +          var count = session.getLength();
    +          editor.editor.gotoLine(count, session.getLine(count-1).length);
    +        }
    +      });
    +    },
    +
    +    serialize: function () {
    +      return {
    +        doc: this.model,
    +        attachments: this.getAttachments()
    +      };
    +    },
    +
    +    getAttachments: function () {
    +      var attachments = this.model.get('_attachments');
    +      if (!attachments) { return false; }
    +
    +      return _.map(attachments, function (att, key) {
    +        return {
    +          fileName: key,
    +          size: att.length,
    +          contentType: att.content_type,
    +          url: this.model.url() + '/' + app.utils.safeURLName(key)
    +        };
    +      }, this);
    +    },
    +
    +    determineStringEditMatch: function (event) {
    +      var selStart = this.editor.getSelectionStart().row;
    +      var selEnd = this.editor.getSelectionEnd().row;
    +
    +      // one JS(ON) string can't span more than one line - we edit one string, so ensure we don't select several lines
    +      if (selStart >=0 && selEnd >= 0 && selStart === selEnd && this.editor.isRowExpanded(selStart)) {
    +        var editLine = this.editor.getLine(selStart),
    +            editMatch = editLine.match(/^([ \t]*)(["|'][a-zA-Z0-9_]*["|']: )?(["|'].*["|'],?[ \t]*)$/);
    +
    +        if (editMatch) {
    +          return editMatch;
    +        }
    +      }
    +      return null;
    +    },
    +
    +    showHideEditDocString: function (event) {
    +      this.$('button.string-edit').attr('disabled', 'true');
    +      if (!this.hasValidCode()) {
    +        return false;
    +      }
    +      var editMatch = this.determineStringEditMatch(event);
    +      if (editMatch) {
    +        this.$('button.string-edit').removeAttr('disabled');
    +        /* remove the following line (along with CSS) to go back to the toolbar: take the offset top of the editor, go down as many lines as we are positioned including fold and adjust by two pixels as the button is slightly larger than a line */
    +        var positionFromTop = (this.$('#editor-container').offset().top - 2 + this.editor.getRowHeight() * this.editor.documentToScreenRow(this.editor.getSelectionStart().row)) - 62;
    +        this.$('button.string-edit').css('top', positionFromTop + 'px');
    +        return true;
    +      }
    +      return false;
    +    },
    +
    +    stringEditing: function (event) {
    +      event.preventDefault();
    +      if (!this.hasValidCode()) {
    +        return;
    +      }
    +      var editMatch = this.determineStringEditMatch(event);
    +      if (editMatch) {
    +        var indent = editMatch[1] || '',
    +          hashKey = editMatch[2] || '',
    +          editText = editMatch[3],
    +          comma = '';
    +        if (editText.substring(editText.length - 1) === ',') {
    +          editText = editText.substring(0, editText.length - 1);
    +          comma = ',';
    +        }
    +        this.stringEditModal.openWin(this.editor, indent, hashKey, editText, comma);
    +      }
         },
     
         cleanup: function () {
    -      if (this.editor) this.editor.remove();
    +      this.editor && this.editor.remove();
    +      $('#dashboard').off('click');
         }
       });
     
    +
    +  Views.StringEditModal = Components.ModalView.extend({
    +    template: 'addons/documents/templates/string_edit_modal',
    +
    +    initialize: function () {
    +      _.bindAll(this);
    --- End diff --
    
    This was old code I didn't touch. It can probably be removed, I'll check.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22095866
  
    --- Diff: app/addons/documents/views-doceditor.js ---
    @@ -207,184 +205,95 @@ function (app, FauxtonAPI, Components, Documents, Databases, prettify) {
           'click button.delete': 'destroy',
           'click button.duplicate': 'duplicate',
           'click button.upload': 'upload',
    -      'click button.js-back': 'goback',
    -      'click button.string-edit': 'stringEditing'
    +      'click a.js-back': 'onClickGoBack'
         },
     
         disableLoader: true,
     
         initialize: function (options) {
           this.database = options.database;
    -      _.bindAll(this);
    -    },
     
    -    goback: function () {
    -      var lastPages = FauxtonAPI.router.lastPages;
    +      // to allow dependency injection
    +      this.components = _.extend({
    +        confirmModal: Components.ConfirmationModal,
    +        uploadModal: Views.UploadModal,
    +        duplicateDocModal: Views.DuplicateDocModal
    +      }, options);
     
    -      // we copy/pasted the url into the browser or came from
    -      // creating a document with "/new" in the end of the path
    -      if (lastPages.length < 2 || /\/new$/.test(lastPages[0])) {
    -        FauxtonAPI.navigate(this.database.url('index') + '?limit=100');
    -      } else {
    -        window.history.back();
    -      }
    +      _.bindAll(this);
    --- End diff --
    
    Is this `bindAll` actually necessary?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22097458
  
    --- Diff: app/addons/fauxton/components.js ---
    @@ -30,7 +30,7 @@ define([
       // this should never be global available:
       // https://github.com/zeroclipboard/zeroclipboard/blob/master/docs/security.md
       "plugins/zeroclipboard/ZeroClipboard",
    -  "velocity.ui"
    +  "velocity.ui",
    --- End diff --
    
    This comma is actually unnecessary. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22097350
  
    --- Diff: app/addons/documents/views-doceditor.js ---
    @@ -398,77 +307,88 @@ function (app, FauxtonAPI, Components, Documents, Databases, prettify) {
                 msg: 'Save failed: ' + responseText,
                 type: 'error',
                 fade: false,
    -            clear: true,
    -            selector: '#doc .errors-container'
    +            clear: true
               });
             });
           } else if(this.model.validationError && this.model.validationError === 'Cannot change a documents id.') {
    -          FauxtonAPI.addNotification({
    -            msg: 'Cannot save: ' + 'Cannot change a documents _id, try Duplicate doc instead!',
    -            type: 'error',
    -            selector: '#doc .errors-container',
    -            clear:  true
    -          });
    +        FauxtonAPI.addNotification({
    +          msg: 'Cannot save. Cannot change a documents _id, try Clone Document instead!',
    +          type: 'error',
    +          clear:  true
    +        });
             delete this.model.validationError;
           } else {
             FauxtonAPI.addNotification({
    -          msg: 'Please fix the JSON errors and try again.',
    +          msg: 'Please fix the JSON errors and try saving again.',
               type: 'error',
    -          selector: '#doc .errors-container',
               clear:  true
             });
           }
         },
     
         getDocFromEditor: function () {
    -      var json;
    -
           if (!this.hasValidCode()) {
             return false;
           }
    -
    -      json = JSON.parse(this.editor.getValue());
    -
    +      var json = JSON.parse(this.editor.getValue());
           this.model.clear().set(json, {validate: true});
           if (this.model.validationError) {
             return false;
           }
    -
           return this.model;
         },
     
    -    hasValidCode: function () {
    -      var errors = this.editor.getAnnotations();
    -      return errors.length === 0;
    +    beforeRender: function () {
    +      var that = this;
    +
    +      this.uploadModal = this.setView('#upload-modal', new this.components.uploadModal({ model: this.model }));
    +      this.duplicateModal = this.setView('#duplicate-modal', new this.components.duplicateDocModal({ model: this.model }));
    +      this.confirmDeleteModal = this.setView('#delete-doc-modal', new this.components.confirmModal({
    +        text: 'Are you sure you want to delete this document?',
    +        action: that.deleteDocument
    +      }));
         },
     
    -    serialize: function () {
    -      return {
    -        doc: this.model,
    -        attachments: this.getAttachments()
    -      };
    +    updateValues: function () {
    +      if (this.model.changedAttributes()) {
    +        FauxtonAPI.addNotification({
    +          msg: 'Document saved successfully.',
    +          type: 'success',
    +          clear: true
    +        });
    +        this.editor.setValue(this.model.prettyJSON());
    +      }
         },
     
    -    getAttachments: function () {
    -      var attachments = this.model.get('_attachments');
    +    establish: function () {
    +      var promise = this.model.fetch(),
    +          deferred = $.Deferred(),
    +          that = this;
     
    -      if (!attachments) { return false; }
    +      promise.then(function () {
    +        deferred.resolve();
    +      }, function (xhr, reason, msg) {
    +        if (xhr.status === 404) {
    +          FauxtonAPI.addNotification({
    +            msg: 'The document does not exist',
    +            type: 'error',
    +            clear: true
    +          });
    +          that.goBack();
    +        }
    +        deferred.reject();
    +     });
     
    -      return _.map(attachments, function (att, key) {
    -        return {
    -          fileName: key,
    -          size: att.length,
    -          contentType: att.content_type,
    -          url: this.model.url() + '/' + app.utils.safeURLName(key)
    -        };
    -      }, this);
    +      return deferred;
         },
     
    -    afterRender: function () {
    -      var saveDoc = this.saveDoc,
    -          editor,
    -          model;
    +    hasValidCode: function () {
    +      var errors = this.editor.getAnnotations();
    +      return errors.length === 0;
    +    },
     
    +    afterRender: function () {
    +      var saveDoc = this.saveDoc;
    --- End diff --
    
    This needs to change to use `_.bind`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22274200
  
    --- Diff: app/addons/documents/views-doceditor.js ---
    @@ -207,184 +205,95 @@ function (app, FauxtonAPI, Components, Documents, Databases, prettify) {
           'click button.delete': 'destroy',
           'click button.duplicate': 'duplicate',
           'click button.upload': 'upload',
    -      'click button.js-back': 'goback',
    -      'click button.string-edit': 'stringEditing'
    +      'click a.js-back': 'onClickGoBack'
         },
     
         disableLoader: true,
     
         initialize: function (options) {
           this.database = options.database;
    -      _.bindAll(this);
    -    },
     
    -    goback: function () {
    -      var lastPages = FauxtonAPI.router.lastPages;
    +      // to allow dependency injection
    +      this.components = _.extend({
    --- End diff --
    
    I see the good intention, but I don't get why we write code that _maybe could_ be tested, that is more complex without writing tests for it. 
    
    I could also say, playing the devils advocate: additionally we don't have any test included that confirms that this makes the view easier to test. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by ocelotpotpie <gi...@git.apache.org>.
Github user ocelotpotpie commented on the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#issuecomment-67388208
  
    Looks good! +1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22274463
  
    --- Diff: app/addons/documents/views-doceditor.js ---
    @@ -529,13 +454,160 @@ function (app, FauxtonAPI, Components, Documents, Databases, prettify) {
               showHideEditDocString(event);
             });
     
    +        // place focus on the editor
    +        editor.editor.focus();
    +
           }.bind(this));
    +
    +      // this sucks having to reference #dashboard, but when user clicks in the region below the editor I want it
    +      // to focus on the final line. It feels natural that way
    +      $('.scrollable').on('click', function(e) {
    --- End diff --
    
    this is a bit funky - we have access to `.scrollable` so I also can't imagine why we should do it that way? to reuse the local variables in the scope?
    
    here is a solution (at the top of the literal):
    ```javascript
      Views.CodeEditor = FauxtonAPI.View.extend({
        template: 'addons/documents/templates/code_editor',
        className: 'editor-content-page',
        events: {
          'click button.save-doc': 'saveDoc',
          'click button.delete': 'destroy',
          'click button.duplicate': 'duplicate',
          'click button.upload': 'upload',
          'click button.string-edit': 'stringEditing',
          'click a.js-back': 'onClickGoBack',
          'click .scrollable': 'focusOnLastLine'
        },
    
        focusOnLastLine: function (e) {
          var clickedInEditor = $(e.target).closest('#editor-container');
          if (clickedInEditor.length === 0) {
            this.editor.editor.focus();
            var session = this.editor.editor.getSession();
            var count = session.getLength();
            this.editor.editor.gotoLine(count, session.getLine(count-1).length);
          }
        },
    
    
    ```



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22095392
  
    --- Diff: app/addons/documents/routes-doc-editor.js ---
    @@ -24,10 +22,26 @@ define([
     
     function(app, FauxtonAPI, Documents, DocEditor, Databases) {
     
    +
    +  // helper
    +  function getPreviousPage (database) {
    --- End diff --
    
    This should maybe be moved into the helper or utils file.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by seanbarclay <gi...@git.apache.org>.
Github user seanbarclay commented on the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#issuecomment-68754220
  
    @benkeen let's try these for starters:
    key: white
    value: string: #72cdf4 (teal)
    value: numbers: #29be9d (light blue)
    value: boolean: #ff6532 (orange)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#issuecomment-68084734
  
    Excellent feedback, thanks so much @garrensmith. Adding the string edit modal back now, but I won't merge until it's been looked over a bit more. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22274488
  
    --- Diff: app/addons/documents/views-doceditor.js ---
    @@ -505,23 +428,25 @@ function (app, FauxtonAPI, Components, Documents, Databases, prettify) {
                 keyChecked.push('_rev');
               }
     
    -          //check the changedDoc has all the required standard keys
    -          if (_.isEmpty(_.difference(keyChecked, _.keys(changedDoc)))) { return; }
    +          // check the changedDoc has all the required standard keys
    +          if (_.isEmpty(_.difference(keyChecked, _.keys(changedDoc)))) {
    +            return;
    +          }
     
               editor.setReadOnly(true);
               setTimeout(function () { editor.setReadOnly(false) ;}, 400);
    +
               // use extend so that _id stays at the top of the object with displaying the doc
               changedDoc = _.extend({_id: model.id, _rev: model.get('_rev')}, changedDoc);
               editor.setValue(JSON.stringify(changedDoc, null, '  '));
               FauxtonAPI.addNotification({
                 type: 'error',
    -            msg: 'Cannot remove a documents Id or Revision.',
    -            clear:  true
    +            msg: 'Cannot remove a document\'s id or revision.',
    --- End diff --
    
    Sure... but I wanted to keep the single space consistency, as per style guide.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by michellephung <gi...@git.apache.org>.
Github user michellephung commented on the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#issuecomment-69813589
  
    +1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22266516
  
    --- Diff: app/addons/documents/views-doceditor.js ---
    @@ -207,184 +205,95 @@ function (app, FauxtonAPI, Components, Documents, Databases, prettify) {
           'click button.delete': 'destroy',
           'click button.duplicate': 'duplicate',
           'click button.upload': 'upload',
    -      'click button.js-back': 'goback',
    -      'click button.string-edit': 'stringEditing'
    +      'click a.js-back': 'onClickGoBack'
         },
     
         disableLoader: true,
     
         initialize: function (options) {
           this.database = options.database;
    -      _.bindAll(this);
    -    },
     
    -    goback: function () {
    -      var lastPages = FauxtonAPI.router.lastPages;
    +      // to allow dependency injection
    +      this.components = _.extend({
    --- End diff --
    
    Yeah I added this because @robertkowalski mentioned how our code isn't very testable. 
    
    The idea here is to allow this View to be tested in isolation (not that, I confess, the text exists). It uses `extend` to lays down the default components, but if someone wanted to test this View, they could pass it test components that just stubbed out the functionality of `Components.ConfirmationModal`, `Views.UploadModal` and `Views.DuplicateDocModal`. This would be done like so: 
    
    `var whatever = Views.CodeEditor({ confirmModal: TestConfirmModal, uploadModal: TestUploadModal ... });` 
    
    Basically this just provides more control to the person testing it and not have to worry about those extra components doing extra things that made testing hard.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22266411
  
    --- Diff: app/addons/documents/routes-doc-editor.js ---
    @@ -24,10 +22,26 @@ define([
     
     function(app, FauxtonAPI, Documents, DocEditor, Databases) {
     
    +
    +  // helper
    +  function getPreviousPage (database) {
    --- End diff --
    
    It doesn't belong here, I agree. But it's not really general enough for most pages: the extra code in it that checks that it doesn't come from `/new` is specific to this particular page. I think I'll stick it in the document addon's resources file. I almost want to introduce a new addon-specific helpers.js file for this sort of thing (i.e. non-model/collection-related, but plain ol' helper functions).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#issuecomment-69786845
  
    Thanks for everyone's input! The colours have been adjusted a little; the header darkened (grey not white) and the highlight row darkened for increased emphasis, as per @kxepal's suggestions.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22275146
  
    --- Diff: app/addons/documents/views-doceditor.js ---
    @@ -398,77 +308,89 @@ function (app, FauxtonAPI, Components, Documents, Databases, prettify) {
                 msg: 'Save failed: ' + responseText,
                 type: 'error',
                 fade: false,
    -            clear: true,
    -            selector: '#doc .errors-container'
    +            clear: true
               });
             });
           } else if(this.model.validationError && this.model.validationError === 'Cannot change a documents id.') {
    -          FauxtonAPI.addNotification({
    -            msg: 'Cannot save: ' + 'Cannot change a documents _id, try Duplicate doc instead!',
    -            type: 'error',
    -            selector: '#doc .errors-container',
    -            clear:  true
    -          });
    +        FauxtonAPI.addNotification({
    +          msg: 'Cannot save. Cannot change a documents _id, try Clone Document instead!',
    +          type: 'error',
    +          clear:  true
    +        });
             delete this.model.validationError;
           } else {
             FauxtonAPI.addNotification({
    -          msg: 'Please fix the JSON errors and try again.',
    +          msg: 'Please fix the JSON errors and try saving again.',
               type: 'error',
    -          selector: '#doc .errors-container',
               clear:  true
             });
           }
         },
     
         getDocFromEditor: function () {
    -      var json;
    -
           if (!this.hasValidCode()) {
             return false;
           }
    -
    -      json = JSON.parse(this.editor.getValue());
    -
    +      var json = JSON.parse(this.editor.getValue());
           this.model.clear().set(json, {validate: true});
           if (this.model.validationError) {
             return false;
           }
    -
           return this.model;
         },
     
    -    hasValidCode: function () {
    -      var errors = this.editor.getAnnotations();
    -      return errors.length === 0;
    +    beforeRender: function () {
    +      this.uploadModal = this.setView('#upload-modal', new this.components.uploadModal({ model: this.model }));
    +      this.duplicateModal = this.setView('#duplicate-modal', new this.components.duplicateDocModal({ model: this.model }));
    +      this.confirmDeleteModal = this.setView('#delete-doc-modal', new this.components.confirmModal({
    +        text: 'Are you sure you want to delete this document?',
    +        action: this.deleteDocument
    +      }));
    +
    +      // ensures it's initialized only once
    +      this.stringEditModal = this.stringEditModal || this.setView('#string-edit-modal', new Views.StringEditModal());
         },
     
    -    serialize: function () {
    -      return {
    -        doc: this.model,
    -        attachments: this.getAttachments()
    -      };
    +    updateValues: function () {
    +      if (this.model.changedAttributes()) {
    +        FauxtonAPI.addNotification({
    +          msg: 'Document saved successfully.',
    +          type: 'success',
    +          clear: true
    +        });
    +        this.editor.setValue(this.model.prettyJSON());
    +      }
         },
     
    -    getAttachments: function () {
    -      var attachments = this.model.get('_attachments');
    +    establish: function () {
    +      var promise = this.model.fetch(),
    +          deferred = $.Deferred(),
    +          that = this;
     
    -      if (!attachments) { return false; }
    +      promise.then(function () {
    +        deferred.resolve();
    +      }, function (xhr, reason, msg) {
    +        if (xhr.status === 404) {
    +          FauxtonAPI.addNotification({
    +            msg: 'The document does not exist',
    +            type: 'error',
    +            clear: true
    +          });
    +          that.goBack();
    +        }
    +        deferred.reject();
    +     });
     
    -      return _.map(attachments, function (att, key) {
    -        return {
    -          fileName: key,
    -          size: att.length,
    -          contentType: att.content_type,
    -          url: this.model.url() + '/' + app.utils.safeURLName(key)
    -        };
    -      }, this);
    +      return deferred;
         },
     
    -    afterRender: function () {
    -      var saveDoc = this.saveDoc,
    -          editor,
    -          model;
    +    hasValidCode: function () {
    +      var errors = this.editor.getAnnotations();
    +      return errors.length === 0;
    +    },
     
    +    afterRender: function () {
    +      _.bind(this.saveDoc, this);
    --- End diff --
    
    Thanks for the info! After playing around with it, it turns out it didn't need to be bound at all. Will commit shortly. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#issuecomment-68726442
  
    Hi @SoleilPhan, @seanbarclay - any word on the JSON colors for this? Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#issuecomment-67397451
  
    That's a great point @michellephung. I'll see if there's a nice way to add that. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#issuecomment-69386993
  
    @michellephung I've done some digging, but I can't think of a decent way to handle the scroll-right scenario you found. It's not like it's a show stopper, but it would be sure nice to fix. Do you mind if I don't include it with this PR, though?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22274498
  
    --- Diff: app/addons/documents/resources.js ---
    @@ -675,6 +675,21 @@ function(app, FauxtonAPI, PagingCollection) {
       });
     
     
    +  function getPreviousPage (database) {
    --- End diff --
    
    Apologies, I'd added it into Resources then realized I didn't like it there. I'll remove the duplicate.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22274246
  
    --- Diff: app/addons/documents/templates/code_editor.html ---
    @@ -12,44 +12,64 @@
     the License.
     */%>
     
    -<div id="doc">
    -  <div class="errors-container"></div>
    -   
    -<div id="doc-actions" class="nav">
    -  <div class="span3">
    +<div id="doc-editor-actions-panel">
    +  <div class="doc-actions-left">
         <button class="save-doc btn btn-success save" type="button"><i class="icon fonticon-ok-circled"></i> Save</button>
    -    <button class="btn js-back">Back</button>
    +    <div>
    +      <a href="#" class="js-back cancel-button">Cancel</a>
    +    </div>
       </div>
     
    -  <div class="span7">
    -    <button class="btn string-edit" title="Edit line" disabled="true"><i class="icon icon-edit"></i></button>  
    +  <div class="alignRight">
    +    <button class="btn string-edit" title="Edit line" disabled="true"><i class="icon icon-edit"></i></button>
    +
         <% if (attachments) { %>
    -    <div class="btn-group">
    -      <button class="dropdown-toggle btn" data-bypass="true" data-toggle="dropdown">
    -        View Attachments
    +    <div class="panelSection btn-group">
    +      <button class="panelButton dropdown-toggle btn" data-bypass="true" data-toggle="dropdown" title="View Attachments"
    +        id="view-attachments-menu">
    +        <i class="icon fonticon-picture"></i>
    +        <span>View Attachments</span>
             <span class="caret"></span>
           </button>
    -      <ul class="dropdown-menu">
    +
    +      <ul class="dropdown-menu" role="menu" aria-labelledby="view-attachments-menu">
             <%_.each(attachments, function (att) { %>
             <li>
    -        <a href="<%- att.url %>" target="_blank" data-bypass="true"> <strong> <%- att.fileName %> </strong> -
    -          <span> <%- att.contentType %>, <%- formatSize(att.size)%> </span>
    -        </a>
    +          <a href="<%- att.url %>" target="_blank" data-bypass="true"> <strong> <%- att.fileName %> </strong> -
    +            <span> <%- att.contentType %>, <%- formatSize(att.size)%> </span>
    +          </a>
             </li>
             <% }) %>
           </ul>
         </div>
    -    <% } %> 
    -    <button class="btn upload"><i class="icon icon-circle-arrow-up"></i> Upload Attachment</button>
    -    <button class="btn duplicate"><i class="icon icon-repeat"></i> Clone document</button>
    -    <button class="btn btn-danger delete"><i class="icon icon-trash"></i></button>
    -  </div>
    +    <% } %>
     
    -<div id="upload-modal"> </div>
    -<div id="duplicate-modal"> </div> 
    -<div id="string-edit-modal"> </div> 
    --- End diff --
    
    please add it back, we should stay feature complete


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by michellephung <gi...@git.apache.org>.
Github user michellephung commented on the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#issuecomment-69813944
  
    nice job on this btw. :D 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22095449
  
    --- Diff: app/addons/documents/templates/code_editor.html ---
    @@ -12,44 +12,64 @@
     the License.
     */%>
     
    -<div id="doc">
    -  <div class="errors-container"></div>
    -   
    -<div id="doc-actions" class="nav">
    -  <div class="span3">
    +<div id="doc-editor-actions-panel">
    +  <div class="doc-actions-left">
         <button class="save-doc btn btn-success save" type="button"><i class="icon fonticon-ok-circled"></i> Save</button>
    -    <button class="btn js-back">Back</button>
    +    <div>
    +      <a href="#" class="js-back cancel-button">Cancel</a>
    +    </div>
       </div>
     
    -  <div class="span7">
    -    <button class="btn string-edit" title="Edit line" disabled="true"><i class="icon icon-edit"></i></button>  
    +  <div class="alignRight">
    +    <button class="btn string-edit" title="Edit line" disabled="true"><i class="icon icon-edit"></i></button>
    +
         <% if (attachments) { %>
    -    <div class="btn-group">
    -      <button class="dropdown-toggle btn" data-bypass="true" data-toggle="dropdown">
    -        View Attachments
    +    <div class="panelSection btn-group">
    +      <button class="panelButton dropdown-toggle btn" data-bypass="true" data-toggle="dropdown" title="View Attachments"
    +        id="view-attachments-menu">
    +        <i class="icon fonticon-picture"></i>
    +        <span>View Attachments</span>
             <span class="caret"></span>
           </button>
    -      <ul class="dropdown-menu">
    +
    +      <ul class="dropdown-menu" role="menu" aria-labelledby="view-attachments-menu">
             <%_.each(attachments, function (att) { %>
             <li>
    -        <a href="<%- att.url %>" target="_blank" data-bypass="true"> <strong> <%- att.fileName %> </strong> -
    -          <span> <%- att.contentType %>, <%- formatSize(att.size)%> </span>
    -        </a>
    +          <a href="<%- att.url %>" target="_blank" data-bypass="true"> <strong> <%- att.fileName %> </strong> -
    +            <span> <%- att.contentType %>, <%- formatSize(att.size)%> </span>
    +          </a>
             </li>
             <% }) %>
           </ul>
         </div>
    -    <% } %> 
    -    <button class="btn upload"><i class="icon icon-circle-arrow-up"></i> Upload Attachment</button>
    -    <button class="btn duplicate"><i class="icon icon-repeat"></i> Clone document</button>
    -    <button class="btn btn-danger delete"><i class="icon icon-trash"></i></button>
    -  </div>
    +    <% } %>
     
    -<div id="upload-modal"> </div>
    -<div id="duplicate-modal"> </div> 
    -<div id="string-edit-modal"> </div> 
    --- End diff --
    
    Seems like the string-edit-modal has been completely removed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22274235
  
    --- Diff: app/addons/documents/routes-doc-editor.js ---
    @@ -12,22 +12,22 @@
     
     define([
       "app",
    -
       "api",
     
       // Modules
    -  //views
    +  "addons/documents/helpers",
       "addons/documents/views",
       "addons/documents/views-doceditor",
       "addons/databases/base"
     ],
     
    -function(app, FauxtonAPI, Documents, DocEditor, Databases) {
    +function(app, FauxtonAPI, Helpers, Documents, DocEditor, Databases) {
    +
     
       var DocEditorRouteObject = FauxtonAPI.RouteObject.extend({
    -    layout: "one_pane",
    +    layout: 'doc_editor',
    --- End diff --
    
    please separate styling-commits from feature-commits, there is a lot of diff noise that makes it hard to review.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Updating edit doc page

Posted by seanbarclay <gi...@git.apache.org>.
Github user seanbarclay commented on the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#issuecomment-67378203
  
    FYI- @SoleilPhan is looking into updating the colors on the JSON to be more consistent with the API ref she's working on. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request: Update full page document editor

Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/204#discussion_r22097446
  
    --- Diff: app/addons/documents/views-doceditor.js ---
    @@ -505,35 +426,68 @@ function (app, FauxtonAPI, Components, Documents, Databases, prettify) {
                 keyChecked.push('_rev');
               }
     
    -          //check the changedDoc has all the required standard keys
    -          if (_.isEmpty(_.difference(keyChecked, _.keys(changedDoc)))) { return; }
    +          // check the changedDoc has all the required standard keys
    +          if (_.isEmpty(_.difference(keyChecked, _.keys(changedDoc)))) {
    +            return;
    +          }
     
               editor.setReadOnly(true);
               setTimeout(function () { editor.setReadOnly(false) ;}, 400);
    +
               // use extend so that _id stays at the top of the object with displaying the doc
               changedDoc = _.extend({_id: model.id, _rev: model.get('_rev')}, changedDoc);
               editor.setValue(JSON.stringify(changedDoc, null, '  '));
               FauxtonAPI.addNotification({
                 type: 'error',
    -            msg: 'Cannot remove a documents Id or Revision.',
    -            clear:  true
    +            msg: 'Cannot remove a document\'s id or revision.',
    +            clear: true
               });
             });
     
    -        var showHideEditDocString = _.bind(this.showHideEditDocString, this);
    -
    -        this.listenTo(editor.editor, 'changeSelection', function (event) {
    -          showHideEditDocString(event);
    -        });
    -        this.listenTo(editor.editor.session, 'changeBackMarker', function (event) {
    -          showHideEditDocString(event);
    -        });
    +        // place focus on the editor
    +        editor.editor.focus();
     
           }.bind(this));
    +
    +      // this sucks having to reference #dashboard, but when user clicks in the region below the editor I want it
    +      // to focus on the final line. It feels natural that way
    +      $('.scrollable').on('click', function(e) {
    +        var clickedInEditor = $(e.target).closest('#editor-container');
    +        if (clickedInEditor.length === 0) {
    +          editor.editor.focus();
    +          var session = editor.editor.getSession();
    +          var count = session.getLength();
    +          editor.editor.gotoLine(count, session.getLine(count-1).length);
    +        }
    +      });
    +    },
    +
    +    serialize: function () {
    +      return {
    +        doc: this.model,
    +        attachments: this.getAttachments()
    +      };
    +    },
    +
    +    getAttachments: function () {
    +      var attachments = this.model.get('_attachments');
    +      if (!attachments) { return false; }
    +
    +      return _.map(attachments, function (att, key) {
    +        return {
    +          fileName: key,
    +          size: att.length,
    +          contentType: att.content_type,
    +          url: this.model.url() + '/' + app.utils.safeURLName(key)
    +        };
    +      }, this);
         },
     
         cleanup: function () {
    -      if (this.editor) this.editor.remove();
    +      if (this.editor) {
    --- End diff --
    
    This is cool but you can one-line this `this.editor && this.editor.remove()`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---