Merge "Prevent duplicates in change_table preference"
diff --git a/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.html b/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.html
index fa188d7..4f69513 100644
--- a/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.html
+++ b/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.html
@@ -37,6 +37,9 @@
cursor: pointer;
text-align: center;
}
+ .checkboxContainer input {
+ cursor: pointer;
+ }
.checkboxContainer:hover {
outline: 1px solid var(--border-color);
}
@@ -52,12 +55,12 @@
<tbody>
<tr>
<td>Number</td>
- <td
- class="checkboxContainer"
- on-tap="_handleTargetTap">
+ <td class="checkboxContainer"
+ on-tap="_handleCheckboxContainerTap">
<input
type="checkbox"
name="number"
+ on-tap="_handleNumberCheckboxTap"
checked$="[[showNumber]]">
</td>
</tr>
@@ -65,10 +68,11 @@
<tr>
<td>[[item]]</td>
<td class="checkboxContainer"
- on-tap="_handleTargetTap">
+ on-tap="_handleCheckboxContainerTap">
<input
type="checkbox"
name="[[item]]"
+ on-tap="_handleTargetTap"
checked$="[[!isColumnHidden(item, displayedColumns)]]">
</td>
</tr>
diff --git a/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.js b/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.js
index 7b74096..7d109633 100644
--- a/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.js
+++ b/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.js
@@ -35,40 +35,42 @@
Gerrit.ChangeTableBehavior,
],
- _getButtonText(isShown) {
- return isShown ? 'Hide' : 'Show';
- },
-
- _updateDisplayedColumns(displayedColumns, name, checked) {
- if (!checked) {
- return displayedColumns.filter(column => {
- return name.toLowerCase() !== column.toLowerCase();
- });
- } else {
- return displayedColumns.concat([name]);
- }
+ /**
+ * Get the list of enabled column names from whichever checkboxes are
+ * checked (excluding the number checkbox).
+ * @return {!Array<string>}
+ */
+ _getDisplayedColumns() {
+ return Polymer.dom(this.root)
+ .querySelectorAll('.checkboxContainer input:not([name=number])')
+ .filter(checkbox => checkbox.checked)
+ .map(checkbox => checkbox.name);
},
/**
- * Handles tap on either the checkbox itself or the surrounding table cell.
+ * Handle a tap on a checkbox container and relay the tap to the checkbox it
+ * contains.
+ */
+ _handleCheckboxContainerTap(e) {
+ const checkbox = Polymer.dom(e.target).querySelector('input');
+ if (!checkbox) { return; }
+ checkbox.click();
+ },
+
+ /**
+ * Handle a tap on the number checkbox and update the showNumber property
+ * accordingly.
+ */
+ _handleNumberCheckboxTap(e) {
+ this.showNumber = Polymer.dom(e).rootTarget.checked;
+ },
+
+ /**
+ * Handle a tap on a displayed column checkboxes (excluding number) and
+ * update the displayedColumns property accordingly.
*/
_handleTargetTap(e) {
- let checkbox = Polymer.dom(e.target).querySelector('input');
- if (checkbox) {
- checkbox.click();
- } else {
- // The target is the checkbox itself.
- checkbox = Polymer.dom(e).rootTarget;
- }
-
- if (checkbox.name === 'number') {
- this.showNumber = checkbox.checked;
- return;
- }
-
- this.set('displayedColumns',
- this._updateDisplayedColumns(
- this.displayedColumns, checkbox.name, checkbox.checked));
+ this.set('displayedColumns', this._getDisplayedColumns());
},
});
})();
diff --git a/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor_test.html b/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor_test.html
index 587cc3b..32fab9d 100644
--- a/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor_test.html
+++ b/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor_test.html
@@ -53,6 +53,7 @@
];
element.set('displayedColumns', columns);
+ element.showNumber = false;
flushAsynchronousOperations();
});
@@ -108,66 +109,50 @@
displayedLength + 1);
});
- test('_handleTargetTap', () => {
- const checkbox = element.$$('table tr:nth-child(2) input');
- let originalDisplayedColumns = element.displayedColumns;
- const td = element.$$('table tr:nth-child(2) .checkboxContainer');
- const displayedColumnStub =
- sandbox.stub(element, '_updateDisplayedColumns');
-
- MockInteractions.tap(checkbox);
- assert.isTrue(displayedColumnStub.lastCall.calledWithExactly(
- originalDisplayedColumns,
- checkbox.name,
- checkbox.checked));
-
- originalDisplayedColumns = element.displayedColumns;
- MockInteractions.tap(td);
- assert.isTrue(displayedColumnStub.lastCall.calledWithExactly(
- originalDisplayedColumns,
- checkbox.name,
- checkbox.checked));
+ test('_getDisplayedColumns', () => {
+ assert.deepEqual(element._getDisplayedColumns(), columns);
+ MockInteractions.tap(
+ element.$$('.checkboxContainer input[name=Assignee]'));
+ assert.deepEqual(element._getDisplayedColumns(),
+ columns.filter(c => c !== 'Assignee'));
});
- test('_handleTargetTap on number', () => {
- element.showNumber = false;
- const checkbox = element.$$('table tr:nth-child(1) input');
- const displayedColumnStub =
- sandbox.stub(element, '_updateDisplayedColumns');
+ test('_handleCheckboxContainerTap relayes taps to checkboxes', () => {
+ sandbox.stub(element, '_handleNumberCheckboxTap');
+ sandbox.stub(element, '_handleTargetTap');
- MockInteractions.tap(checkbox);
- assert.isFalse(displayedColumnStub.called);
+ MockInteractions.tap(
+ element.$$('table tr:first-of-type .checkboxContainer'));
+ assert.isTrue(element._handleNumberCheckboxTap.calledOnce);
+ assert.isFalse(element._handleTargetTap.called);
+
+ MockInteractions.tap(
+ element.$$('table tr:last-of-type .checkboxContainer'));
+ assert.isTrue(element._handleNumberCheckboxTap.calledOnce);
+ assert.isTrue(element._handleTargetTap.calledOnce);
+ });
+
+ test('_handleNumberCheckboxTap', () => {
+ sandbox.spy(element, '_handleNumberCheckboxTap');
+
+ MockInteractions
+ .tap(element.$$('.checkboxContainer input[name=number]'));
+ assert.isTrue(element._handleNumberCheckboxTap.calledOnce);
assert.isTrue(element.showNumber);
- MockInteractions.tap(checkbox);
+ MockInteractions
+ .tap(element.$$('.checkboxContainer input[name=number]'));
+ assert.isTrue(element._handleNumberCheckboxTap.calledTwice);
assert.isFalse(element.showNumber);
});
- test('_updateDisplayedColumns', () => {
- let name = 'Subject';
- let checked = false;
- assert.deepEqual(element._updateDisplayedColumns(columns, name, checked),
- [
- 'Status',
- 'Owner',
- 'Assignee',
- 'Repo',
- 'Branch',
- 'Updated',
- ]);
- name = 'Size';
- checked = true;
- assert.deepEqual(element._updateDisplayedColumns(columns, name, checked),
- [
- 'Subject',
- 'Status',
- 'Owner',
- 'Assignee',
- 'Repo',
- 'Branch',
- 'Updated',
- 'Size',
- ]);
+ test('_handleTargetTap', () => {
+ sandbox.spy(element, '_handleTargetTap');
+ assert.include(element.displayedColumns, 'Assignee');
+ MockInteractions
+ .tap(element.$$('.checkboxContainer input[name=Assignee]'));
+ assert.isTrue(element._handleTargetTap.calledOnce);
+ assert.notInclude(element.displayedColumns, 'Assignee');
});
});
</script>
