opendev/gerrit
Code Issues Proposed changes

Refactor file path truncation into behavior

Browse Source 
Change-Id: I21af00d3efc28291aa6918fde643cc6c77a7d05c
This commit is contained in:
Kasper Nilsson 2017-09-12 14:15:44 -07:00
parent 512eb25dcd
commit ce67e2a921
12 changed files with 88 additions and 124 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

49
polygerrit-ui/app/behaviors/gr-path-list-behavior/gr-path-list-behavior.html
View File

@ -13,6 +13,7 @@ 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.
-->
<script src="../../scripts/util.js"></script>
<script>
(function(window) {
'use strict';
@ -20,6 +21,10 @@ limitations under the License.
window.Gerrit = window.Gerrit || {};
/** @polymerBehavior Gerrit.PathListBehavior */
Gerrit.PathListBehavior = {
COMMIT_MESSAGE_PATH: '/COMMIT_MSG',
MERGE_LIST_PATH: '/MERGE_LIST',
/**
* @param {string} a
* @param {string} b
@ -27,20 +32,18 @@ limitations under the License.
*/
specialFilePathCompare(a, b) {
// The commit message always goes first.
const COMMIT_MESSAGE_PATH = '/COMMIT_MSG';
if (a === COMMIT_MESSAGE_PATH) {
if (a === Gerrit.PathListBehavior.COMMIT_MESSAGE_PATH) {
return -1;
}
if (b === COMMIT_MESSAGE_PATH) {
if (b === Gerrit.PathListBehavior.COMMIT_MESSAGE_PATH) {
return 1;
}
// The merge list always comes next.
const MERGE_LIST_PATH = '/MERGE_LIST';
if (a === MERGE_LIST_PATH) {
if (a === Gerrit.PathListBehavior.MERGE_LIST_PATH) {
return -1;
}
if (b === MERGE_LIST_PATH) {
if (b === Gerrit.PathListBehavior.MERGE_LIST_PATH) {
return 1;
}
@ -67,6 +70,40 @@ limitations under the License.
}
return aFile.localeCompare(bFile) || a.localeCompare(b);
},
computeDisplayPath(path) {
if (path === Gerrit.PathListBehavior.COMMIT_MESSAGE_PATH) {
return 'Commit message';
} else if (path === Gerrit.PathListBehavior.MERGE_LIST_PATH) {
return 'Merge list';
}
return path;
},
computeTruncatedPath(path) {
return Gerrit.PathListBehavior.truncatePath(
Gerrit.PathListBehavior.computeDisplayPath(path));
},
/**
* Truncates URLs to display filename only
* Example
* // returns '.../text.html'
* util.truncatePath.('dir/text.html');
* Example
* // returns 'text.html'
* util.truncatePath.('text.html');
* @return {string} Returns the truncated value of a URL.
*/
truncatePath(path) {
const pathPieces = path.split('/');
if (pathPieces.length < 2) {
return path;
}
// Character is an ellipsis.
return '\u2026/' + pathPieces[pathPieces.length - 1];
},
};
})(window);
</script>

29
polygerrit-ui/app/behaviors/gr-path-list-behavior/gr-path-list-behavior_test.html
View File

@ -44,5 +44,34 @@ limitations under the License.
'/mrPeanutbutter.py',
]);
});
test('file display name', () => {
const name = Gerrit.PathListBehavior.computeDisplayPath;
assert.equal(name('/foo/bar/baz'), '/foo/bar/baz');
assert.equal(name('/foobarbaz'), '/foobarbaz');
assert.equal(name('/COMMIT_MSG'), 'Commit message');
assert.equal(name('/MERGE_LIST'), 'Merge list');
});
test('truncatePath with long path should add ellipsis', () => {
const truncatePath = Gerrit.PathListBehavior.truncatePath;
let path = 'level1/level2/level3/level4/file.js';
let shortenedPath = truncatePath(path);
// The expected path is truncated with an ellipsis.
const expectedPath = '\u2026/file.js';
assert.equal(shortenedPath, expectedPath);
path = 'level2/file.js';
shortenedPath = truncatePath(path);
assert.equal(shortenedPath, expectedPath);
});
test('truncatePath with short path should not add ellipsis', () => {
const truncatePath = Gerrit.PathListBehavior.truncatePath;
const path = 'file.js';
const expectedPath = 'file.js';
const shortenedPath = truncatePath(path);
assert.equal(shortenedPath, expectedPath);
});
});
</script>

