[PolyGerrit] Render (read-only) comments and column width ruler. Also some small additional changes as well: + Correct some :host selectors that were causing issues when Shadow DOM was used instead of Shady DOM. + Two of the same revisions should not be shown in side-by-side view. When two are specified via the url as /N..N/ redirect to /N/. + Added header to the diff view to give context and allow the user to go back to the top-level change. Feature: Issue 3676 Feature: Issue 3648 Feature: Issue 3649 Change-Id: Ib39bb7cb2eab26740f80971d4e00d579a2e08b0b
diff --git a/polygerrit-ui/app/elements/gr-app.html b/polygerrit-ui/app/elements/gr-app.html index aa60c47..92256a8 100644 --- a/polygerrit-ui/app/elements/gr-app.html +++ b/polygerrit-ui/app/elements/gr-app.html
@@ -15,10 +15,10 @@ --> <link rel="import" href="../bower_components/polymer/polymer.html"> -<link rel="import" href="./gr-change-list-view.html"> -<link rel="import" href="./gr-change-view.html"> -<link rel="import" href="./gr-diff-view.html"> -<link rel="import" href="./gr-search-bar.html"> +<link rel="import" href="gr-change-list-view.html"> +<link rel="import" href="gr-change-view.html"> +<link rel="import" href="gr-diff-view.html"> +<link rel="import" href="gr-search-bar.html"> <dom-module id="gr-app"> <template> @@ -28,7 +28,7 @@ min-height: 100vh; flex-direction: column; } - :host[constrained] main { + :host([constrained]) main { margin: 0 auto; width: 100%; max-width: 980px;
diff --git a/polygerrit-ui/app/elements/gr-change-list-item.html b/polygerrit-ui/app/elements/gr-change-list-item.html index 5bbed82..3743337 100644 --- a/polygerrit-ui/app/elements/gr-change-list-item.html +++ b/polygerrit-ui/app/elements/gr-change-list-item.html
@@ -15,7 +15,7 @@ --> <link rel="import" href="../bower_components/polymer/polymer.html"> -<link rel="import" href="./gr-date-formatter.html"> +<link rel="import" href="gr-date-formatter.html"> <dom-module id="gr-change-list-item"> <template>
diff --git a/polygerrit-ui/app/elements/gr-change-list-view.html b/polygerrit-ui/app/elements/gr-change-list-view.html index 09c9033..2885209 100644 --- a/polygerrit-ui/app/elements/gr-change-list-view.html +++ b/polygerrit-ui/app/elements/gr-change-list-view.html
@@ -16,7 +16,7 @@ <link rel="import" href="../bower_components/polymer/polymer.html"> <link rel="import" href="../bower_components/iron-ajax/iron-ajax.html"> -<link rel="import" href="./gr-change-list.html"> +<link rel="import" href="gr-change-list.html"> <dom-module id="gr-change-list-view"> <template>
diff --git a/polygerrit-ui/app/elements/gr-change-list.html b/polygerrit-ui/app/elements/gr-change-list.html index c4f4e0c..f3f3ca2 100644 --- a/polygerrit-ui/app/elements/gr-change-list.html +++ b/polygerrit-ui/app/elements/gr-change-list.html
@@ -16,7 +16,7 @@ <link rel="import" href="../bower_components/polymer/polymer.html"> <link rel="import" href="../bower_components/iron-a11y-keys/iron-a11y-keys.html"> -<link rel="import" href="./gr-change-list-item.html"> +<link rel="import" href="gr-change-list-item.html"> <dom-module id="gr-change-list"> <template> @@ -84,7 +84,7 @@ }, _selectedIndexChanged: function(value) { - // Don’t re-render the entire list. + // Don't re-render the entire list. var changeEls = this._getNonHeaderListItems(); for (var i = 0; i < changeEls.length; i++) { changeEls[i].toggleAttribute('selected', i == value);
diff --git a/polygerrit-ui/app/elements/gr-change-view.html b/polygerrit-ui/app/elements/gr-change-view.html index 40371c1..c955210 100644 --- a/polygerrit-ui/app/elements/gr-change-view.html +++ b/polygerrit-ui/app/elements/gr-change-view.html
@@ -16,9 +16,9 @@ <link rel="import" href="../bower_components/polymer/polymer.html"> <link rel="import" href="../bower_components/iron-ajax/iron-ajax.html"> -<link rel="import" href="./gr-date-formatter.html"> -<link rel="import" href="./gr-file-list.html"> -<link rel="import" href="./gr-messages-list.html"> +<link rel="import" href="gr-date-formatter.html"> +<link rel="import" href="gr-file-list.html"> +<link rel="import" href="gr-messages-list.html"> <dom-module id="gr-change-view"> <template> @@ -61,16 +61,17 @@ url="[[_computeDetailPath(changeNum)]]" params="[[_computeDetailQueryParams()]]" json-prefix=")]}'" - last-response="{{change}}" - debounce-duration="300"></iron-ajax> + last-response="{{change}}"></iron-ajax> <iron-ajax id="commentsXHR" url="[[_computeCommentsPath(changeNum)]]" json-prefix=")]}'" - last-response="{{comments}}" - debounce-duration="300"></iron-ajax> + last-response="{{comments}}"></iron-ajax> <div class="container"> - <h2>[[change.subject]]</h2> + <h2> + <a href$="[[_computeChangePath(change._number)]]">[[change._number]]</a><span>:</span> + <span>[[change.subject]]</span> + </h2> <div class="changeInfo"> <table> <tr> @@ -152,6 +153,10 @@ this.$.commentsXHR.generateRequest(); }, + _computeChangePath: function(changeNum) { + return '/c/' + changeNum; + }, + _computeDetailPath: function(changeNum) { return '/changes/' + changeNum + '/detail'; },
diff --git a/polygerrit-ui/app/elements/gr-diff-comment-thread.html b/polygerrit-ui/app/elements/gr-diff-comment-thread.html new file mode 100644 index 0000000..b908cf7 --- /dev/null +++ b/polygerrit-ui/app/elements/gr-diff-comment-thread.html
@@ -0,0 +1,111 @@ +<!-- +Copyright (C) 2015 The Android Open Source Project + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +--> + +<link rel="import" href="../bower_components/polymer/polymer.html"> +<link rel="import" href="gr-diff-comment.html"> + +<dom-module id="gr-diff-comment-thread"> + <template> + <style> + :host { + display: block; + max-width: 50em; + white-space: normal; + } + </style> + <template id="commentList" is="dom-repeat" items="{{_orderedComments}}" as="comment"> + <gr-diff-comment comment="[[comment]]"></gr-diff-comment> + </template> + </template> + <script> + (function() { + 'use strict'; + + + Polymer({ + is: 'gr-diff-comment-thread', + + /** + * Fired when the height of the thread changes. + * + * @event gr-diff-comment-thread-height-changed + */ + + properties: { + comments: { + type: Array, + observer: '_commentsChanged', + }, + _orderedComments: Array, + }, + + ready: function() { + this.addEventListener('gr-diff-comment-height-changed', + this._handleCommentHeightChange.bind(this)); + }, + + _commentsChanged: function(comments) { + this._orderedComments = this._sortedComments(comments); + }, + + _sortedComments: function(comments) { + var comments = comments || []; + comments.sort(function(c1, c2) { + return util.parseDate(c1.updated) - util.parseDate(c2.updated); + }); + + var commentIDToReplies = {}; + var topLevelComments = []; + for (var i = 0; i < comments.length; i++) { + var c = comments[i]; + if (c.in_reply_to) { + if (commentIDToReplies[c.in_reply_to] == null) { + commentIDToReplies[c.in_reply_to] = []; + } + commentIDToReplies[c.in_reply_to].push(c); + } else { + topLevelComments.push(c); + } + } + var results = []; + for (var i = 0; i < topLevelComments.length; i++) { + this._visitComment(topLevelComments[i], commentIDToReplies, results); + } + return results; + }, + + _visitComment: function(parent, commentIDToReplies, results) { + results.push(parent); + + var replies = commentIDToReplies[parent.id]; + if (!replies) { return; } + for (var i = 0; i < replies.length; i++) { + this._visitComment(replies[i], commentIDToReplies, results); + } + }, + + _handleCommentHeightChange: function(e) { + // TODO: This fires for each comment on initialization. Optimize to only + // fire the top level "thread height has changed" event once during + // initial DOM stamp. + this.fire('gr-diff-comment-thread-height-changed', + {height: this.offsetHeight}); + }, + + }); + })(); + </script> +</dom-module>
diff --git a/polygerrit-ui/app/elements/gr-diff-comment.html b/polygerrit-ui/app/elements/gr-diff-comment.html new file mode 100644 index 0000000..343f7d3 --- /dev/null +++ b/polygerrit-ui/app/elements/gr-diff-comment.html
@@ -0,0 +1,113 @@ +<!-- +Copyright (C) 2015 The Android Open Source Project + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +--> + +<link rel="import" href="../bower_components/polymer/polymer.html"> +<link rel="import" href="gr-date-formatter.html"> + +<dom-module id="gr-diff-comment"> + <template> + <style> + :host { + border: 1px solid #ddd; + display: block; + } + .header, + .message, + .actions { + padding: .5em .7em; + } + .header { + background-color: #eee; + font-family: 'Open Sans', sans-serif; + } + gr-date-formatter { + float: right; + margin-left: 5px; + } + .authorName { + font-weight: bold; + } + .message { + white-space: pre-wrap; + } + .actions { + /** TODO: remove once the actions actually do something. **/ + display: none; + padding-top: 0; + } + </style> + <div class="header" id="header"> + <span class="authorName">[[comment.author.name]]</span> + <gr-date-formatter date-str="[[comment.updated]]"></gr-date-formatter> + </div> + <div class="message">[[comment.message]]</div> + <div class="actions"> + <a class="reply" href="#" on-tap="_handleReply">Reply</a> + <a class="done" href="#" on-tap="_handleDone">Done</a> + </div> + </template> + <script> + (function() { + 'use strict'; + + Polymer({ + is: 'gr-diff-comment', + + /** + * Fired when the height of the comment changes. + * + * @event gr-diff-comment-height-changed + */ + + /** + * Fired when the Reply action is triggered. + * + * @event gr-diff-comment-reply + */ + + /** + * Fired when the Done action is triggered. + * + * @event gr-diff-comment-done + */ + + properties: { + comment: Object, + draft: { + type: Boolean, + value: false, + }, + }, + + attached: function() { + this.fire('gr-diff-comment-height-changed', + {height: this.offsetHeight}); + }, + + _handleReply: function(e) { + e.preventDefault(); + this.fire('gr-diff-comment-reply'); + }, + + _handleDone: function(e) { + e.preventDefault(); + this.fire('gr-diff-comment-done'); + }, + + }); + })(); + </script> +</dom-module>
diff --git a/polygerrit-ui/app/elements/gr-diff-view.html b/polygerrit-ui/app/elements/gr-diff-view.html index f532948..3fdef07 100644 --- a/polygerrit-ui/app/elements/gr-diff-view.html +++ b/polygerrit-ui/app/elements/gr-diff-view.html
@@ -15,6 +15,7 @@ --> <link rel="import" href="../bower_components/polymer/polymer.html"> +<link rel="import" href="gr-diff-comment-thread.html"> <dom-module id="gr-diff-view"> <template> @@ -22,6 +23,16 @@ :host { display: block; } + h3 { + border-bottom: 1px solid #ddd; + margin: 0 20px 20px 20px; + padding-bottom: 10px; + } + .mainContainer { + margin: 0 auto; + max-width: calc(100% - 40px); + overflow: auto; + } .diffContainer { display: flex; font-family: 'Source Code Pro', monospace; @@ -34,16 +45,28 @@ text-align: right; } .diffContent { + min-width: 80ch; padding-left: 2px; } + .ruler { + display: block; + background-color: #ddd; + height: 1.3em; + position: absolute; + top: 0; + width: 1px; + } .lineNum:before, .content:before { /* To ensure the height is non-zero in these elements, a zero-width space is set as its content. The character - itself doesn’t matter. Just that there is something + itself doesn't matter. Just that there is something there. */ content: '\200B'; } + .content { + position: relative; + } .lineNum.blank { border-right: 2px solid #F34D4D; margin-right: 3px; @@ -78,13 +101,28 @@ id="diffXHR" url="[[_computeDiffPath(_changeNum, _patchNum, _path)]]" json-prefix=")]}'" - on-response="_handleDiffResponse" - debounce-duration="300"></iron-ajax> - <div class="diffContainer" id="diffContainer"> - <div class="diffNumbers" id="leftDiffNumbers"></div> - <div class="diffContent" id="leftDiffContent"></div> - <div class="diffNumbers" id="rightDiffNumbers"></div> - <div class="diffContent" id="rightDiffContent"></div> + on-response="_handleDiffResponse"></iron-ajax> + <iron-ajax + id="leftCommentsXHR" + url="[[_computeCommentsPath(_changeNum, _basePatchNum)]]" + json-prefix=")]}'" + on-response="_handleLeftCommentsResponse"></iron-ajax> + <iron-ajax + id="rightCommentsXHR" + url="[[_computeCommentsPath(_changeNum, _patchNum)]]" + json-prefix=")]}'" + on-response="_handleRightCommentsResponse"></iron-ajax> + <h3> + <a href$="[[_computeChangePath(_changeNum)]]">[[_changeNum]]</a><span>:</span> + <span>[[_change.subject]]</span> + </h3> + <div class="mainContainer"> + <div class="diffContainer" id="diffContainer"> + <div class="diffNumbers" id="leftDiffNumbers"></div> + <div class="diffContent" id="leftDiffContent"></div> + <div class="diffNumbers" id="rightDiffNumbers"></div> + <div class="diffContent" id="rightDiffContent"></div> + </div> </div> </template> <script> @@ -102,11 +140,24 @@ type: Object, observer: '_paramsChanged', }, + rulerWidth: { + type: Number, + value: 80, + observer: '_rulerWidthChanged', + }, _change: Object, _changeNum: String, + _diff: Object, _basePatchNum: String, _patchNum: String, _path: String, + _leftComments: Array, + _rightComments: Array, + _rendered: Boolean, + }, + + listeners: { + 'diffContainer.tap': '_diffContainerTapHandler' }, _paramsChanged: function(value) { @@ -114,17 +165,48 @@ this._patchNum = value.patchNum; this._basePatchNum = value.basePatchNum; this._path = value.path; - if (!this._changeNum) { + if (!this._patchNum) { this._change = null; this._basePatchNum = null; this._patchNum = null; + this._diff = null; this._path = null; + this._leftComments = null; + this._rightComments = null; + this._rendered = false; return; } // Assign the params here since a computed binding relying on - // `_basePatchNum` won’t fire in the case where it’s not defined. - this.$.diffXHR.params = this._diffQueryParams(); + // `_basePatchNum` won't fire in the case where it's not defined. + this.$.diffXHR.params = this._diffQueryParams(this._basePatchNum); this.$.diffXHR.generateRequest(); + + if (this._basePatchNum) { + this.$.leftCommentsXHR.generateRequest(); + } + this.$.rightCommentsXHR.generateRequest(); + }, + + _rulerWidthChanged: function(newValue, oldValue) { + if (newValue < 0) { + throw Error('ruler width must be greater than zero.'); + } + if (oldValue == 0) { + this._renderRulerElements(); + } + var remove = newValue == 0; + var rulerEls = document.querySelectorAll('.ruler'); + for (var i = 0; i < rulerEls.length; i++) { + if (remove) { + rulerEls[i].parentNode.removeChild(rulerEls[i]); + } else { + rulerEls[i].style.left = newValue + 'ch'; + } + } + }, + + _computeChangePath: function(changeNum) { + return '/c/' + changeNum; }, _computeChangeDetailPath: function(changeNum) { @@ -143,6 +225,10 @@ encodeURIComponent(path) + '/diff'; }, + _computeCommentsPath: function(changeNum, patchNum) { + return '/changes/' + changeNum + '/revisions/' + patchNum + '/comments'; + }, + _diffQueryParams: function(basePatchNum) { var params = { context: 'ALL', @@ -154,15 +240,114 @@ return params; }, - _handleDiffResponse: function(e, req) { - var diff = e.detail.response; - this._constructDOM(diff); + _diffContainerTapHandler: function(e) { + var el = e.detail.sourceEvent.target; + if (el.classList.contains('lineNum')) { + // TODO: Implement adding draft comments. + } }, - _constructDOM: function(diff) { - if (!diff.content) { return; } + _handleLeftCommentsResponse: function(e, req) { + this._leftComments = e.detail.response[this._path] || []; + this._maybeRenderDiff(this._diff, this._leftComments, + this._rightComments); + }, + + _handleRightCommentsResponse: function(e, req) { + this._rightComments = e.detail.response[this._path] || []; + this._maybeRenderDiff(this._diff, this._leftComments, + this._rightComments); + }, + + _handleDiffResponse: function(e, req) { + this._diff = e.detail.response; + this._maybeRenderDiff(this._diff, this._leftComments, + this._rightComments); + }, + + _threadID: function(patchNum, lineNum) { + return 'thread-' + patchNum + '-' + lineNum; + }, + + _renderComments: function(comments, patchNum) { + // Group the comments by line number. Absense of a line number indicates + // a top-level file comment. + var threads = {}; + + for (var i = 0; i < comments.length; i++) { + var line = comments[i].line || 'FILE'; + if (threads[line] == null) { + threads[line] = [] + } + threads[line].push(comments[i]); + } + for (var lineNum in threads) { + this._addThread(threads[lineNum], patchNum, lineNum); + } + }, + + _addThread: function(comments, patchNum, lineNum) { + var el = document.createElement('gr-diff-comment-thread'); + el.comments = comments; + var threadID = this._threadID(patchNum, lineNum); + el.setAttribute('data-thread-id', threadID); + + // Find the element that the thread should be appended after. In the + // case of a file comment, it will be appended after the first line. + // TODO: Show file comment above the file itself. + var fileComment = lineNum == 'FILE'; + if (fileComment) { + lineNum = 1; + } + var contentEl = this.$$('.content' + + '[data-patch-num="' + patchNum + '"]' + + '[data-line-num="' + lineNum + '"]'); + var rowNum = contentEl.getAttribute('data-row-num'); + el.addEventListener('gr-diff-comment-thread-height-changed', + this._handleCommentThreadHeightChange.bind(this, rowNum, threadID)); + + Polymer.dom(contentEl.parentNode).insertBefore( + el, contentEl.nextSibling); + }, + + _handleCommentThreadHeightChange: function(rowNum, threadID, e) { + // Adjust the filler element heights if they're present in the DOM. + var els = this.querySelectorAll( + '.js-threadFiller[data-thread-id="' + threadID + '"]'); + if (els.length > 0) { + for (var i = 0; i < els.length; i++) { + els[i].style.height = e.detail.height + 'px'; + } + return; + } + + // Create the filler elements if they're not already present. + var els = this.querySelectorAll('[data-row-num="' + rowNum + '"]'); + for (var i = 0; i < els.length; i++) { + // Is this is the side with the comment? Skip if so. + if (els[i].nextSibling && + els[i].nextSibling.tagName == 'GR-DIFF-COMMENT-THREAD') { + continue; + } + var fillerEl = document.createElement('div'); + fillerEl.setAttribute('data-thread-id', threadID); + fillerEl.className = 'js-threadFiller'; + fillerEl.style.height = e.detail.height + 'px'; + Polymer.dom(els[i].parentNode).insertBefore( + fillerEl, els[i].nextSibling); + } + }, + + _maybeRenderDiff: function(diff, leftComments, rightComments) { + if (this._rendered) { + throw Error('diff has already been rendered'); + } + if (!diff || !diff.content) { return; } + if (this._basePatchNum && leftComments == null) { return; } + if (rightComments == null) { return; } var initialLineNum = 0 + (diff.content.skip || 0); var ctx = { + rowNum: 0, left: { lineNum: initialLineNum, content: '', @@ -177,6 +362,19 @@ for (var i = 0; i < diff.content.length; i++) { this._addDiffChunk(ctx, diff.content[i]); } + + if (leftComments) { + this._renderComments(leftComments, this._basePatchNum); + } + if (rightComments) { + this._renderComments(rightComments, this._patchNum); + } + + if (this.rulerWidth) { + this._renderRulerElements(); + } + + this._rendered = true; }, _addDiffChunk: function(ctx, diffChunk) { @@ -224,32 +422,41 @@ }, _addRow: function(ctx) { - var leftLineNumEl = document.createElement('div'); - var rightLineNumEl = document.createElement('div'); - var leftColEl = document.createElement('div'); - var rightColEl = document.createElement('div'); + var leftLineNumEl = this._createElement('div', 'lineNum'); + var leftColEl = this._createElement('div', 'content'); + var rightLineNumEl = this._createElement('div', 'lineNum'); + var rightColEl = this._createElement('div', 'content'); - // These classes are added to account for Polymer’s polyfill behavior. - // In order to guarantee sufficient specificity within the CSS rules, - // these are added to every element. Since the Polymer DOM utility - // functions (which would do this automatically) are not being used for - // performance reasons, this is done manually. - leftColEl.className = 'style-scope gr-diff-view content'; - rightColEl.className = 'style-scope gr-diff-view content'; - leftLineNumEl.className = 'style-scope gr-diff-view lineNum'; - rightLineNumEl.className = 'style-scope gr-diff-view lineNum'; + [leftColEl, + rightColEl, + leftLineNumEl, + rightLineNumEl].forEach(function(el) { + el.setAttribute('data-row-num', ctx.rowNum); + }); - if (ctx.left.content != undefined) { + var self = this; + if (this._basePatchNum) { + [leftLineNumEl, leftColEl].forEach(function(el) { + el.setAttribute('data-patch-num', self._basePatchNum); + }); + } + [rightLineNumEl, rightColEl].forEach(function(el) { + el.setAttribute('data-patch-num', self._patchNum); + }); + + if (ctx.left.content != null) { leftLineNumEl.textContent = ctx.left.lineNum; - leftLineNumEl.id = 'L' + ctx.left.lineNum; - leftColEl.id = 'LC' + ctx.left.lineNum; + [leftLineNumEl, leftColEl].forEach(function(el) { + el.setAttribute('data-line-num', ctx.left.lineNum); + }); } else { leftLineNumEl.classList.add('blank'); } - if (ctx.right.content != undefined) { + if (ctx.right.content != null) { rightLineNumEl.textContent = ctx.right.lineNum; - rightLineNumEl.id = 'R' + ctx.right.lineNum; - rightColEl.id = 'RC' + ctx.right.lineNum; + [rightLineNumEl, rightColEl].forEach(function(el) { + el.setAttribute('data-line-num', ctx.right.lineNum); + }); } else { rightLineNumEl.classList.add('blank'); } @@ -287,8 +494,29 @@ this.$.leftDiffContent.appendChild(leftColEl); this.$.rightDiffNumbers.appendChild(rightLineNumEl); this.$.rightDiffContent.appendChild(rightColEl); + + ctx.rowNum++; }, + _renderRulerElements: function() { + var contentEls = this.querySelectorAll('.content'); + for (var i = 0; i < contentEls.length; i++) { + var rulerEl = this._createElement('i', 'ruler'); + rulerEl.style.left = this.rulerWidth + 'ch'; + contentEls[i].appendChild(rulerEl); + } + }, + + _createElement: function(tagName, className) { + var el = document.createElement(tagName); + // These classes are added to account for Polymer's polyfill behavior. + // In order to guarantee sufficient specificity within the CSS rules, + // these are added to every element. Since the Polymer DOM utility + // functions (which would do this automatically) are not being used for + // performance reasons, this is done manually. + el.className = 'style-scope gr-diff-view ' + className; + return el; + }, }); })(); </script>
diff --git a/polygerrit-ui/app/elements/gr-file-list.html b/polygerrit-ui/app/elements/gr-file-list.html index 1a5dcf0..1b0e932 100644 --- a/polygerrit-ui/app/elements/gr-file-list.html +++ b/polygerrit-ui/app/elements/gr-file-list.html
@@ -46,8 +46,7 @@ <iron-ajax id="xhr" url="[[_computeFilesURL(changeNum, revision)]]" json-prefix=")]}'" - on-response="_handleResponse" - debounce-duration="300"></iron-ajax> + on-response="_handleResponse"></iron-ajax> <div class="tableContainer"> <table> <tr>
diff --git a/polygerrit-ui/app/elements/gr-message.html b/polygerrit-ui/app/elements/gr-message.html index c0770a0..f105501 100644 --- a/polygerrit-ui/app/elements/gr-message.html +++ b/polygerrit-ui/app/elements/gr-message.html
@@ -15,7 +15,7 @@ --> <link rel="import" href="../bower_components/polymer/polymer.html"> -<link rel="import" href="./gr-date-formatter.html"> +<link rel="import" href="gr-date-formatter.html"> <dom-module id="gr-message"> <template> @@ -25,7 +25,7 @@ display: block; position: relative; } - :host:not([expanded]) { + :host(:not([expanded])) { cursor: pointer; } .avatar {
diff --git a/polygerrit-ui/app/elements/gr-messages-list.html b/polygerrit-ui/app/elements/gr-messages-list.html index 7771ef4..add285d 100644 --- a/polygerrit-ui/app/elements/gr-messages-list.html +++ b/polygerrit-ui/app/elements/gr-messages-list.html
@@ -15,7 +15,7 @@ --> <link rel="import" href="../bower_components/polymer/polymer.html"> -<link rel="import" href="./gr-message.html"> +<link rel="import" href="gr-message.html"> <dom-module id="gr-messages-list"> <template>
diff --git a/polygerrit-ui/app/scripts/app.js b/polygerrit-ui/app/scripts/app.js index ac9ee7d..7014074 100644 --- a/polygerrit-ui/app/scripts/app.js +++ b/polygerrit-ui/app/scripts/app.js
@@ -53,6 +53,12 @@ patchNum: ctx.params[4], path: ctx.params[5] }; + // Don't allow diffing the same patch number against itself because WHY? + if (params.basePatchNum == params.patchNum) { + page.redirect('/c/' + params.changeNum + '/' + params.patchNum + '/' + + params.path); + return; + } if (!params.patchNum) { params.patchNum = params.basePatchNum; delete(params.basePatchNum);
diff --git a/polygerrit-ui/app/test/gr-diff-comment-test.html b/polygerrit-ui/app/test/gr-diff-comment-test.html new file mode 100644 index 0000000..e673371 --- /dev/null +++ b/polygerrit-ui/app/test/gr-diff-comment-test.html
@@ -0,0 +1,57 @@ +<!DOCTYPE html> +<!-- +Copyright (C) 2015 The Android Open Source Project + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +--> + +<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes"> +<title>gr-diff-comment</title> + +<script src="../../bower_components/webcomponentsjs/webcomponents.min.js"></script> +<script src="../../bower_components/web-component-tester/browser.js"></script> +<script src="../../bower_components/test-fixture/test-fixture-mocha.js"></script> + +<link rel="import" href="../../bower_components/test-fixture/test-fixture.html"> +<link rel="import" href="../../bower_components/iron-test-helpers/iron-test-helpers.html"> +<link rel="import" href="../elements/gr-diff-comment.html"> + +<test-fixture id="basic"> + <template> + <gr-diff-comment></gr-diff-comment> + </template> +</test-fixture> + +<script> + suite('gr-diff-comment tests', function() { + var element; + setup(function() { + element = fixture('basic'); + }); + + test('proper event fires on reply', function(done) { + element.addEventListener('gr-diff-comment-reply', function(e) { + done(); + }); + MockInteractions.tap(element.$$('.reply')); + }); + + test('proper event fires on done', function(done) { + element.addEventListener('gr-diff-comment-done', function(e) { + done(); + }); + MockInteractions.tap(element.$$('.done')); + }); + + }); +</script>
diff --git a/polygerrit-ui/app/test/gr-diff-comment-thread-test.html b/polygerrit-ui/app/test/gr-diff-comment-thread-test.html new file mode 100644 index 0000000..c0a1358 --- /dev/null +++ b/polygerrit-ui/app/test/gr-diff-comment-thread-test.html
@@ -0,0 +1,118 @@ +<!DOCTYPE html> +<!-- +Copyright (C) 2015 The Android Open Source Project + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +--> + +<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes"> +<title>gr-diff-comment-thread</title> + +<script src="../../bower_components/webcomponentsjs/webcomponents.min.js"></script> +<script src="../../bower_components/web-component-tester/browser.js"></script> +<script src="../../bower_components/test-fixture/test-fixture-mocha.js"></script> +<script src="../scripts/util.js"></script> + +<link rel="import" href="../../bower_components/test-fixture/test-fixture.html"> +<link rel="import" href="../../bower_components/iron-test-helpers/iron-test-helpers.html"> +<link rel="import" href="../elements/gr-diff-comment-thread.html"> + +<test-fixture id="basic"> + <template> + <gr-diff-comment-thread></gr-diff-comment-thread> + </template> +</test-fixture> + +<script> + suite('gr-diff-comment-thread tests', function() { + var element; + setup(function() { + element = fixture('basic'); + }); + + test('comments are sorted correctly', function() { + var comments = [ + { + id: 'jacks_reply', + message: 'i like you, too', + in_reply_to: 'sallys_confession', + updated: '2015-12-25 15:00:20.396000000', + }, + { + id: 'sallys_confession', + message: 'i like you, jack', + updated: '2015-12-24 15:00:20.396000000', + }, + { + id: 'sally_to_dr_finklestein', + message: 'i’m running away', + updated: '2015-10-31 09:00:20.396000000', + }, + { + id: 'sallys_defiance', + in_reply_to: 'sally_to_dr_finklestein', + message: 'i will poison you so i can get away', + updated: '2015-10-31 15:00:20.396000000', + }, + { + id: 'dr_finklesteins_response', + in_reply_to: 'sally_to_dr_finklestein', + message: 'no i will pull a thread and your arm will fall off', + updated: '2015-10-31 11:00:20.396000000' + }, + { + id: 'sallys_mission', + message: 'i have to find santa', + updated: '2015-12-24 21:00:20.396000000' + } + ]; + var results = element._sortedComments(comments); + assert.deepEqual(results, [ + { + id: 'sally_to_dr_finklestein', + message: 'i’m running away', + updated: '2015-10-31 09:00:20.396000000', + }, + { + id: 'dr_finklesteins_response', + in_reply_to: 'sally_to_dr_finklestein', + message: 'no i will pull a thread and your arm will fall off', + updated: '2015-10-31 11:00:20.396000000' + }, + { + id: 'sallys_defiance', + in_reply_to: 'sally_to_dr_finklestein', + message: 'i will poison you so i can get away', + updated: '2015-10-31 15:00:20.396000000', + }, + { + id: 'sallys_confession', + message: 'i like you, jack', + updated: '2015-12-24 15:00:20.396000000', + }, + { + id: 'jacks_reply', + message: 'i like you, too', + in_reply_to: 'sallys_confession', + updated: '2015-12-25 15:00:20.396000000', + }, + { + id: 'sallys_mission', + message: 'i have to find santa', + updated: '2015-12-24 21:00:20.396000000' + } + ]); + }); + + }); +</script>
diff --git a/polygerrit-ui/app/test/gr-diff-view-test.html b/polygerrit-ui/app/test/gr-diff-view-test.html index e33aee8..a615d82 100644 --- a/polygerrit-ui/app/test/gr-diff-view-test.html +++ b/polygerrit-ui/app/test/gr-diff-view-test.html
@@ -16,7 +16,7 @@ --> <meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes"> -<title>gr-diff</title> +<title>gr-diff-view</title> <script src="../../bower_components/webcomponentsjs/webcomponents.min.js"></script> <script src="../../bower_components/web-component-tester/browser.js"></script> @@ -29,13 +29,21 @@ <test-fixture id="basic"> <template> + <gr-diff-view ruler-width="0"></gr-diff-view> + </template> +</test-fixture> + +<test-fixture id="comments"> + <template> <gr-diff-view></gr-diff-view> </template> </test-fixture> + <script> suite('gr-diff-view tests', function() { var element; + var elWithComments; // Original diff: // Left side (side A): // 1: if i < 5 { // "comments" &= \'fun\' && true @@ -86,17 +94,22 @@ ]; setup(function() { element = fixture('basic'); - element._constructDOM({content: diffContent}); + element._maybeRenderDiff({content: diffContent}, [], []); flushAsynchronousOperations(); + + elWithComments = fixture('comments'); + elWithComments._patchNum = 1; }); test('ab content is the same for left and right sides', function() { for (var i = 0; i < diffContent[0].ab.length; i++) { - var leftEl = element.$$('#LC' + (i + 1)); - assert.isNotNull(leftEl); - var rightEl = element.$$('#RC' + (i + 1)); - assert.isNotNull(rightEl); - assert.equal(leftEl.textContent, rightEl.textContent); + var leftEls = element.querySelectorAll( + '#leftDiffContent .content[data-line-num="' + (i + 1) + '"]'); + assert.equal(leftEls.length, 1); + var rightEls = element.querySelectorAll( + '#rightDiffContent .content[data-line-num="' + (i + 1) + '"]'); + assert.equal(rightEls.length, 1); + assert.equal(leftEls[0].textContent, rightEls[0].textContent); } }); @@ -112,7 +125,9 @@ }); test('content is properly escaped', function() { - var firstLineEls = element.querySelectorAll('#LC1, #RC1'); + var firstLineEls = element.querySelectorAll( + '#leftDiffContent .content[data-line-num="1"], ' + + '#rightDiffContent .content[data-line-num="1"]'); assert.equal(2, firstLineEls.length); for (var i = 0; i < firstLineEls.length; i++) { assert.equal(firstLineEls[i].innerHTML, @@ -121,11 +136,72 @@ }); test('content and line numbers are correct for a/b edit', function() { - assert.equal(element.$$('#LC7').textContent, 'foo'); - assert.equal(element.$$('#LC8').textContent, 'bar'); - assert.equal(element.$$('#RC7').textContent, 'baz'); - assert.equal(element.$$('#LC9').textContent, - element.$$('#RC8').textContent); + assert.equal(element.$$( + '#leftDiffContent .content[data-line-num="7"]').textContent, 'foo'); + assert.equal(element.$$( + '#leftDiffContent .content[data-line-num="8"]').textContent, 'bar'); + assert.equal(element.$$( + '#rightDiffContent .content[data-line-num="7"]').textContent, 'baz'); + assert.equal(element.$$( + '#leftDiffContent .content[data-line-num="9"]').textContent, + element.$$( + '#rightDiffContent .content[data-line-num="8"]').textContent); + }); + + test('ruler width changes are applied correctly', function() { + assert.equal(element.rulerWidth, 0); + assert.equal(element.querySelectorAll('.ruler').length, 0); + element.rulerWidth = 80; + flushAsynchronousOperations(); + var els = element.querySelectorAll('.ruler'); + assert.isAbove(els.length, 0); + for (var i = 0; i < els.length; i++) { + assert.equal(els[i].style.left, '80ch'); + } + element.rulerWidth = 0; + flushAsynchronousOperations(); + assert.equal(element.querySelectorAll('.ruler').length, 0); + element.rulerWidth = 100; + flushAsynchronousOperations(); + els = element.querySelectorAll('.ruler'); + assert.isAbove(els.length, 0); + for (var i = 0; i < els.length; i++) { + assert.equal(els[i].style.left, '100ch'); + } + }); + + test('comment threads are rendered correctly', function() { + elWithComments._maybeRenderDiff({content: diffContent}, [], [ + { + id: 'file_comment', + message: 'this is a file comment about the meaninglessness of life', + author: { + name: 'GLaDOS' + } + }, + { + id: 'all_the_lemons', + line: 8, + message: 'MAKE LIFE TAKE THE LEMONS BACK', + author: { + name: 'Cave Johnson', + }, + } + ]); + flushAsynchronousOperations(); + var threadEls = elWithComments.querySelectorAll( + 'gr-diff-comment-thread[data-thread-id="thread-1-8"]'); + assert.equal(threadEls.length, 1); + var fillerEls = elWithComments.querySelectorAll( + '.js-threadFiller[data-thread-id="thread-1-8"]'); + assert.equal(fillerEls.length, 3); + + threadEls = elWithComments.querySelectorAll( + 'gr-diff-comment-thread[data-thread-id="thread-1-FILE"]'); + assert.equal(threadEls.length, 1); + fillerEls = elWithComments.querySelectorAll( + '.js-threadFiller[data-thread-id="thread-1-FILE"]'); + assert.equal(fillerEls.length, 3); }); });
diff --git a/polygerrit-ui/app/test/index.html b/polygerrit-ui/app/test/index.html index 477b4044..df78fb9 100644 --- a/polygerrit-ui/app/test/index.html +++ b/polygerrit-ui/app/test/index.html
@@ -25,6 +25,8 @@ 'gr-change-list-item-test.html', 'gr-change-list-test.html', 'gr-date-formatter-test.html', + 'gr-diff-comment-test.html', + 'gr-diff-comment-thread-test.html', 'gr-diff-view-test.html' ]); </script>