Remove 'edit' button from file list rows
- Clicking on file path when in edit mode now navigates to editor view
- 'Edit' action renamed to 'Open'
- 'More' menu renamed to 'Actions'
Bug: Issue 8333
Change-Id: I1dcbec6a742692d3d07fc6070fdd3dc72fabe77a
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index 2c49e6c..1662dbd 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -1416,7 +1416,7 @@
case GrEditConstants.Actions.DELETE.id:
controls.openDeleteDialog(path);
break;
- case GrEditConstants.Actions.EDIT.id:
+ case GrEditConstants.Actions.OPEN.id:
Gerrit.Nav.navigateToRelativeUrl(
Gerrit.Nav.getEditUrlForDiff(this._change, path,
this._patchRange.patchNum));
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
index b7ae295..b958718 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -1439,9 +1439,9 @@
assert.isTrue(controls.openRenameDialog.called);
assert.equal(controls.openRenameDialog.lastCall.args[0], 'foo');
- // Edit
+ // Open
fileList.dispatchEvent(new CustomEvent('file-action-tap',
- {detail: {action: Actions.EDIT.id, path: 'foo'}, bubbles: true}));
+ {detail: {action: Actions.OPEN.id, path: 'foo'}, bubbles: true}));
flushAsynchronousOperations();
assert.isTrue(Gerrit.Nav.getEditUrlForDiff.called);
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
index bbfbe7f..500b4cb 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
@@ -225,8 +225,7 @@
opacity: 100;
}
.editFileControls {
- margin-left: 1em;
- width: 10em;
+ width: 7em;
}
@media screen and (max-width: 50em) {
.desktop {
@@ -279,9 +278,9 @@
[[_computeFileStatus(file.status)]]
</div>
<span
- data-url="[[_computeDiffURL(change, patchRange.patchNum, patchRange.basePatchNum, file.__path)]]"
+ data-url="[[_computeDiffURL(change, patchRange.patchNum, patchRange.basePatchNum, file.__path, editMode)]]"
class="path">
- <a class="pathLink" href$="[[_computeDiffURL(change, patchRange.patchNum, patchRange.basePatchNum, file.__path)]]">
+ <a class="pathLink" href$="[[_computeDiffURL(change, patchRange.patchNum, patchRange.basePatchNum, file.__path, editMode)]]">
<span title$="[[computeDisplayPath(file.__path)]]"
class="fullFileName">
[[computeDisplayPath(file.__path)]]
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
index 2c686c3..fc994ce 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -442,6 +442,7 @@
while (!row.classList.contains('row') && row.parentElement) {
row = row.parentElement;
}
+
const path = row.dataset.path;
// Handle checkbox mark as reviewed.
if (e.target.classList.contains('markReviewed')) {
@@ -667,7 +668,13 @@
return status || 'M';
},
- _computeDiffURL(change, patchNum, basePatchNum, path) {
+ _computeDiffURL(change, patchNum, basePatchNum, path, editMode) {
+ // TODO(kaspern): Fix editing for commit messages and merge lists.
+ if (editMode && path !== this.COMMIT_MESSAGE_PATH &&
+ path !== this.MERGE_LIST_PATH) {
+ return Gerrit.Nav.getEditUrlForDiff(change, path, patchNum,
+ basePatchNum);
+ }
return Gerrit.Nav.getUrlForDiff(change, path, patchNum, basePatchNum);
},
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
index d5f8d62..f7e4b96 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
@@ -986,6 +986,7 @@
element.change = {_number: 123};
element.patchRange = {patchNum: undefined, basePatchNum: 'PARENT'};
element._files = [{__path: 'foo/bar.cpp'}];
+ element.editMode = false;
flush(() => {
assert.isFalse(urlStub.called);
element.set('patchRange.patchNum', 4);
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-constants.html b/polygerrit-ui/app/elements/edit/gr-edit-constants.html
index 1941dc8..4bae900 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-constants.html
+++ b/polygerrit-ui/app/elements/edit/gr-edit-constants.html
@@ -19,8 +19,9 @@
const GrEditConstants = window.GrEditConstants || {};
+ // Order corresponds to order in the UI.
GrEditConstants.Actions = {
- EDIT: {label: 'Edit', id: 'edit'},
+ OPEN: {label: 'Open', id: 'open'},
DELETE: {label: 'Delete', id: 'delete'},
RENAME: {label: 'Rename', id: 'rename'},
RESTORE: {label: 'Restore', id: 'restore'},
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.html b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.html
index 5a59a37..9287592 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.html
+++ b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.html
@@ -79,13 +79,15 @@
</template>
<gr-overlay id="overlay" with-backdrop>
<gr-confirm-dialog
- id="editDialog"
+ id="openDialog"
class="invisible dialog"
disabled$="[[!_isValidPath(_path)]]"
- confirm-label="Edit"
- on-confirm="_handleEditConfirm"
+ confirm-label="Open"
+ on-confirm="_handleOpenConfirm"
on-cancel="_handleDialogCancel">
- <div class="header" slot="header">Edit a file</div>
+ <div class="header" slot="header">
+ Open an existing or new file
+ </div>
<div class="main" slot="main">
<gr-autocomplete
placeholder="Enter an existing or new full file path."
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.js b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.js
index cffae06..17b4f81 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.js
+++ b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.js
@@ -59,8 +59,8 @@
e.preventDefault();
const action = Polymer.dom(e).localTarget.id;
switch (action) {
- case GrEditConstants.Actions.EDIT.id:
- this.openEditDialog();
+ case GrEditConstants.Actions.OPEN.id:
+ this.openOpenDialog();
return;
case GrEditConstants.Actions.DELETE.id:
this.openDeleteDialog();
@@ -74,9 +74,9 @@
}
},
- openEditDialog(opt_path) {
+ openOpenDialog(opt_path) {
if (opt_path) { this._path = opt_path; }
- return this._showDialog(this.$.editDialog);
+ return this._showDialog(this.$.openDialog);
},
openDeleteDialog(opt_path) {
@@ -148,7 +148,7 @@
this._closeDialog(this._getDialogFromEvent(e));
},
- _handleEditConfirm(e) {
+ _handleOpenConfirm(e) {
const url = Gerrit.Nav.getEditUrlForDiff(this.change, this._path,
this.patchNum);
Gerrit.Nav.navigateToRelativeUrl(url);
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.html b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.html
index 5dcc581..00c8a3c 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.html
+++ b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.html
@@ -75,17 +75,17 @@
assert.isTrue(element._isValidPath('test.js'));
});
- test('edit', () => {
- MockInteractions.tap(element.$$('#edit'));
+ test('open', () => {
+ MockInteractions.tap(element.$$('#open'));
element.patchNum = 1;
return showDialogSpy.lastCall.returnValue.then(() => {
- assert.isTrue(element.$.editDialog.disabled);
+ assert.isTrue(element.$.openDialog.disabled);
assert.isFalse(queryStub.called);
- element.$.editDialog.querySelector('gr-autocomplete').text =
+ element.$.openDialog.querySelector('gr-autocomplete').text =
'src/test.cpp';
assert.isTrue(queryStub.called);
- assert.isFalse(element.$.editDialog.disabled);
- MockInteractions.tap(element.$.editDialog.$$('gr-button[primary]'));
+ assert.isFalse(element.$.openDialog.disabled);
+ MockInteractions.tap(element.$.openDialog.$$('gr-button[primary]'));
for (const stub of navStubs) { assert.isTrue(stub.called); }
assert.deepEqual(Gerrit.Nav.getEditUrlForDiff.lastCall.args,
[element.change, 'src/test.cpp', element.patchNum]);
@@ -94,13 +94,13 @@
});
test('cancel', () => {
- MockInteractions.tap(element.$$('#edit'));
+ MockInteractions.tap(element.$$('#open'));
return showDialogSpy.lastCall.returnValue.then(() => {
- assert.isTrue(element.$.editDialog.disabled);
- element.$.editDialog.querySelector('gr-autocomplete').text =
+ assert.isTrue(element.$.openDialog.disabled);
+ element.$.openDialog.querySelector('gr-autocomplete').text =
'src/test.cpp';
- assert.isFalse(element.$.editDialog.disabled);
- MockInteractions.tap(element.$.editDialog.$$('gr-button'));
+ assert.isFalse(element.$.openDialog.disabled);
+ MockInteractions.tap(element.$.openDialog.$$('gr-button'));
for (const stub of navStubs) { assert.isFalse(stub.called); }
assert.isTrue(closeDialogSpy.called);
assert.equal(element._path, 'src/test.cpp');
@@ -319,10 +319,10 @@
});
});
- test('openEditDialog', () => {
- return element.openEditDialog('test/path.cpp').then(() => {
- assert.isFalse(element.$.editDialog.hasAttribute('hidden'));
- assert.equal(element.$.editDialog.querySelector('gr-autocomplete').text,
+ test('openOpenDialog', () => {
+ return element.openOpenDialog('test/path.cpp').then(() => {
+ assert.isFalse(element.$.openDialog.hasAttribute('hidden'));
+ assert.equal(element.$.openDialog.querySelector('gr-autocomplete').text,
'test/path.cpp');
});
});
@@ -331,9 +331,9 @@
const spy = sandbox.spy(element, '_getDialogFromEvent');
element.addEventListener('tap', element._getDialogFromEvent);
- MockInteractions.tap(element.$.editDialog);
+ MockInteractions.tap(element.$.openDialog);
flushAsynchronousOperations();
- assert.equal(spy.lastCall.returnValue.id, 'editDialog');
+ assert.equal(spy.lastCall.returnValue.id, 'openDialog');
MockInteractions.tap(element.$.deleteDialog);
flushAsynchronousOperations();
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-file-controls/gr-edit-file-controls.html b/polygerrit-ui/app/elements/edit/gr-edit-file-controls/gr-edit-file-controls.html
index 96ba196b..f46f414 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-file-controls/gr-edit-file-controls.html
+++ b/polygerrit-ui/app/elements/edit/gr-edit-file-controls/gr-edit-file-controls.html
@@ -30,11 +30,7 @@
display: flex;
justify-content: flex-end;
}
- #edit {
- text-decoration: none;
- }
- #edit,
- #more {
+ #actions {
margin-right: 1em;
}
gr-button,
@@ -53,18 +49,13 @@
}
}
</style>
- <gr-button
- id="edit"
- link
- on-tap="_handleEditTap">Edit</gr-button>
- <!-- TODO(kaspern): implement more menu. -->
<gr-dropdown
- id="more"
+ id="actions"
items="[[_fileActions]]"
down-arrow
vertical-offset="20"
on-tap-item="_handleActionTap"
- link>More</gr-dropdown>
+ link>Actions</gr-dropdown>
</template>
<script src="gr-edit-file-controls.js"></script>
</dom-module>
\ No newline at end of file
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-file-controls/gr-edit-file-controls.js b/polygerrit-ui/app/elements/edit/gr-edit-file-controls/gr-edit-file-controls.js
index 62a4785..6a0ccf5 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-file-controls/gr-edit-file-controls.js
+++ b/polygerrit-ui/app/elements/edit/gr-edit-file-controls/gr-edit-file-controls.js
@@ -25,11 +25,9 @@
properties: {
filePath: String,
- // Edit action not needed in the overflow.
_allFileActions: {
type: Array,
- value: () => Object.values(GrEditConstants.Actions)
- .filter(action => action !== GrEditConstants.Actions.EDIT),
+ value: () => Object.values(GrEditConstants.Actions),
},
_fileActions: {
type: Array,
@@ -37,12 +35,6 @@
},
},
- _handleEditTap(e) {
- e.preventDefault();
- e.stopPropagation();
- this._dispatchFileAction(GrEditConstants.Actions.EDIT.id, this.filePath);
- },
-
_handleActionTap(e) {
e.preventDefault();
e.stopPropagation();
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-file-controls/gr-edit-file-controls_test.html b/polygerrit-ui/app/elements/edit/gr-edit-file-controls/gr-edit-file-controls_test.html
index 42fb466..27b28fa 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-file-controls/gr-edit-file-controls_test.html
+++ b/polygerrit-ui/app/elements/edit/gr-edit-file-controls/gr-edit-file-controls_test.html
@@ -47,55 +47,56 @@
teardown(() => { sandbox.restore(); });
- test('edit tap emits event', () => {
+ test('open tap emits event', () => {
+ const actions = element.$.actions;
element.filePath = 'foo';
+ actions._open();
+ flushAsynchronousOperations();
- MockInteractions.tap(element.$.edit);
+ MockInteractions.tap(actions.$$('li [data-id="open"]'));
assert.isTrue(fileActionHandler.called);
assert.deepEqual(fileActionHandler.lastCall.args[0].detail,
- {action: GrEditConstants.Actions.EDIT.id, path: 'foo'});
+ {action: GrEditConstants.Actions.OPEN.id, path: 'foo'});
});
test('delete tap emits event', () => {
- const more = element.$.more;
+ const actions = element.$.actions;
element.filePath = 'foo';
- more._open();
+ actions._open();
flushAsynchronousOperations();
- MockInteractions.tap(more.$$('li [data-id="delete"]'));
+ MockInteractions.tap(actions.$$('li [data-id="delete"]'));
assert.isTrue(fileActionHandler.called);
assert.deepEqual(fileActionHandler.lastCall.args[0].detail,
{action: GrEditConstants.Actions.DELETE.id, path: 'foo'});
});
test('restore tap emits event', () => {
- const more = element.$.more;
+ const actions = element.$.actions;
element.filePath = 'foo';
- more._open();
+ actions._open();
flushAsynchronousOperations();
- MockInteractions.tap(more.$$('li [data-id="restore"]'));
+ MockInteractions.tap(actions.$$('li [data-id="restore"]'));
assert.isTrue(fileActionHandler.called);
assert.deepEqual(fileActionHandler.lastCall.args[0].detail,
{action: GrEditConstants.Actions.RESTORE.id, path: 'foo'});
});
test('rename tap emits event', () => {
- const more = element.$.more;
+ const actions = element.$.actions;
element.filePath = 'foo';
- more._open();
+ actions._open();
flushAsynchronousOperations();
- MockInteractions.tap(more.$$('li [data-id="rename"]'));
+ MockInteractions.tap(actions.$$('li [data-id="rename"]'));
assert.isTrue(fileActionHandler.called);
assert.deepEqual(fileActionHandler.lastCall.args[0].detail,
{action: GrEditConstants.Actions.RENAME.id, path: 'foo'});
});
test('computed properties', () => {
- assert.equal(element._allFileActions.length, 3);
- assert.notOk(element._allFileActions
- .find(action => action.id === GrEditConstants.Actions.EDIT.id));
+ assert.equal(element._allFileActions.length, 4);
});
});
</script>
\ No newline at end of file