2
polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.html
View File

@ -57,7 +57,7 @@ limitations under the License.
<template is="dom-repeat" items="[[_computeFilesFromComments(comments)]]" as="file">
<div class="file">
<a href$="[[_computeFileDiffURL(file, changeNum, patchNum)]]">
[[_computeFileDisplayName(file)]]
[[computeDisplayPath(file)]]
</a>:
</div>
<template is="dom-repeat"

13
polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.js
View File

@ -13,10 +13,6 @@
// limitations under the License.
(function() {
'use strict';
const COMMIT_MESSAGE_PATH = '/COMMIT_MSG';
const MERGE_LIST_PATH = '/MERGE_LIST';
Polymer({
is: 'gr-comment-list',
@ -46,15 +42,6 @@
file, patchNum);
},
_computeFileDisplayName(path) {
if (path === COMMIT_MESSAGE_PATH) {
return 'Commit message';
} else if (path === MERGE_LIST_PATH) {
return 'Merge list';
}
return path;
},
_isOnParent(comment) {
return comment.side === 'PARENT';
},

9
polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list_test.html
View File

@ -60,15 +60,6 @@ limitations under the License.
assert.deepEqual(element._computeFilesFromComments(null), []);
});
test('_computeFileDisplayName', () => {
assert.equal(element._computeFileDisplayName('/COMMIT_MSG'),
'Commit message');
assert.equal(element._computeFileDisplayName('/MERGE_LIST'),
'Merge list');
assert.equal(element._computeFileDisplayName('/foo/bar/baz'),
'/foo/bar/baz');
});
test('_computePatchDisplayName', () => {
const comment = {line: 123, side: 'REVISION', patch_set: 10};

9
polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
View File

@ -16,6 +16,7 @@ limitations under the License.
<link rel="import" href="../../../behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html">
<link rel="import" href="../../../behaviors/gr-patch-set-behavior/gr-patch-set-behavior.html">
<link rel="import" href="../../../behaviors/gr-path-list-behavior/gr-path-list-behavior.html">
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../core/gr-navigation/gr-navigation.html">
<link rel="import" href="../../core/gr-reporting/gr-reporting.html">
@ -240,13 +241,13 @@ limitations under the License.
data-url="[[_computeDiffURL(change, patchRange.patchNum, patchRange.basePatchNum, file.__path)]]"
class$="[[_computePathClass(file.__path, _expandedFilePaths.*)]]">
<a href$="[[_computeDiffURL(change, patchRange.patchNum, patchRange.basePatchNum, file.__path)]]">
<span title$="[[_computeFileDisplayName(file.__path)]]"
<span title$="[[computeDisplayPath(file.__path)]]"
class="fullFileName">
[[_computeFileDisplayName(file.__path)]]
[[computeDisplayPath(file.__path)]]
</span>
<span title$="[[_computeFileDisplayName(file.__path)]]"
<span title$="[[computeDisplayPath(file.__path)]]"
class="truncatedFileName">
[[_computeTruncatedFileDisplayName(file.__path)]]
[[computeTruncatedPath(file.__path)]]
</span>
</a>
<div class="oldPath" hidden$="[[!file.old_path]]" hidden

18
polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
View File

@ -19,8 +19,6 @@
// Maximum length for patch set descriptions.
const PATCH_DESC_MAX_LENGTH = 500;
const WARN_SHOW_ALL_THRESHOLD = 1000;
const COMMIT_MESSAGE_PATH = '/COMMIT_MSG';
const MERGE_LIST_PATH = '/MERGE_LIST';
const LOADING_DEBOUNCE_INTERVAL = 100;
const FileStatus = {
@ -122,6 +120,7 @@
behaviors: [
Gerrit.KeyboardShortcutBehavior,
Gerrit.PatchSetBehavior,
Gerrit.PathListBehavior,
],
observers: [
@ -665,19 +664,6 @@
return Gerrit.Nav.getUrlForDiff(change, path, patchNum, basePatchNum);
},
_computeFileDisplayName(path) {
if (path === COMMIT_MESSAGE_PATH) {
return 'Commit message';
} else if (path === MERGE_LIST_PATH) {
return 'Merge list';
}
return path;
},
_computeTruncatedFileDisplayName(path) {
return util.truncatePath(this._computeFileDisplayName(path));
},
_formatBytes(bytes) {
if (bytes == 0) return '+/-0 B';
const bits = 1024;
@ -706,7 +692,7 @@
_computeClass(baseClass, path) {
const classes = [baseClass];
if (path === COMMIT_MESSAGE_PATH || path === MERGE_LIST_PATH) {
if (path === this.COMMIT_MESSAGE_PATH || path === this.MERGE_LIST_PATH) {
classes.push('invisible');
}
return classes.join(' ');

5
polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
View File

@ -577,11 +577,6 @@ limitations under the License.
assert.equal(element._computeFileStatus(undefined), 'M');
assert.equal(element._computeFileStatus(null), 'M');
assert.equal(element._computeFileDisplayName('/foo/bar/baz'),
'/foo/bar/baz');
assert.equal(element._computeFileDisplayName('/COMMIT_MSG'),
'Commit message');
assert.equal(element._computeClass('clazz', '/foo/bar/baz'), 'clazz');
assert.equal(element._computeClass('clazz', '/COMMIT_MSG'),
'clazz invisible');

9
polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
View File

@ -16,6 +16,7 @@ limitations under the License.
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../behaviors/gr-patch-set-behavior/gr-patch-set-behavior.html">
<link rel="import" href="../../../behaviors/gr-path-list-behavior/gr-path-list-behavior.html">
<link rel="import" href="../../../behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html">
<link rel="import" href="../../../behaviors/rest-client-behavior/rest-client-behavior.html">
<link rel="import" href="../../../bower_components/iron-dropdown/iron-dropdown.html">
@ -229,7 +230,7 @@ limitations under the License.
hidden$="[[!_loggedIn]]" hidden>
<div class="jumpToFileContainer desktop">
<gr-button link class="dropdown-trigger" id="trigger" on-tap="_showDropdownTapHandler">
<span>[[_computeFileDisplayName(_path)]]</span>
<span>[[computeDisplayPath(_path)]]</span>
<span class="downArrow">&#9660;</span>
</gr-button>
<!-- *-align="" to disable iron-dropdown's element positioning. -->
@ -246,7 +247,7 @@ limitations under the License.
<a href$="[[_computeDiffURL(_change, _patchRange.*, path)]]"
selected$="[[_computeFileSelected(path, _path)]]"
data-key-nav$="[[_computeKeyNav(path, _path, _fileList)]]"
on-tap="_handleFileTap">[[_computeFileDisplayName(path)]]</a>
on-tap="_handleFileTap">[[computeDisplayPath(path)]]</a>
</template>
</div>
</iron-dropdown>
@ -257,7 +258,7 @@ limitations under the License.
<option
value$="[[path]]"
selected$="[[_computeFileSelected(path, _path)]]">
[[_computeTruncatedFileDisplayName(path)]]
[[computeTruncatedPath(path)]]
</option>
</template>
</select>
@ -331,7 +332,7 @@ limitations under the License.
<a class="mobileNavLink"
href$="[[_computeNavLinkURL(_change, _path, _fileList, -1, 1)]]">
&lt;</a>
<div class="fullFileName mobile">[[_computeFileDisplayName(_path)]]
<div class="fullFileName mobile">[[computeDisplayPath(_path)]]
</div>
<a class="mobileNavLink"
href$="[[_computeNavLinkURL(_change, _path, _fileList, 1, 1)]]">

21
polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
View File

@ -14,9 +14,6 @@
(function() {
'use strict';
const COMMIT_MESSAGE_PATH = '/COMMIT_MSG';
const MERGE_LIST_PATH = '/MERGE_LIST';
const ERR_REVIEW_STATUS = 'Couldn’t change file review status.';
const MSG_LOADING_BLAME = 'Loading blame...';
const MSG_LOADED_BLAME = 'Blame loaded';
@ -142,6 +139,7 @@
behaviors: [
Gerrit.KeyboardShortcutBehavior,
Gerrit.PatchSetBehavior,
Gerrit.PathListBehavior,
Gerrit.RESTClientBehavior,
],
@ -495,7 +493,7 @@
// has been queued, the event can bubble up to the handler in gr-app.
this.async(() => {
this.fire('title-change',
{title: this._computeTruncatedFileDisplayName(this._path)});
{title: this.computeTruncatedPath(this._path)});
});
// When navigating away from the page, there is a possibility that the
@ -568,7 +566,7 @@
_pathChanged(path) {
if (path) {
this.fire('title-change',
{title: this._computeTruncatedFileDisplayName(path)});
{title: this.computeTruncatedPath(path)});
}
if (this._fileList.length == 0) { return; }
@ -640,19 +638,6 @@
return this._getChangePath(change, patchRangeRecord.base, revisions);
},
_computeFileDisplayName(path) {
if (path === COMMIT_MESSAGE_PATH) {
return 'Commit message';
} else if (path === MERGE_LIST_PATH) {
return 'Merge list';
}
return path;
},
_computeTruncatedFileDisplayName(path) {
return util.truncatePath(this._computeFileDisplayName(path));
},
_computeFileSelected(path, currentPath) {
return path == currentPath;
},

28
polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
View File

@ -336,15 +336,6 @@ limitations under the License.
'42-glados.txt-10-PARENT');
assert.equal(linkEls[2].getAttribute('href'),
'42-wheatley.md-10-PARENT');
assert.equal(element._computeFileDisplayName('/foo/bar/baz'),
'/foo/bar/baz');
assert.equal(element._computeFileDisplayName('/foobarbaz'),
'/foobarbaz');
assert.equal(element._computeFileDisplayName('/COMMIT_MSG'),
'Commit message');
assert.equal(element._computeFileDisplayName('/MERGE_LIST'),
'Merge list');
});
test('jump to file dropdown with patch range', () => {
@ -614,25 +605,6 @@ limitations under the License.
assert.equal(element.$.cursor.side, 'right');
});
test('_shortenPath with long path should add ellipsis', () => {
let path = 'level1/level2/level3/level4/file.js';
let shortenedPath = util.truncatePath(path);
// The expected path is truncated with an ellipsis.
const expectedPath = '\u2026/file.js';
assert.equal(shortenedPath, expectedPath);
path = 'level2/file.js';
shortenedPath = util.truncatePath(path);
assert.equal(shortenedPath, expectedPath);
});
test('_shortenPath with short path should not add ellipsis', () => {
const path = 'file.js';
const expectedPath = 'file.js';
const shortenedPath = util.truncatePath(path);
assert.equal(shortenedPath, expectedPath);
});
test('_onLineSelected', () => {
const getUrlStub = sandbox.stub(Gerrit.Nav, 'getUrlForDiffById');
const replaceStateStub = sandbox.stub(history, 'replaceState');

20
polygerrit-ui/app/scripts/util.js
View File

@ -38,25 +38,5 @@
}
return '';
};
/**
* Truncates URLs to display filename only
* Example
* // returns '.../text.html'
* util.truncatePath.('dir/text.html');
* Example
* // returns 'text.html'
* util.truncatePath.('text.html');
* @return {string} Returns the truncated value of a URL.
*/
util.truncatePath = function(path) {
const pathPieces = path.split('/');
if (pathPieces.length < 2) {
return path;
}
// Character is an ellipsis.
return '\u2026/' + pathPieces[pathPieces.length - 1];
};
window.util = util;
})(window);