Modify change table editor so that all columns are in a single section
Previously the change table editor had a section for shown columns and a separate one for hidden columns. From the shown columns section, you could 'delete' a column, and from the hidden section, you could 'add' a column. This was confusing for users, particularly when there were no hidden columns. This change keeps all columns in the section, but instead adds checkbox indicating whether or not each column should be visible. Bug: Issue 5547 Change-Id: I65d1afd4c5e3e9cbbf823ad5773db1f13ca182ef
This commit is contained in:
Becky Siegel
parent
b3ae6f0d6a
commit
14aa6ab6fc
|
|
@ -19,22 +19,28 @@ limitations under the License.
|
|||
|
||||
/** @polymerBehavior Gerrit.ChangeTableBehavior */
|
||||
var ChangeTableBehavior = {
|
||||
CHANGE_TABLE_COLUMNS: [
|
||||
'Subject',
|
||||
'Status',
|
||||
'Owner',
|
||||
'Project',
|
||||
'Branch',
|
||||
'Updated',
|
||||
'Size',
|
||||
],
|
||||
properties: {
|
||||
columnNames: {
|
||||
type: Array,
|
||||
value: [
|
||||
'Subject',
|
||||
'Status',
|
||||
'Owner',
|
||||
'Project',
|
||||
'Branch',
|
||||
'Updated',
|
||||
'Size',
|
||||
],
|
||||
readOnly: true,
|
||||
}
|
||||
},
|
||||
|
||||
/**
|
||||
* Returns the complement to the given column array
|
||||
* @param {Array} columns
|
||||
*/
|
||||
getComplementColumns: function(columns) {
|
||||
return this.CHANGE_TABLE_COLUMNS.filter(function(column) {
|
||||
return this.columnNames.filter(function(column) {
|
||||
return columns.indexOf(column) === -1;
|
||||
});
|
||||
},
|
||||
|
|
|
|||
|
|
@ -151,7 +151,7 @@ limitations under the License.
|
|||
|
||||
flushAsynchronousOperations();
|
||||
|
||||
element.CHANGE_TABLE_COLUMNS.forEach(function(column) {
|
||||
element.columnNames.forEach(function(column) {
|
||||
var elementClass = '.' + column.toLowerCase();
|
||||
assert.isFalse(element.$$(elementClass).hidden);
|
||||
});
|
||||
|
|
@ -170,7 +170,7 @@ limitations under the License.
|
|||
|
||||
flushAsynchronousOperations();
|
||||
|
||||
element.CHANGE_TABLE_COLUMNS.forEach(function(column) {
|
||||
element.columnNames.forEach(function(column) {
|
||||
var elementClass = '.' + column.toLowerCase();
|
||||
assert.isFalse(element.$$(elementClass).hidden);
|
||||
});
|
||||
|
|
@ -188,7 +188,7 @@ limitations under the License.
|
|||
|
||||
flushAsynchronousOperations();
|
||||
|
||||
element.CHANGE_TABLE_COLUMNS.forEach(function(column) {
|
||||
element.columnNames.forEach(function(column) {
|
||||
var elementClass = '.' + column.toLowerCase();
|
||||
if (column === 'Project') {
|
||||
assert.isTrue(element.$$(elementClass).hidden);
|
||||
|
|
|
|||
|
|
@ -111,18 +111,18 @@
|
|||
|
||||
_loadPreferences: function() {
|
||||
return this._getLoggedIn().then(function(loggedIn) {
|
||||
this.changeTableColumns = this.CHANGE_TABLE_COLUMNS;
|
||||
this.changeTableColumns = this.columnNames;
|
||||
|
||||
if (!loggedIn) {
|
||||
this.showNumber = false;
|
||||
this.visibleChangeTableColumns = this.CHANGE_TABLE_COLUMNS;
|
||||
this.visibleChangeTableColumns = this.columnNames;
|
||||
return;
|
||||
}
|
||||
return this._getPreferences().then(function(preferences) {
|
||||
this.showNumber = !!(preferences &&
|
||||
preferences.legacycid_in_change_table);
|
||||
this.visibleChangeTableColumns = preferences.change_table.length > 0 ?
|
||||
preferences.change_table : this.CHANGE_TABLE_COLUMNS;
|
||||
preferences.change_table : this.columnNames;
|
||||
}.bind(this));
|
||||
}.bind(this));
|
||||
},
|
||||
|
|
|
|||
|
|
@ -266,7 +266,7 @@ limitations under the License.
|
|||
});
|
||||
|
||||
test('all columns visible', function() {
|
||||
element.CHANGE_TABLE_COLUMNS.forEach(function(column) {
|
||||
element.columnNames.forEach(function(column) {
|
||||
var elementClass = '.' + element._lowerCase(column);
|
||||
assert.isFalse(element.$$(elementClass).hidden);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -25,21 +25,19 @@ limitations under the License.
|
|||
<dom-module id="gr-change-table-editor">
|
||||
<template>
|
||||
<style>
|
||||
table {
|
||||
margin-top: 1em;
|
||||
}
|
||||
th.nameHeader {
|
||||
width: 11em;
|
||||
}
|
||||
tbody tr:first-of-type td .move-up-button,
|
||||
tbody tr:last-of-type td .move-down-button {
|
||||
display: none;
|
||||
td.checkboxContainer {
|
||||
border: 1px solid #fff;
|
||||
cursor: pointer;
|
||||
text-align: center;
|
||||
}
|
||||
.newTitleInput {
|
||||
width: 10em;
|
||||
}
|
||||
.newUrlInput {
|
||||
width: 23em;
|
||||
}
|
||||
.addOptions {
|
||||
margin-top: 1em;
|
||||
td.checkboxContainer:hover {
|
||||
border: 1px solid #ddd;
|
||||
}
|
||||
</style>
|
||||
<style include="gr-settings-styles"></style>
|
||||
|
|
@ -48,47 +46,24 @@ limitations under the License.
|
|||
<thead>
|
||||
<tr>
|
||||
<th class="nameHeader">Column</th>
|
||||
<th>Visible</th>
|
||||
</tr>
|
||||
</thead>
|
||||
<tbody>
|
||||
<template is="dom-repeat" items="[[changeTableItems]]">
|
||||
<template is="dom-repeat" items="[[columnNames]]">
|
||||
<tr>
|
||||
<td>[[item]]</td>
|
||||
<td>
|
||||
<gr-button
|
||||
data-index="[[index]]"
|
||||
on-tap="_handleDeleteButton"
|
||||
class="remove-button">Delete</gr-button>
|
||||
<td class="checkboxContainer"
|
||||
on-tap="_handleTargetTap">
|
||||
<input
|
||||
type="checkbox"
|
||||
name="[[item]]"
|
||||
checked$="[[!isColumnHidden(item, displayedColumns)]]">
|
||||
</td>
|
||||
</tr>
|
||||
</template>
|
||||
</tbody>
|
||||
</table>
|
||||
|
||||
<template is="dom-if" if="[[changeTableNotDisplayed.length]]">
|
||||
<table class="addOptions">
|
||||
<thead>
|
||||
<tr>
|
||||
<th class="nameHeader">Hidden</th>
|
||||
</tr>
|
||||
</thead>
|
||||
<tbody>
|
||||
<template is="dom-repeat" items="[[changeTableNotDisplayed]]">
|
||||
<tr>
|
||||
<td>[[item]]</td>
|
||||
<td>
|
||||
<gr-button
|
||||
on-tap="_handleAddButton"
|
||||
class="add-button"
|
||||
data-index="[[index]]">
|
||||
Add
|
||||
</gr-button>
|
||||
</td>
|
||||
</tr>
|
||||
</template>
|
||||
</tbody>
|
||||
</table>
|
||||
</template>
|
||||
</div>
|
||||
</template>
|
||||
<script src="gr-change-table-editor.js"></script>
|
||||
|
|
|
|||
|
|
@ -18,33 +18,44 @@
|
|||
is: 'gr-change-table-editor',
|
||||
|
||||
properties: {
|
||||
changeTableItems: Array,
|
||||
changeTableNotDisplayed: Array,
|
||||
displayedColumns: {
|
||||
type: Array,
|
||||
notify: true,
|
||||
},
|
||||
},
|
||||
|
||||
behaviors: [
|
||||
Gerrit.ChangeTableBehavior,
|
||||
],
|
||||
|
||||
_handleDeleteButton: function(e) {
|
||||
var index = e.target.dataIndex;
|
||||
this.splice('changeTableItems', index, 1);
|
||||
|
||||
// Use the change table behavior to make sure ordering of unused
|
||||
// columns ends up in the correct order. If the removed item is appended
|
||||
// to the end, when it is saved, the unused column order may shift around.
|
||||
this.set('changeTableNotDisplayed',
|
||||
this.getComplementColumns(this.changeTableItems));
|
||||
|
||||
_getButtonText: function(isShown) {
|
||||
return isShown ? 'Hide' : 'Show';
|
||||
},
|
||||
|
||||
_handleAddButton: function(e) {
|
||||
var index = e.target.dataIndex;
|
||||
var newColumn = this.changeTableNotDisplayed[index];
|
||||
this.splice('changeTableNotDisplayed', index, 1);
|
||||
_updateDisplayedColumns: function(displayedColumns, name, checked) {
|
||||
if (!checked) {
|
||||
return displayedColumns.filter(function(column) {
|
||||
return name.toLowerCase() !== column.toLowerCase();
|
||||
});
|
||||
} else {
|
||||
return displayedColumns.concat([name]);
|
||||
}
|
||||
},
|
||||
|
||||
this.splice('changeTableItems', this.getComplementColumns(
|
||||
this.changeTableNotDisplayed).indexOf(newColumn), 0, newColumn);
|
||||
/**
|
||||
* Handles tap on either the checkbox itself or the surrounding table cell.
|
||||
*/
|
||||
_handleTargetTap: function(e) {
|
||||
var checkbox = Polymer.dom(e.target).querySelector('input');
|
||||
if (checkbox) {
|
||||
checkbox.click();
|
||||
} else {
|
||||
// The target is the checkbox itself.
|
||||
checkbox = Polymer.dom(event).rootTarget;
|
||||
}
|
||||
this.set('displayedColumns',
|
||||
this._updateDisplayedColumns(
|
||||
this.displayedColumns, checkbox.name, checkbox.checked));
|
||||
},
|
||||
});
|
||||
})();
|
||||
|
|
|
|||
|
|
@ -31,22 +31,15 @@ limitations under the License.
|
|||
</test-fixture>
|
||||
|
||||
<script>
|
||||
suite('gr-settings-view tests', function() {
|
||||
suite('gr-change-table-editor tests', function() {
|
||||
var element;
|
||||
var columns;
|
||||
|
||||
// Click the up/down button (according to direction) for the index'th row.
|
||||
// The index of the first row is 0, corresponding to the array.
|
||||
function move(element, index, direction) {
|
||||
var selector =
|
||||
'tr:nth-child(' + (index + 1) + ') .move-' + direction + '-button';
|
||||
var button = element.$$('tbody').querySelector(selector);
|
||||
MockInteractions.tap(button);
|
||||
flushAsynchronousOperations();
|
||||
}
|
||||
var sandbox;
|
||||
|
||||
setup(function() {
|
||||
element = fixture('basic');
|
||||
sandbox = sinon.sandbox.create();
|
||||
|
||||
columns = [
|
||||
'Subject',
|
||||
'Status',
|
||||
|
|
@ -56,138 +49,104 @@ limitations under the License.
|
|||
'Updated',
|
||||
];
|
||||
|
||||
columnsNotDisplayed = ['Size'];
|
||||
|
||||
element.set('changeTableItems', columns);
|
||||
element.set('changeTableNotDisplayed', columnsNotDisplayed);
|
||||
element.set('displayedColumns', columns);
|
||||
flushAsynchronousOperations();
|
||||
});
|
||||
|
||||
teardown(function() {
|
||||
sandbox.restore();
|
||||
});
|
||||
|
||||
test('renders', function() {
|
||||
var rows = element.$$('tbody').querySelectorAll('tr');
|
||||
var tds;
|
||||
|
||||
assert.equal(rows.length, columns.length);
|
||||
assert.equal(rows.length, element.columnNames.length);
|
||||
for (var i = 0; i < columns.length; i++) {
|
||||
tds = rows[i].querySelectorAll('td');
|
||||
assert.equal(tds[0].textContent, columns[i]);
|
||||
}
|
||||
});
|
||||
|
||||
test('add hidden item', function() {
|
||||
var originalNumberColumns = element.changeTableItems.length;
|
||||
var originalNumberHiddenColumns = element.changeTableNotDisplayed.length;
|
||||
test('hide item', function() {
|
||||
var checkbox = element.$$('table input');
|
||||
var isChecked = checkbox.checked;
|
||||
var displayedLength = element.displayedColumns.length;
|
||||
assert.isTrue(isChecked);
|
||||
|
||||
var addBtn = element.$$('.addOptions gr-button');
|
||||
var columnName = element.$$('.addOptions tr td').innerHTML;
|
||||
|
||||
assert.equal(element.$$('.addOptions').style.display, '');
|
||||
|
||||
MockInteractions.tap(addBtn);
|
||||
MockInteractions.tap(checkbox);
|
||||
flushAsynchronousOperations();
|
||||
|
||||
assert.equal(element.changeTableItems.length, originalNumberColumns + 1);
|
||||
assert.equal(element.changeTableNotDisplayed.length,
|
||||
originalNumberHiddenColumns - 1);
|
||||
|
||||
assert.equal(
|
||||
element.changeTableItems[element.changeTableItems.length - 1],
|
||||
columnName);
|
||||
|
||||
assert.equal(element.$$('.addOptions').style.display, 'none');
|
||||
assert.equal(element.displayedColumns.length,
|
||||
displayedLength - 1);
|
||||
});
|
||||
|
||||
test('remove item', function() {
|
||||
var columns = element.changeTableItems.length;
|
||||
assert.deepEqual(element.changeTableItems, [
|
||||
'Subject',
|
||||
test('show item', function() {
|
||||
element.set('displayedColumns', [
|
||||
'Status',
|
||||
'Owner',
|
||||
'Project',
|
||||
'Branch',
|
||||
'Updated',
|
||||
]);
|
||||
flushAsynchronousOperations();
|
||||
var checkbox = element.$$('table input');
|
||||
var isChecked = checkbox.checked;
|
||||
var displayedLength = element.displayedColumns.length;
|
||||
assert.isFalse(isChecked);
|
||||
assert.equal(element.$$('table').style.display, '');
|
||||
|
||||
// Tap the delete button for the second item.
|
||||
MockInteractions.tap(
|
||||
element.$$('tbody').querySelector('tr:nth-child(2) .remove-button'));
|
||||
MockInteractions.tap(checkbox);
|
||||
flushAsynchronousOperations();
|
||||
|
||||
assert.deepEqual(element.changeTableItems, [
|
||||
'Subject',
|
||||
'Owner',
|
||||
'Project',
|
||||
'Branch',
|
||||
'Updated',
|
||||
]);
|
||||
|
||||
assert.deepEqual(element.changeTableNotDisplayed, [
|
||||
'Status',
|
||||
'Size',
|
||||
]);
|
||||
|
||||
// Delete remaining items.
|
||||
for (var i = 0; i < columns - 1; i++) {
|
||||
MockInteractions.tap(
|
||||
element.$$('tbody').querySelector('tr:first-child .remove-button'));
|
||||
}
|
||||
assert.deepEqual(element.changeTableItems, []);
|
||||
assert.deepEqual(element.changeTableNotDisplayed, [
|
||||
'Subject',
|
||||
'Status',
|
||||
'Owner',
|
||||
'Project',
|
||||
'Branch',
|
||||
'Updated',
|
||||
'Size',
|
||||
]);
|
||||
assert.equal(element.displayedColumns.length,
|
||||
displayedLength + 1);
|
||||
});
|
||||
|
||||
test('add item', function() {
|
||||
test('_handleTargetTap', function() {
|
||||
var checkbox = element.$$('table input');
|
||||
var originalDisplayedColumns = element.displayedColumns;
|
||||
var td = element.$$('table .checkboxContainer');
|
||||
var displayedColumnStub =
|
||||
sandbox.stub(element, '_updateDisplayedColumns');
|
||||
|
||||
element.set('changeTableItems', [
|
||||
'Status',
|
||||
'Owner',
|
||||
'Project',
|
||||
'Branch',
|
||||
'Updated',
|
||||
]);
|
||||
MockInteractions.tap(checkbox);
|
||||
assert.isTrue(displayedColumnStub.lastCall.calledWithExactly(
|
||||
originalDisplayedColumns,
|
||||
checkbox.name,
|
||||
checkbox.checked));
|
||||
|
||||
element.set('changeTableNotDisplayed', ['Subject', 'Size']);
|
||||
originalDisplayedColumns = element.displayedColumns;
|
||||
MockInteractions.tap(td);
|
||||
assert.isTrue(displayedColumnStub.lastCall.calledWithExactly(
|
||||
originalDisplayedColumns,
|
||||
checkbox.name,
|
||||
checkbox.checked));
|
||||
});
|
||||
|
||||
var columns = element.changeTableItems.length;
|
||||
flushAsynchronousOperations();
|
||||
|
||||
// Tap the add button for the second item.
|
||||
MockInteractions.tap(
|
||||
element.$$('.addOptions').querySelector(
|
||||
'tr:nth-child(2) .add-button'));
|
||||
|
||||
flushAsynchronousOperations();
|
||||
assert.deepEqual(element.changeTableItems, [
|
||||
'Status',
|
||||
'Owner',
|
||||
'Project',
|
||||
'Branch',
|
||||
'Updated',
|
||||
'Size',
|
||||
]);
|
||||
assert.deepEqual(element.changeTableNotDisplayed, ['Subject']);
|
||||
|
||||
// Add remaining item.
|
||||
MockInteractions.tap(
|
||||
element.$$('.addOptions').querySelector('.add-button'));
|
||||
flushAsynchronousOperations();
|
||||
|
||||
assert.deepEqual(element.changeTableNotDisplayed, []);
|
||||
assert.deepEqual(element.changeTableItems, [
|
||||
'Subject',
|
||||
'Status',
|
||||
'Owner',
|
||||
'Project',
|
||||
'Branch',
|
||||
'Updated',
|
||||
'Size'
|
||||
]);
|
||||
test('_updateDisplayedColumns', function() {
|
||||
var name = 'Subject';
|
||||
var checked = false;
|
||||
assert.deepEqual(element._updateDisplayedColumns(columns, name, checked),
|
||||
[
|
||||
'Status',
|
||||
'Owner',
|
||||
'Project',
|
||||
'Branch',
|
||||
'Updated',
|
||||
]);
|
||||
name = 'Size';
|
||||
checked = true;
|
||||
assert.deepEqual(element._updateDisplayedColumns(columns, name, checked),
|
||||
[
|
||||
'Subject',
|
||||
'Status',
|
||||
'Owner',
|
||||
'Project',
|
||||
'Branch',
|
||||
'Updated',
|
||||
'Size',
|
||||
]);
|
||||
});
|
||||
});
|
||||
</script>
|
||||
|
|
|
|||
|
|
@ -314,8 +314,7 @@ limitations under the License.
|
|||
</h2>
|
||||
<fieldset id="changeTableColumns">
|
||||
<gr-change-table-editor
|
||||
change-table-items="{{_localChangeTableColumns}}"
|
||||
change-table-not-displayed="{{_changeTableColumnsNotDisplayed}}">
|
||||
displayed-columns="{{_localChangeTableColumns}}">
|
||||
</gr-change-table-editor>
|
||||
<gr-button
|
||||
id="saveChangeTable"
|
||||
|
|
|
|||
|
|
@ -117,7 +117,7 @@
|
|||
'_handlePrefsChanged(_localPrefs.*)',
|
||||
'_handleDiffPrefsChanged(_diffPrefs.*)',
|
||||
'_handleMenuChanged(_localMenu.splices)',
|
||||
'_handleChangeTableChanged(_localChangeTableColumns.splices)',
|
||||
'_handleChangeTableChanged(_localChangeTableColumns)',
|
||||
],
|
||||
|
||||
attached: function() {
|
||||
|
|
@ -220,7 +220,7 @@
|
|||
var columns = this.prefs.change_table;
|
||||
|
||||
if (columns.length === 0) {
|
||||
columns = this.CHANGE_TABLE_COLUMNS;
|
||||
columns = this.columnNames;
|
||||
this._changeTableColumnsNotDisplayed = [];
|
||||
} else {
|
||||
this._changeTableColumnsNotDisplayed = this.getComplementColumns(
|
||||
|
|
|
|||
|
|
@ -71,6 +71,7 @@ limitations under the License.
|
|||
'diff/gr-syntax-lib-loader/gr-syntax-lib-loader_test.html',
|
||||
'gr-app_test.html',
|
||||
'settings/gr-account-info/gr-account-info_test.html',
|
||||
'settings/gr-change-table-editor/gr-change-table-editor_test.html',
|
||||
'settings/gr-email-editor/gr-email-editor_test.html',
|
||||
'settings/gr-group-list/gr-group-list_test.html',
|
||||
'settings/gr-http-password/gr-http-password_test.html',
|
||||
|
|
|
|||
Loading…
Reference in New Issue