From 1a44b342477f335e58d65157995e20a8070708a6 Mon Sep 17 00:00:00 2001 From: Becky Siegel Date: Mon, 14 Nov 2016 08:37:06 -0800 Subject: [PATCH] Add setting to Show/Hide change table columns in settings This change adds a widget to adjust change table columns in the user's settings. If no change table columns are adjusted, the server returns an empty array and the client determines the default columns, stored in the gr-change-table-behavior. Columns can get removed/re-ordered by the user but they are limited to those specified in gr-change-table-behavior. Column preferences persist across all gr-change-list items (dashboards, searches, etc). Feature: Issue 4753 Change-Id: I47d5b9f53a95c0c010b04c4495094f188d85e67e --- .../gr-change-table-behavior.html | 50 +++++ .../gr-change-table-behavior_test.html | 106 ++++++++++ .../gr-change-list-item.html | 59 ++++-- .../gr-change-list-item.js | 2 + .../gr-change-list-item_test.html | 73 ++++++- .../gr-change-list/gr-change-list.html | 85 +++++--- .../gr-change-list/gr-change-list.js | 10 + .../gr-change-list/gr-change-list_test.html | 127 +++++++++++- .../gr-change-table-editor.html | 95 +++++++++ .../gr-change-table-editor.js | 50 +++++ .../gr-change-table-editor_test.html | 193 ++++++++++++++++++ .../gr-settings-view/gr-settings-view.html | 14 ++ .../gr-settings-view/gr-settings-view.js | 47 +++++ .../gr-settings-view_test.html | 1 + 14 files changed, 852 insertions(+), 60 deletions(-) create mode 100644 polygerrit-ui/app/behaviors/gr-change-table-behavior/gr-change-table-behavior.html create mode 100644 polygerrit-ui/app/behaviors/gr-change-table-behavior/gr-change-table-behavior_test.html create mode 100644 polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.html create mode 100644 polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.js create mode 100644 polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor_test.html diff --git a/polygerrit-ui/app/behaviors/gr-change-table-behavior/gr-change-table-behavior.html b/polygerrit-ui/app/behaviors/gr-change-table-behavior/gr-change-table-behavior.html new file mode 100644 index 0000000000..aee9d7c137 --- /dev/null +++ b/polygerrit-ui/app/behaviors/gr-change-table-behavior/gr-change-table-behavior.html @@ -0,0 +1,50 @@ + + diff --git a/polygerrit-ui/app/behaviors/gr-change-table-behavior/gr-change-table-behavior_test.html b/polygerrit-ui/app/behaviors/gr-change-table-behavior/gr-change-table-behavior_test.html new file mode 100644 index 0000000000..71d831dc07 --- /dev/null +++ b/polygerrit-ui/app/behaviors/gr-change-table-behavior/gr-change-table-behavior_test.html @@ -0,0 +1,106 @@ + + + + +keyboard-shortcut-behavior + + + + + + + + + + + + + + + + diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html index df1ade3250..ad6eaa2879 100644 --- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html +++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html @@ -13,7 +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. --> - + @@ -66,30 +66,51 @@ limitations under the License. } - + ▶ - - - - [[change.subject]] - [[changeStatusString(change)]] - + + + [[change._number]] + + + [[change.subject]] + + + [[changeStatusString(change)]] + + - - [[change.project]] - [[change.branch]] - - + + + [[change.project]] + + + + [[change.branch]] + + + + + + +[[change.insertions]], -[[change.deletions]] - + diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js index 275a8cddee..f8f3c6fa93 100644 --- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js +++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js @@ -18,6 +18,7 @@ is: 'gr-change-list-item', properties: { + visibleChangeTableColumns: Array, selected: { type: Boolean, value: false, @@ -43,6 +44,7 @@ }, behaviors: [ + Gerrit.ChangeTableBehavior, Gerrit.RESTClientBehavior, Gerrit.URLEncodingBehavior, ], diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.html b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.html index b66c70bc38..3df613fa9e 100644 --- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.html +++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.html @@ -22,6 +22,7 @@ limitations under the License. + @@ -45,7 +46,7 @@ limitations under the License. test('change status', function() { var getStatusForChange = function(change) { element.change = change; - return element.$$('.cell.status').textContent; + return element.$$('.cell.status').textContent.trim(); }; assert.equal(getStatusForChange({mergeable: true}), ''); @@ -137,5 +138,75 @@ limitations under the License. assert.equal(element.changeURL, '/c/43/'); }); + test('no hidden columns', function() { + element.visibleChangeTableColumns = [ + 'Subject', + 'Status', + 'Owner', + 'Project', + 'Branch', + 'Updated', + 'Size', + ]; + + flushAsynchronousOperations(); + + element.CHANGE_TABLE_COLUMNS.forEach(function(column) { + var elementClass = '.' + column.toLowerCase(); + assert.isFalse(element.$$(elementClass).hidden); + }); + }); + + test('no hidden columns', function() { + element.visibleChangeTableColumns = [ + 'Subject', + 'Status', + 'Owner', + 'Project', + 'Branch', + 'Updated', + 'Size', + ]; + + flushAsynchronousOperations(); + + element.CHANGE_TABLE_COLUMNS.forEach(function(column) { + var elementClass = '.' + column.toLowerCase(); + assert.isFalse(element.$$(elementClass).hidden); + }); + }); + + test('project column hidden', function() { + element.visibleChangeTableColumns = [ + 'Subject', + 'Status', + 'Owner', + 'Branch', + 'Updated', + 'Size', + ]; + + flushAsynchronousOperations(); + + element.CHANGE_TABLE_COLUMNS.forEach(function(column) { + var elementClass = '.' + column.toLowerCase(); + if (column === 'Project') { + assert.isTrue(element.$$(elementClass).hidden); + } else { + assert.isFalse(element.$$(elementClass).hidden); + } + }); + }); + + test('random column does not exist', function() { + element.visibleChangeTableColumns = [ + 'Bad', + ]; + + flushAsynchronousOperations(); + var elementClass = '.bad'; + assert.isNotOk(element.$$(elementClass)); + }); + }); diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.html b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.html index bd81a484d3..87d4177493 100644 --- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.html +++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.html @@ -15,6 +15,7 @@ limitations under the License. --> + @@ -28,42 +29,60 @@ limitations under the License. display: flex; flex-direction: column; } + #changeList { + border-collapse: collapse; + width: 100%; + } + .cell { + flex-shrink: 0; + padding: .3em .5em; + } + th { + text-align: left; + } -
- - - - Subject - Status - Owner - Project - Branch - Updated - Size - diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js index 3f9de03ff3..2736d38b90 100644 --- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js +++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js @@ -74,6 +74,7 @@ }, behaviors: [ + Gerrit.ChangeTableBehavior, Gerrit.KeyboardShortcutBehavior, Gerrit.RESTClientBehavior, ], @@ -88,15 +89,24 @@ this._loadPreferences(); }, + _lowerCase: function(column) { + return column.toLowerCase(); + }, + _loadPreferences: function() { return this._getLoggedIn().then(function(loggedIn) { + this.changeTableColumns = this.CHANGE_TABLE_COLUMNS; + if (!loggedIn) { this.showNumber = false; + this.visibleChangeTableColumns = this.CHANGE_TABLE_COLUMNS; 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; }.bind(this)); }.bind(this)); }, diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html index 33b4279105..55b278e597 100644 --- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html +++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html @@ -70,9 +70,10 @@ limitations under the License. suite('test show change number preference enabled', function() { setup(function(done) { - return stubRestAPI( - {legacycid_in_change_table: true, time_format: 'HHMM_12'} - ).then(function() { + return stubRestAPI({legacycid_in_change_table: true, + time_format: 'HHMM_12', + change_table: [], + }).then(function() { element = fixture('basic'); element._loadPreferences().then(function() { done(); }); }); @@ -86,9 +87,8 @@ limitations under the License. suite('test show change number preference disabled', function() { setup(function(done) { // legacycid_in_change_table is not set when false. - return stubRestAPI( - {time_format: 'HHMM_12'} - ).then(function() { + return stubRestAPI({time_format: 'HHMM_12', change_table: []}).then( + function() { element = fixture('basic'); element._loadPreferences().then(function() { done(); }); }); @@ -235,6 +235,120 @@ limitations under the License. '.noChanges'); assert.equal(noChangesMsg.length, 2); }); + + suite('empty column preference', function() { + var element; + + setup(function(done) { + return stubRestAPI({ + legacycid_in_change_table: true, + time_format: 'HHMM_12', + change_table: [], + } + ).then(function() { + element = fixture('basic'); + element._loadPreferences().then(function() { done(); }); + }); + }); + + test('show number enabled', function() { + assert.isTrue(element.showNumber); + }); + + test('all columns visible', function() { + element.CHANGE_TABLE_COLUMNS.forEach(function(column) { + var elementClass = '.' + element._lowerCase(column); + assert.isFalse(element.$$(elementClass).hidden); + }); + }); + }); + + suite('full column preference', function() { + var element; + + setup(function(done) { + return stubRestAPI({ + legacycid_in_change_table: true, + time_format: 'HHMM_12', + change_table: [ + 'Subject', + 'Status', + 'Owner', + 'Project', + 'Branch', + 'Updated', + 'Size', + ], + }).then(function() { + element = fixture('basic'); + element._loadPreferences().then(function() { done(); }); + }); + }); + + test('all columns visible', function() { + element.changeTableColumns.forEach(function(column) { + var elementClass = '.' + element._lowerCase(column); + assert.isFalse(element.$$(elementClass).hidden); + }); + }); + }); + + suite('partial column preference', function() { + var element; + + setup(function(done) { + return stubRestAPI({ + legacycid_in_change_table: true, + time_format: 'HHMM_12', + change_table: [ + 'Subject', + 'Status', + 'Owner', + 'Branch', + 'Updated', + 'Size', + ], + }).then(function() { + element = fixture('basic'); + element._loadPreferences().then(function() { done(); }); + }); + }); + + test('all columns except project visible', function() { + element.changeTableColumns.forEach(function(column) { + var elementClass = '.' + column.toLowerCase(); + if (column === 'Project') { + assert.isTrue(element.$$(elementClass).hidden); + } else { + assert.isFalse(element.$$(elementClass).hidden); + } + }); + }); + }); + + suite('random column does not exist', function() { + var element; + + /* This would only exist if somebody manually updated the config + file. */ + setup(function(done) { + return stubRestAPI({ + legacycid_in_change_table: true, + time_format: 'HHMM_12', + change_table: [ + 'Bad', + ], + }).then(function() { + element = fixture('basic'); + element._loadPreferences().then(function() { done(); }); + }); + }); + + test('bad column does not exist', function() { + var elementClass = '.bad'; + assert.isNotOk(element.$$(elementClass)); + }); + }); }); suite('gr-change-list groups', function() { @@ -294,6 +408,5 @@ limitations under the License. 'Should navigate to /c/4/'); showStub.restore(); }); - }); 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 new file mode 100644 index 0000000000..df4947deb7 --- /dev/null +++ b/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.html @@ -0,0 +1,95 @@ + + + + + + + + + + + + + + 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 new file mode 100644 index 0000000000..6486397996 --- /dev/null +++ b/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor.js @@ -0,0 +1,50 @@ +// Copyright (C) 2016 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. +(function() { + 'use strict'; + + Polymer({ + is: 'gr-change-table-editor', + + properties: { + changeTableItems: Array, + changeTableNotDisplayed: Array, + }, + + 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)); + + }, + + _handleAddButton: function(e) { + var index = e.target.dataIndex; + var newColumn = this.changeTableNotDisplayed[index]; + this.splice('changeTableNotDisplayed', index, 1); + + this.splice('changeTableItems', this.getComplementColumns( + this.changeTableNotDisplayed).indexOf(newColumn), 0, newColumn); + }, + }); +})(); 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 new file mode 100644 index 0000000000..0030e2e7d9 --- /dev/null +++ b/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor_test.html @@ -0,0 +1,193 @@ + + + + +gr-settings-view + + + + + + + + + + + + diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html index ab65a51256..273ee4557f 100644 --- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html +++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html @@ -20,6 +20,7 @@ limitations under the License. + @@ -295,6 +296,19 @@ limitations under the License. on-tap="_handleSaveMenu" disabled="[[!_menuChanged]]">Save Changes +

+ Change Table Columns +

+
+ + + Save Changes +

diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js index 34ef958495..cce8f33a49 100644 --- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js +++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js @@ -39,10 +39,15 @@ _accountInfoMutable: Boolean, _accountInfoChanged: Boolean, _diffPrefs: Object, + _changeTableColumnsNotDisplayed: Array, _localPrefs: { type: Object, value: function() { return {}; }, }, + _localChangeTableColumns: { + type: Array, + value: function() { return []; }, + }, _localMenu: { type: Array, value: function() { return []; }, @@ -51,6 +56,10 @@ type: Boolean, value: true, }, + _changeTableChanged: { + type: Boolean, + value: false, + }, _prefsChanged: { type: Boolean, value: false, @@ -89,10 +98,15 @@ _loadingPromise: Object, }, + behaviors: [ + Gerrit.ChangeTableBehavior, + ], + observers: [ '_handlePrefsChanged(_localPrefs.*)', '_handleDiffPrefsChanged(_diffPrefs.*)', '_handleMenuChanged(_localMenu.splices)', + '_handleChangeTableChanged(_localChangeTableColumns.splices)', ], attached: function() { @@ -110,6 +124,7 @@ this.prefs = prefs; this._copyPrefs('_localPrefs', 'prefs'); this._cloneMenu(); + this._cloneChangeTableColumns(); }.bind(this))); promises.push(this.$.restAPI.getDiffPreferences().then(function(prefs) { @@ -179,6 +194,30 @@ this._localMenu = menu; }, + _cloneChangeTableColumns: function() { + var columns = this.prefs.change_table; + + if (columns.length === 0) { + columns = this.CHANGE_TABLE_COLUMNS; + this._changeTableColumnsNotDisplayed = []; + } else { + this._changeTableColumnsNotDisplayed = this.getComplementColumns( + this.prefs.change_table); + } + this._localChangeTableColumns = columns; + }, + + _formatChangeTableColumns: function(changeTableArray) { + return changeTableArray.map(function(item) { + return {column: item}; + }); + }, + + _handleChangeTableChanged: function() { + if (this._isLoading()) { return; } + this._changeTableChanged = true; + }, + _handlePrefsChanged: function(prefs) { if (this._isLoading()) { return; } this._prefsChanged = true; @@ -224,6 +263,14 @@ this.$.syntaxHighlighting.checked); }, + _handleSaveChangeTable: function() { + this.set('prefs.change_table', this._localChangeTableColumns); + this._cloneChangeTableColumns(); + return this.$.restAPI.savePreferences(this.prefs).then(function() { + this._changeTableChanged = false; + }.bind(this)); + }, + _handleSaveDiffPreferences: function() { return this.$.restAPI.saveDiffPreferences(this._diffPrefs) .then(function() { diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html index b9b049ac5c..b5f828d570 100644 --- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html +++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html @@ -88,6 +88,7 @@ limitations under the License. {url: '/first/url', name: 'first name', target: '_blank'}, {url: '/second/url', name: 'second name', target: '_blank'}, ], + change_table: [], }; diffPreferences = { context: 10,