Github user robertkowalski commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/6#discussion_r15239192
  
    --- Diff: app/addons/documents/views.js ---
    @@ -765,6 +822,57 @@ function(app, FauxtonAPI, Components, Documents, 
Databases, pouchdb,
           FauxtonAPI.navigate(this.database.url("index") + "?limit=100");
         },
     
    +    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);
    +   var editMatch = editLine.match(/^([ \t]*)("[a-zA-Z0-9_]*": )?(".*",?[ 
\t]*)$/);
    +   if (editMatch) {
    +          return editMatch;
    +   } else {
    +          return null;
    +   }
    +      } else {
    +        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 
*/
    +        this.$("button.string-edit").css("top", 
(this.$("#editor-container")[0].offsetTop - 2 + this.editor.getRowHeight() * 
this.editor.documentToScreenRow(this.editor.getSelectionStart().row)) + "px");
    --- End diff --
    
    I have several points here:
    
     - `this.$("#editor-container")[0].offsetTop`: you can use jquery here: 
`this.$("#editor-container").offset().top`
     - can we split this into two lines for a better readability, like:
    
    ```javascript
    var positionFromTop = ...
    this.$("button...
    ```
    - I am not sure what the `2` is? is it a border or padding?


---
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 [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to