Fixed comments undo/redo

bower.json

@@ -27,6 +27,7 @@
     "prism": "gh-pages",
     "MutationObservers": "https://github.com/Polymer/MutationObservers.git#~0.2.1",
     "rangy": "~1.2.3",
-    "google-diff-match-patch-js": "~1.0.0"
+    "google-diff-match-patch-js": "~1.0.0",
+    "jsondiffpatch": "~0.1.5"
   }
 }

public/res/classes/FileDescriptor.js

@@ -86,6 +86,15 @@ define([
                 storage[this.fileIndex + ".discussionList"] = JSON.stringify(discussionList);
             }
         });
+        Object.defineProperty(this, 'discussionListJSON', {
+            get: function() {
+                return storage[this.fileIndex + ".discussionList"];
+            },
+            set: function(discussionList) {
+                this._discussionList = JSON.parse(discussionList);
+                storage[this.fileIndex + ".discussionList"] = discussionList;
+            }
+        });
     }
 
     FileDescriptor.prototype.addSyncLocation = function(syncAttributes) {

public/res/editor.js

@@ -6,11 +6,23 @@ define([
     'eventMgr',
     'prism-core',
     'diff_match_patch_uncompressed',
+    'jsondiffpatch',
     'crel',
     'MutationObservers',
     'libs/prism-markdown'
-], function ($, _, settings, eventMgr, Prism, diff_match_patch, crel) {
+], function ($, _, settings, eventMgr, Prism, diff_match_patch, jsondiffpatch, crel) {
     var diffMatchPatch = new diff_match_patch();
+    var jsonDiffPatch = jsondiffpatch.create({
+        objectHash: function(obj) {
+            return JSON.stringify(obj);
+        },
+        arrays: {
+            detectMove: false,
+        },
+        textDiff: {
+            minLength: 9999999
+        }
+    });
 
     function strSplice(str, i, remove, add) {
         remove = +remove || 0;
@@ -63,6 +75,17 @@ define([
         fileDesc = selectedFileDesc;
     });
 
+    var contentObserver;
+    function noWatch(cb) {
+        contentObserver.disconnect();
+        cb();
+        contentObserver.observe(editor.contentElt, {
+            childList: true,
+            subtree: true,
+            characterData: true
+        });
+    }
+
     var previousTextContent;
     var currentMode;
     editor.undoManager = (function() {
@@ -81,13 +104,9 @@ define([
         };
         undoManager.setMode = function() {}; // For compatibility with PageDown
         undoManager.saveState = function() {
-            if(currentMode == 'undoredo') {
-                currentMode = undefined;
-                return;
-            }
             redoStack = [];
             var currentTime = Date.now();
-            if((currentMode != lastMode && lastMode != 'newlines') || currentTime - lastTime > 1000) {
+            if(currentMode == 'comment' || (currentMode != lastMode && lastMode != 'newlines') || currentTime - lastTime > 1000) {
                 undoStack.push(currentState);
                 // Limit the size of the stack
                 if(undoStack.length === 100) {
@@ -104,7 +123,7 @@ define([
                 selectionStartAfter: selectionStart,
                 selectionEndAfter: selectionEnd,
                 content: previousTextContent,
-                discussionList: JSON.stringify(fileDesc.discussionList)
+                discussionListJSON: fileDesc.discussionListJSON
             };
             lastTime = currentTime;
             lastMode = currentMode;
@@ -124,13 +143,42 @@ define([
             return redoStack.length;
         };
         function restoreState(state, selectionStart, selectionEnd) {
-            currentMode = 'undoredo';
+            // Update editor
+            noWatch(function() {
+                if(previousTextContent != state.content) {
                     inputElt.value = state.content;
-            fileDesc.discussionList = JSON.parse(state.discussionList);
+                    fileDesc.content = state.content;
+                    eventMgr.onContentChanged(fileDesc, state.content);
+                    previousTextContent = state.content;
+                }
+                inputElt.setSelectionStartEnd(selectionStart, selectionEnd);
+                var discussionListJSON = fileDesc.discussionListJSON;
+                if(discussionListJSON != state.discussionListJSON) {
+                    currentMode = 'undoredo'; // In order to avoid saveState
+                    var oldDiscussionList = fileDesc.discussionList;
+                    fileDesc.discussionListJSON = state.discussionListJSON;
+                    var newDiscussionList = fileDesc.discussionList;
+                    var diff = jsonDiffPatch.diff(oldDiscussionList, newDiscussionList);
+                    var commentsChanged = false;
+                    _.each(diff, function(discussionDiff, discussionIndex) {
+                        if(!_.isArray(discussionDiff)) {
+                            commentsChanged = true;
+                        }
+                        else if(discussionDiff.length === 1) {
+                            eventMgr.onDiscussionCreated(fileDesc, newDiscussionList[discussionIndex]);
+                        }
+                        else {
+                            eventMgr.onDiscussionRemoved(fileDesc, oldDiscussionList[discussionIndex]);
+                        }
+                    });
+                    commentsChanged && eventMgr.onCommentsChanged(fileDesc);
+                }
+            });
+
             selectionStartBefore = selectionStart;
             selectionEndBefore = selectionEnd;
-            inputElt.setSelectionStartEnd(selectionStart, selectionEnd);
             currentState = state;
+            currentMode = undefined;
             lastMode = undefined;
             undoManager.onButtonStateChange();
             adjustCursorPosition();
@@ -160,7 +208,7 @@ define([
                 selectionStartAfter: fileDesc.selectionStart,
                 selectionEndAfter: fileDesc.selectionEnd,
                 content: content,
-                discussionList: JSON.stringify(fileDesc.discussionList)
+                discussionListJSON: fileDesc.discussionListJSON
             };
             currentMode = undefined;
             lastMode = undefined;
@@ -169,6 +217,16 @@ define([
         return undoManager;
     })();
 
+    function onComment() {
+        if(!currentMode) {
+            currentMode = 'comment';
+            editor.undoManager.saveState();
+        }
+    }
+    eventMgr.addListener('onDiscussionCreated', onComment);
+    eventMgr.addListener('onDiscussionRemoved', onComment);
+    eventMgr.addListener('onCommentsChanged', onComment);
+
     function saveSelectionState() {
         if(fileChanged === false) {
             var selection = window.getSelection();
@@ -208,12 +266,12 @@ define([
             if(!/\n$/.test(currentTextContent)) {
                 currentTextContent += '\n';
             }
-            if(currentMode != 'undoredo') {
+            currentMode = currentMode || 'typing';
             var changes = diffMatchPatch.diff_main(previousTextContent, currentTextContent);
             // Move comments according to changes
             var updateDiscussionList = false;
             var startOffset = 0;
-                var discussionList = _.map(fileDesc.discussionList, _.identity);
+            var discussionList = _.values(fileDesc.discussionList);
             fileDesc.newDiscussion && discussionList.push(fileDesc.newDiscussion);
             changes.forEach(function(change) {
                 var changeType = change[0];
@@ -228,7 +286,7 @@ define([
                     endOffset += diffOffset;
                     diffOffset = -diffOffset;
                 }
-                    _.each(discussionList, function(discussion) {
+                discussionList.forEach(function(discussion) {
                     // selectionEnd
                     if(discussion.selectionEnd >= endOffset) {
                         discussion.selectionEnd += diffOffset;
@@ -252,16 +310,10 @@ define([
             });
             if(updateDiscussionList === true) {
                 fileDesc.discussionList = fileDesc.discussionList; // Write discussionList in localStorage
-                    eventMgr.onCommentsChanged(fileDesc);
-                }
-            }
-            else {
-                // Comments have been restored by undo/redo
-                eventMgr.onCommentsChanged(fileDesc);
             }
             fileDesc.content = currentTextContent;
             eventMgr.onContentChanged(fileDesc, currentTextContent);
-            currentMode = currentMode || 'typing';
+            updateDiscussionList && eventMgr.onCommentsChanged(fileDesc);
             previousTextContent = currentTextContent;
             editor.undoManager.saveState();
         }
@@ -429,7 +481,6 @@ define([
     }, 0);
     eventMgr.addListener('onLayoutResize', adjustCursorPosition);
 
-    var contentObserver;
     editor.init = function(elt1, elt2) {
         inputElt = elt1;
         $inputElt = $(inputElt);
@@ -524,6 +575,8 @@ define([
         inputElt.setSelectionStartEnd = function (start, end) {
             selectionStart = start;
             selectionEnd = end;
+            fileDesc.editorStart = selectionStart;
+            fileDesc.editorEnd = selectionEnd;
             var range = inputElt.createRange(start, end);
             var selection = window.getSelection();
             selection.removeAllRanges();
@@ -750,7 +803,7 @@ define([
             highlight(section);
             newSectionEltList.appendChild(section.elt);
         });
-        contentObserver.disconnect();
+        noWatch(function() {
             if(fileChanged === true) {
                 editor.contentElt.innerHTML = '';
                 editor.contentElt.appendChild(newSectionEltList);
@@ -782,10 +835,6 @@ define([
 
                 inputElt.setSelectionStartEnd(selectionStart, selectionEnd);
             }
-        contentObserver.observe(editor.contentElt, {
-            childList: true,
-            subtree: true,
-            characterData: true
         });
     }
 

public/res/eventMgr.js

@@ -211,6 +211,8 @@ define([
     addEventHook("onCursorCoordinates");
 
     // Operations on comments
+    addEventHook("onDiscussionCreated");
+    addEventHook("onDiscussionRemoved");
     addEventHook("onCommentsChanged");
 
     // Refresh twitter buttons

public/res/extensions/comments.js

@@ -54,15 +54,71 @@ define([
         setCommentEltCoordinates(newCommentElt, cursorY);
     };
 
-    var refreshId;
+    var currentContext;
+    function movePopover(commentElt) {
+        // Move popover in the margin
+        var context = currentContext;
+        context.popoverElt = document.querySelector('.comments-popover .popover:last-child');
+        var left = 0;
+        if(context.popoverElt.offsetWidth < marginElt.offsetWidth - 10) {
+            left = marginElt.offsetWidth - 10 - context.popoverElt.offsetWidth;
+        }
+        context.popoverElt.style.left = left + 'px';
+        context.popoverElt.querySelector('.arrow').style.left = (marginElt.offsetWidth - parseInt(commentElt.style.right) - commentElt.offsetWidth / 2 - left) + 'px';
+    }
+
     var cssApplier;
     var currentFileDesc;
-    var currentContext;
+    var refreshDiscussions = _.debounce(function() {
-    function refreshDiscussions() {
         if(currentFileDesc === undefined) {
             return;
         }
 
+        var author = storage['author.name'];
+        commentEltList.forEach(function(commentElt) {
+            marginElt.removeChild(commentElt);
+        });
+        commentEltList = [];
+        offsetMap = {};
+        _.each(currentFileDesc.discussionList, function(discussion) {
+            var isReplied = _.last(discussion.commentList).author != author;
+            var commentElt = crel('a', {
+                class: 'icon-comment' + (isReplied ? ' replied' : ' added')
+            });
+            commentElt.discussionIndex = discussion.discussionIndex;
+            var coordinates = inputElt.getOffsetCoordinates(discussion.selectionEnd);
+            var lineIndex = setCommentEltCoordinates(commentElt, coordinates.y);
+            offsetMap[lineIndex] = (offsetMap[lineIndex] || 0) + 1;
+            marginElt.appendChild(commentElt);
+            commentEltList.push(commentElt);
+
+            if(currentContext && currentContext.discussion == discussion) {
+                inputElt.scrollTop += parseInt(commentElt.style.top) - inputElt.scrollTop - inputElt.offsetHeight * 3 / 4;
+                movePopover(commentElt);
+            }
+        });
+
+        // Move newCommentElt
+        setCommentEltCoordinates(newCommentElt, cursorY);
+        if(currentContext && !currentContext.discussion.discussionIndex) {
+            inputElt.scrollTop += parseInt(newCommentElt.style.top) - inputElt.scrollTop - inputElt.offsetHeight * 3 / 4;
+            movePopover(newCommentElt);
+        }
+    }, 50);
+
+    comments.onFileOpen = function(fileDesc) {
+        currentFileDesc = fileDesc;
+        refreshDiscussions();
+    };
+
+    comments.onContentChanged = function(fileDesc, content) {
+        currentFileDesc === fileDesc && refreshDiscussions();
+    };
+
+    comments.onCommentsChanged = function(fileDesc) {
+        if(currentFileDesc !== fileDesc) {
+            return;
+        }
         if(currentContext !== undefined) {
             // Refresh conversation if popover is open
             var context = currentContext;
@@ -70,7 +126,10 @@ define([
                 context.discussion = currentFileDesc.discussionList[context.discussion.discussionIndex];
                 context.popoverElt.querySelector('.discussion-comment-list').innerHTML = getDiscussionComments();
             }
+            try {
                 cssApplier.undoToRange(context.rangyRange);
+            }
+            catch(e) {}
             context.selectionRange = inputElt.createRange(context.discussion.selectionStart, context.discussion.selectionEnd);
 
             // Highlight selected text
@@ -83,61 +142,28 @@ define([
                 }
             }, 50);
         }
-        
-        var author = storage['author.name'];
-        clearTimeout(refreshId);
-        commentEltList.forEach(function(commentElt) {
-            marginElt.removeChild(commentElt);
-        });
-        commentEltList = [];
-        offsetMap = {};
-        var discussionList = _.map(currentFileDesc.discussionList, _.identity);
-        function refreshOne() {
-            if(discussionList.length === 0) {
-                return;
-            }
-            var discussion = discussionList.pop();
-            var commentElt = crel('a', {
-                class: 'icon-comment'
-            });
-            commentElt.discussion = discussion;
-            var coordinates = inputElt.getOffsetCoordinates(discussion.selectionEnd);
-            var lineIndex = setCommentEltCoordinates(commentElt, coordinates.y);
-            offsetMap[lineIndex] = (offsetMap[lineIndex] || 0) + 1;
-            marginElt.appendChild(commentElt);
-            commentEltList.push(commentElt);
-
-            // Move newCommentElt
-            setCommentEltCoordinates(newCommentElt, cursorY);
-
-            // Apply class later for fade effect
-            commentElt.offsetWidth; // Refresh
-            var isReplied = _.last(discussion.commentList).author != author;
-            commentElt.className += isReplied ? ' replied' : ' added';
-            refreshId = setTimeout(refreshOne, 50);
-        }
-        refreshId = setTimeout(refreshOne, 50);
-    }
-    var debouncedRefreshDiscussions = _.debounce(refreshDiscussions, 2000);
-
-    comments.onFileOpen = function(fileDesc) {
-        currentFileDesc = fileDesc;
         refreshDiscussions();
     };
 
-    comments.onContentChanged = function(fileDesc, content) {
-        currentFileDesc === fileDesc && debouncedRefreshDiscussions();
-    };
-
-    comments.onCommentsChanged = function(fileDesc) {
-        currentFileDesc === fileDesc && refreshDiscussions();
-    };
-
     function closeCurrentPopover() {
         currentContext && currentContext.$commentElt.popover('toggle').popover('destroy');
     }
-    comments.onLayoutResize = function() {
+
+    comments.onDiscussionCreated = function(fileDesc) {
+        currentFileDesc === fileDesc && refreshDiscussions();
+    };
+
+    comments.onDiscussionRemoved = function(fileDesc, discussion) {
+        if(currentFileDesc === fileDesc) {
+            // Close popover if the discussion has removed
+            if(currentContext !== undefined && currentContext.discussion.discussionIndex == discussion.discussionIndex) {
                 closeCurrentPopover();
+            }
+            refreshDiscussions();
+        }
+    };
+
+    comments.onLayoutResize = function() {
         refreshDiscussions();
     };
 
@@ -200,8 +226,8 @@ define([
             inputElt.scrollTop += parseInt(evt.target.style.top) - inputElt.scrollTop - inputElt.offsetHeight * 3 / 4;
 
             // If it's an existing discussion
-            if(evt.target.discussion) {
+            if(evt.target.discussionIndex) {
-                context.discussion = evt.target.discussion;
+                context.discussion = currentFileDesc.discussionList[evt.target.discussionIndex];
                 context.selectionRange = inputElt.createRange(context.discussion.selectionStart, context.discussion.selectionEnd);
                 return;
             }
@@ -230,15 +256,9 @@ define([
             };
             currentFileDesc.newDiscussion = context.discussion;
         }).on('shown.bs.popover', '#wmd-input > .editor-margin', function(evt) {
-            // Move the popover in the margin
             var context = currentContext;
             context.popoverElt = document.querySelector('.comments-popover .popover:last-child');
-            var left = -5;
+            movePopover(evt.target);
-            if(context.popoverElt.offsetWidth < marginElt.offsetWidth - 5) {
-                left = marginElt.offsetWidth - 10 - context.popoverElt.offsetWidth;
-            }
-            context.popoverElt.style.left = left + 'px';
-            context.popoverElt.querySelector('.arrow').style.left = (marginElt.offsetWidth - parseInt(evt.target.style.right) - evt.target.offsetWidth / 2 - left) + 'px';
 
             // Scroll to the bottom of the discussion
             context.popoverElt.querySelector('.popover-content').scrollTop = 9999999;
@@ -270,6 +290,10 @@ define([
                 context.$contentInputElt.val('');
                 closeCurrentPopover();
 
+                context.discussion.commentList.push({
+                    author: author,
+                    content: content
+                });
                 var discussionList = context.fileDesc.discussionList || {};
                 if(!context.discussion.discussionIndex) {
                     // Create discussion index
@@ -279,18 +303,18 @@ define([
                     } while(_.has(discussionList, discussionIndex));
                     context.discussion.discussionIndex = discussionIndex;
                     discussionList[discussionIndex] = context.discussion;
+                    context.fileDesc.discussionList = discussionList; // Write discussionList in localStorage
+                    eventMgr.onDiscussionCreated(context.fileDesc, context.discussion);
                 }
-                context.discussion.commentList.push({
+                else {
-                    author: author,
-                    content: content
-                });
                     context.fileDesc.discussionList = discussionList; // Write discussionList in localStorage
                     eventMgr.onCommentsChanged(context.fileDesc);
+                }
                 inputElt.focus();
             });
 
             var $removeButton = $(context.popoverElt.querySelector('.action-remove-discussion'));
-            if(evt.target.discussion) {
+            if(evt.target.discussionIndex) {
                 // If it's an existing discussion
                 var $removeCancelButton = $(context.popoverElt.querySelector('.action-remove-discussion-cancel'));
                 var $removeConfirmButton = $(context.popoverElt.querySelector('.action-remove-discussion-confirm'));

public/res/libs/prism-markdown.js

@@ -47,7 +47,7 @@ Prism.languages.md = (function() {
     };
     for (var i = 6; i >= 1; i--) {
         md["h" + i] = {
-            pattern: new RegExp("^#{" + i + "} .*$", "gm"),
+            pattern: new RegExp("^#{" + i + "}.+$", "gm"),
             inside: {
                 "md md-hash": new RegExp("^#{" + i + "} ")
             }
@@ -197,9 +197,6 @@ Prism.languages.md = (function() {
             }
         }
     };
-    md.url = {
-        pattern: urlPattern
-    };
     md.email = {
         pattern: emailPattern
     };

public/res/main.js

@@ -59,7 +59,8 @@ requirejs.config({
         rangy: 'bower-libs/rangy/rangy-core',
         'rangy-cssclassapplier': 'bower-libs/rangy/rangy-cssclassapplier',
         diff_match_patch: 'bower-libs/google-diff-match-patch-js/diff_match_patch',
-        diff_match_patch_uncompressed: 'bower-libs/google-diff-match-patch-js/diff_match_patch_uncompressed'
+        diff_match_patch_uncompressed: 'bower-libs/google-diff-match-patch-js/diff_match_patch_uncompressed',
+        jsondiffpatch: 'bower-libs/jsondiffpatch/build/bundle'
     },
     shim: {
         underscore: {
@@ -77,6 +78,9 @@ requirejs.config({
         diff_match_patch_uncompressed: {
             exports: 'diff_match_patch'
         },
+        jsondiffpatch: [
+            'diff_match_patch_uncompressed'
+        ],
         rangy: {
             exports: 'rangy'
         },

public/res/styles/main.less

@@ -127,8 +127,8 @@
 @popover-arrow-outer-color: @secondary-border-color;
 @popover-title-bg: @transparent;
 @alert-border-radius: 0;
-@label-warning-bg: darken(@logo-yellow, 4%);
+@label-warning-bg: spin(darken(@logo-yellow, 4%), -4);
-@label-danger-bg: darken(@logo-orange, 4%);
+@label-danger-bg: spin(darken(@logo-orange, 4%), -4);
 
 
 body {
@@ -615,7 +615,7 @@ a {
 	}
 	.panel-content {
 		background-color: @list-group-bg;
-		padding-top: 200px;
+		padding-top: 210px;
         .viewer & {
             padding-top: 75px;
         }
@@ -1077,9 +1077,7 @@ a {
 					color: fade(@label-warning-bg, 80%) !important;
 				}
 			}
-			.transition(~"color ease-in-out .25s");
 			position: absolute;
-			color: fade(#fff, 0%);
 			cursor: pointer;
 			&:hover, &.active {
 				text-decoration: none;
@@ -1091,7 +1089,7 @@ a {
 	}
 
 	.comment-highlight {
-		background-color: fade(@label-warning-bg, 25%);
+		background-color: fade(@label-warning-bg, 30%);
 		//border-radius: @border-radius-base;
 	}
 
@@ -1177,7 +1175,8 @@ a {
     .h6 { font-size: 0.9em; }
 
 
-    .url,.email {
+    .url,
+	.email {
         color: @tertiary-color-light;
     }
 

public/res/themes/original.less

@@ -51,7 +51,7 @@
 @btn-success-color: #a0a0a0;
 @btn-success-hover-bg: darken(@navbar-default-bg, 10%);
 @btn-info-hover-bg: #ededed;
-@panel-button-bg-color: #e0e0e0;
+@panel-button-bg-color: #e8e8e8;
 @panel-button-box-shadow: ~"0 0 1px rgba(255,255,255,0.75)";
 @input-bg: #fff;
 @modal-backdrop-bg: #606060;

public/res/utils.js

@@ -682,5 +682,13 @@ define([
         return crc.toString(16);
     };
 
+    window.perfTest = function(cb) {
+        var startTime = Date.now();
+        for(var i=0; i<10000; i++) {
+            cb();
+        }
+        console.log('Run 10,000 times in ' + (Date.now() - startTime) + 'ms');
+    }
+
     return utils;
 });