Use gr-dropdown-list for diff view file select The diff view file select used its own implementation of a dropdown, which also had two versions, a native select for mobile and a custom design for desktop. gr-autocomplete-dropdown was designed so that it could be used in this scenario as well, as it also has a native select built in. This will also allow us to add comments in the dropdown (follow-up change) and use Gerrit.Nav instead of generated URLs. Change-Id: I5613bb8e327a3f6ae227bf69e64bfafbbfe09b6c
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html index 91d3a3e..1dcf25e 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html +++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
@@ -22,6 +22,7 @@ <link rel="import" href="../../../bower_components/iron-dropdown/iron-dropdown.html"> <link rel="import" href="../../core/gr-navigation/gr-navigation.html"> <link rel="import" href="../../shared/gr-button/gr-button.html"> +<link rel="import" href="../../shared/gr-dropdown-list/gr-dropdown-list.html"> <link rel="import" href="../../shared/gr-fixed-panel/gr-fixed-panel.html"> <link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html"> <link rel="import" href="../../shared/gr-select/gr-select.html"> @@ -80,45 +81,6 @@ .mobile { display: none; } - .dropdown-trigger { - cursor: pointer; - padding: 0; - } - iron-dropdown { - position: absolute; - } - .dropdown-content { - background-color: #fff; - box-shadow: 0 1px 5px rgba(0, 0, 0, .3); - max-height: 70vh; - } - .dropdown-content a { - cursor: pointer; - display: block; - font-weight: normal; - padding: .3em .5em; - } - .dropdown-content a:before { - color: #ccc; - content: attr(data-key-nav); - display: inline-block; - margin-right: .5em; - width: .3em; - } - .dropdown-content a:hover { - background-color: var(--color-link); - color: #fff; - } - .dropdown-content a[selected] { - color: #000; - font-family: var(--font-family-bold); - pointer-events: none; - text-decoration: none; - } - .dropdown-content a[selected]:hover { - background-color: #fff; - color: #000; - } gr-button { padding: .3em 0; text-decoration: none; @@ -147,14 +109,6 @@ display: block; overflow: auto; } - #trigger { - --gr-button: { - -moz-user-select: text; - -ms-user-select: text; - -webkit-user-select: text; - user-select: text; - } - } .editLoaded .hideOnEdit { display: none; } @@ -164,6 +118,11 @@ .blameLoader.show { display: inline; } + gr-dropdown-list { + --trigger-style: { + text-transform: none; + } + } @media screen and (max-width: 50em) { header { padding: .5em var(--default-horizontal-margin); @@ -192,13 +151,6 @@ .reviewed { vertical-align: -.1em; } - .mobileJumpToFileContainer { - display: block; - width: 100%; - } - .mobileJumpToFileContainer select { - width: 100%; - } .mobileNavLink { color: #000; font-size: 1.5em; @@ -208,6 +160,20 @@ .mobileNavLink:not([href]) { color: #bbb; } + .jumpToFileContainer { + display: block; + width: 100%; + } + gr-dropdown-list { + width: 100%; + --gr-select-style: { + display: block; + width: 100%; + } + --native-select-style: { + width: 100%; + } + } } </style> <gr-fixed-panel @@ -226,45 +192,14 @@ type="checkbox" on-change="_handleReviewedChange" hidden$="[[!_loggedIn]]" hidden> - <div class="jumpToFileContainer desktop"> - <gr-button - down-arrow - no-uppercase - link - class="dropdown-trigger" - id="trigger" - on-tap="_showDropdownTapHandler"> - <span>[[computeDisplayPath(_path)]]</span> - </gr-button> - <!-- *-align="" to disable iron-dropdown's element positioning. --> - <iron-dropdown id="dropdown" - allow-outside-scroll - vertical-align="" - horizontal-align=""> - <div class="dropdown-content" slot="dropdown-content"> - <template - is="dom-repeat" - items="[[_fileList]]" - as="path" - initial-count="75"> - <a href$="[[_computeDiffURL(_change, _patchRange.*, path)]]" - selected$="[[_computeFileSelected(path, _path)]]" - data-key-nav$="[[_computeKeyNav(path, _path, _fileList)]]" - on-tap="_handleFileTap">[[computeDisplayPath(path)]]</a> - </template> - </div> - </iron-dropdown> - </div> - <div class="mobileJumpToFileContainer mobile"> - <select on-change="_handleMobileSelectChange"> - <template is="dom-repeat" items="[[_fileList]]" as="path"> - <option - value$="[[path]]" - selected$="[[_computeFileSelected(path, _path)]]"> - [[computeTruncatedPath(path)]] - </option> - </template> - </select> + <div class="jumpToFileContainer"> + <gr-dropdown-list + id="dropdown" + value="[[computeDisplayPath(_path)]]" + on-value-change="_handleFileChange" + items="[[_formattedFiles]]" + initial-count="75"> + </gr-dropdown-list> </div> </h3> <div class="navLinks desktop">
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js index 7c46b58..63c774f 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js +++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -75,6 +75,13 @@ _changeComments: Object, _changeNum: String, _diff: Object, + // An array specifically formatted to be used in a gr-dropdown-list + // element for selected a file to view. + _formattedFiles: { + type: Array, + computed: '_formatFilesForDropdown(_fileList)', + }, + // An sorted array of files, as returned by the rest API. _fileList: { type: Array, value() { return []; }, @@ -586,10 +593,6 @@ patchRange.basePatchNum); }, - _computeDiffURL(change, patchRangeRecord, path) { - return this._getDiffUrl(change, patchRangeRecord.base, path); - }, - _patchRangeStr(patchRange) { let patchStr = patchRange.patchNum; if (patchRange.basePatchNum != null && @@ -638,23 +641,32 @@ return this._getChangePath(change, patchRangeRecord.base, revisions); }, - _computeFileSelected(path, currentPath) { - return path == currentPath; + _formatFilesForDropdown(fileList) { + if (!fileList) { return; } + const dropdownContent = []; + for (const path of fileList) { + dropdownContent.push({ + text: this.computeDisplayPath(path), + mobileText: this.computeTruncatedPath(path), + value: this.computeDisplayPath(path), + }); + } + return dropdownContent; }, _computePrefsButtonHidden(prefs, loggedIn) { return !loggedIn || !prefs; }, - _computeKeyNav(path, selectedPath, fileList) { - const selectedIndex = fileList.indexOf(selectedPath); - if (fileList.indexOf(path) == selectedIndex - 1) { - return '['; + _handleFileChange(e) { + // This is when it gets set initially. + const path = e.detail.value; + if (path === this._path) { + return; } - if (fileList.indexOf(path) == selectedIndex + 1) { - return ']'; - } - return ''; + + Gerrit.Nav.navigateToDiff(this._change, path, this._patchRange.patchNum, + this._patchRange.basePatchNum); }, _handleFileTap(e) { @@ -665,16 +677,6 @@ }, 1); }, - _handleMobileSelectChange(e) { - const path = Polymer.dom(e).rootTarget.value; - Gerrit.Nav.navigateToDiff(this._change, path, this._patchRange.patchNum, - this._patchRange.basePatchNum); - }, - - _showDropdownTapHandler(e) { - this.$.dropdown.open(); - }, - _handlePatchChange(e) { const {basePatchNum, patchNum} = e.detail; if (this.patchNumEquals(basePatchNum, this._patchRange.basePatchNum) &&
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html index 26f91801..ca06b8d 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html +++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
@@ -325,7 +325,7 @@ }); }); - test('jump to file dropdown', () => { + test('_formattedFiles', () => { element._changeNum = '42'; element._patchRange = { basePatchNum: PARENT, @@ -334,45 +334,24 @@ element._change = {_number: 42}; element._fileList = ['chell.go', 'glados.txt', 'wheatley.md']; element._path = 'glados.txt'; - flushAsynchronousOperations(); - const linkEls = - Polymer.dom(element.root).querySelectorAll('.dropdown-content > a'); - assert.equal(linkEls.length, 3); - assert.isFalse(linkEls[0].hasAttribute('selected')); - assert.isTrue(linkEls[1].hasAttribute('selected')); - assert.isFalse(linkEls[2].hasAttribute('selected')); - assert.equal(linkEls[0].getAttribute('data-key-nav'), '['); - assert.equal(linkEls[1].getAttribute('data-key-nav'), ''); - assert.equal(linkEls[2].getAttribute('data-key-nav'), ']'); - assert.equal(linkEls[0].getAttribute('href'), '42-chell.go-10-PARENT'); - assert.equal(linkEls[1].getAttribute('href'), - '42-glados.txt-10-PARENT'); - assert.equal(linkEls[2].getAttribute('href'), - '42-wheatley.md-10-PARENT'); - }); + const expectedFormattedFiles = [ + { + text: 'chell.go', + mobileText: 'chell.go', + value: 'chell.go', + }, { + text: 'glados.txt', + mobileText: 'glados.txt', + value: 'glados.txt', + }, { + text: 'wheatley.md', + mobileText: 'wheatley.md', + value: 'wheatley.md', + }, + ]; - test('jump to file dropdown with patch range', () => { - element._changeNum = '42'; - element._patchRange = { - basePatchNum: '5', - patchNum: '10', - }; - element._change = {_number: 42}; - element._fileList = ['chell.go', 'glados.txt', 'wheatley.md']; - element._path = 'glados.txt'; - flushAsynchronousOperations(); - const linkEls = - Polymer.dom(element.root).querySelectorAll('.dropdown-content > a'); - assert.equal(linkEls.length, 3); - assert.isFalse(linkEls[0].hasAttribute('selected')); - assert.isTrue(linkEls[1].hasAttribute('selected')); - assert.isFalse(linkEls[2].hasAttribute('selected')); - assert.equal(linkEls[0].getAttribute('data-key-nav'), '['); - assert.equal(linkEls[1].getAttribute('data-key-nav'), ''); - assert.equal(linkEls[2].getAttribute('data-key-nav'), ']'); - assert.equal(linkEls[0].getAttribute('href'), '42-chell.go-10-5'); - assert.equal(linkEls[1].getAttribute('href'), '42-glados.txt-10-5'); - assert.equal(linkEls[2].getAttribute('href'), '42-wheatley.md-10-5'); + assert.deepEqual(element._formattedFiles, expectedFormattedFiles); + assert.equal(element._formattedFiles[1].value, element._path); }); test('prev/up/next links', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.html b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.html index f532e3f..a11902e 100644 --- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.html +++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.html
@@ -47,6 +47,14 @@ .filesWeblinks { display: none; } + gr-dropdown-list { + --native-select-style: { + max-width: 5.25em; + } + --dropdown-content-stype: { + max-width: 300px; + } + } } </style> <span class="patchRange">
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.html b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.html index 1638a3f..7391deb 100644 --- a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.html +++ b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.html
@@ -45,9 +45,9 @@ background-color: #fff; box-shadow: 0 1px 5px rgba(0, 0, 0, .3); max-height: 70vh; - margin-top: 1.5em; + margin-top: 2em; min-width: 266px; - max-width: 300px; + @apply --dropdown-content-style } paper-listbox { --paper-listbox: { @@ -114,13 +114,14 @@ @media only screen and (max-width: 50em) { gr-select { display: inline; + @apply --gr-select-style; } gr-button, iron-dropdown { display: none; } select { - max-width: 5.25em; + @apply --native-select-style; } } </style> @@ -144,24 +145,26 @@ attr-for-selected="value" selected="{{value}}" on-tap="_handleDropdownTap"> - <template is="dom-repeat" items="[[items]]"> - <paper-item - disabled="[[item.disabled]]" - value="[[item.value]]"> - <div class="topContent"> - <div>[[item.text]]</div> - <template is="dom-if" if="[[item.date]]"> - <gr-date-formatter - date-str="[[item.date]]"></gr-date-formatter> - </template> - </div> - <template is="dom-if" if="[[item.bottomText]]"> - <div class="bottomContent"> - <div>[[item.bottomText]]</div> - </div> + <template is="dom-repeat" + items="[[items]]" + initial-count="[[initialCount]]"> + <paper-item + disabled="[[item.disabled]]" + value="[[item.value]]"> + <div class="topContent"> + <div>[[item.text]]</div> + <template is="dom-if" if="[[item.date]]"> + <gr-date-formatter + date-str="[[item.date]]"></gr-date-formatter> </template> + </div> + <template is="dom-if" if="[[item.bottomText]]"> + <div class="bottomContent"> + <div>[[item.bottomText]]</div> + </div> + </template> </paper-item> - </template> + </template> </paper-listbox> </iron-dropdown> <gr-select bind-value="{{value}}">
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.js b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.js index 8f6a763..d4225d9 100644 --- a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.js +++ b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.js
@@ -54,6 +54,7 @@ */ properties: { + initialCount: Number, /** @type {!Array<!Defs.item>} */ items: Object, text: String,