commit 3f3d9891967f66b4aa9c45f359d1f9dde6b23b6f Author: Ben Rohlfs Date: Sun Oct 4 20:23:01 2020 +0200 Convert to typescript The change converts the following files to typescript: * elements/diff/gr-diff-view/gr-diff-view.ts Change-Id: Id848d3090f4ea44757fdfed7bc4d8430070e05b1 diff --git a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts index 33c1346..38c3c9c 100644 --- a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts +++ b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts @@ -131,15 +131,20 @@ export class ChangeComments { return this._robotComments; } - findCommentById(commentId: UrlEncodedCommentId): Comment | undefined { - const findComment = (comments: {[path: string]: CommentBasics[]}) => { + findCommentById(commentId?: UrlEncodedCommentId): UIComment | undefined { + if (!commentId) return undefined; + const findComment = (comments: {[path: string]: UIComment[]}) => { let comment; for (const path of Object.keys(comments)) { comment = comment || comments[path].find(c => c.id === commentId); } return comment; }; - return findComment(this._comments) || findComment(this._robotComments); + return ( + findComment(this._comments) || + findComment(this._robotComments) || + findComment(this._drafts) + ); } /** @@ -580,14 +585,14 @@ export class ChangeComments { export const _testOnly_findCommentById = ChangeComments.prototype.findCommentById; -interface GrCommentApi { +export interface GrCommentApi { $: { restAPI: RestApiService & Element; }; } @customElement('gr-comment-api') -class GrCommentApi extends GestureEventListeners( +export class GrCommentApi extends GestureEventListeners( LegacyElementMixin(PolymerElement) ) { static get template() { diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts index e7c9a4a..4f94550 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts +++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts @@ -34,7 +34,6 @@ import { import { Comment, isDraft, - PatchSetFile, sortComments, UIComment, } from '../../../utils/comment-util'; @@ -70,6 +69,7 @@ import {PolymerDeepPropertyChange} from '@polymer/polymer/interfaces'; import {FilesWebLinks} from '../gr-patch-range-select/gr-patch-range-select'; import {LineNumber} from '../gr-diff/gr-diff-line'; import {GrCommentThread} from '../../shared/gr-comment-thread/gr-comment-thread'; +import {PatchSetFile} from '../../../types/types'; const MSG_EMPTY_BLAME = 'No blame information for this diff.'; diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts index fa31d09..155da60 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts +++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts @@ -14,70 +14,132 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import '@polymer/iron-dropdown/iron-dropdown.js'; -import '@polymer/iron-input/iron-input.js'; -import '../../../styles/shared-styles.js'; -import '../../shared/gr-button/gr-button.js'; -import '../../shared/gr-dropdown/gr-dropdown.js'; -import '../../shared/gr-dropdown-list/gr-dropdown-list.js'; -import '../../shared/gr-icons/gr-icons.js'; -import '../../shared/gr-rest-api-interface/gr-rest-api-interface.js'; -import '../../shared/gr-select/gr-select.js'; -import '../../shared/revision-info/revision-info.js'; -import '../gr-comment-api/gr-comment-api.js'; -import '../gr-diff-cursor/gr-diff-cursor.js'; -import '../gr-apply-fix-dialog/gr-apply-fix-dialog.js'; -import '../gr-diff-host/gr-diff-host.js'; -import '../gr-diff-mode-selector/gr-diff-mode-selector.js'; -import '../gr-diff-preferences-dialog/gr-diff-preferences-dialog.js'; -import '../gr-patch-range-select/gr-patch-range-select.js'; -import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js'; -import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners.js'; -import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin.js'; -import {PolymerElement} from '@polymer/polymer/polymer-element.js'; -import {htmlTemplate} from './gr-diff-view_html.js'; -import {KeyboardShortcutMixin, Shortcut} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.js'; -import {GrCountStringFormatter} from '../../shared/gr-count-string-formatter/gr-count-string-formatter.js'; -import {GerritNav} from '../../core/gr-navigation/gr-navigation.js'; -import {RevisionInfo} from '../../shared/revision-info/revision-info.js'; -import {appContext} from '../../../services/app-context.js'; +import '@polymer/iron-dropdown/iron-dropdown'; +import '@polymer/iron-input/iron-input'; +import '../../../styles/shared-styles'; +import '../../shared/gr-button/gr-button'; +import '../../shared/gr-dropdown/gr-dropdown'; +import '../../shared/gr-dropdown-list/gr-dropdown-list'; +import '../../shared/gr-icons/gr-icons'; +import '../../shared/gr-rest-api-interface/gr-rest-api-interface'; +import '../../shared/gr-select/gr-select'; +import '../../shared/revision-info/revision-info'; +import '../gr-comment-api/gr-comment-api'; +import '../gr-diff-cursor/gr-diff-cursor'; +import '../gr-apply-fix-dialog/gr-apply-fix-dialog'; +import '../gr-diff-host/gr-diff-host'; +import '../gr-diff-mode-selector/gr-diff-mode-selector'; +import '../gr-diff-preferences-dialog/gr-diff-preferences-dialog'; +import '../gr-patch-range-select/gr-patch-range-select'; +import {dom, EventApi} from '@polymer/polymer/lib/legacy/polymer.dom'; +import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners'; +import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin'; +import {PolymerElement} from '@polymer/polymer/polymer-element'; +import {htmlTemplate} from './gr-diff-view_html'; +import { + CustomKeyboardEvent, + KeyboardShortcutMixin, + Shortcut, +} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin'; +import {GrCountStringFormatter} from '../../shared/gr-count-string-formatter/gr-count-string-formatter'; +import {GerritNav, GerritView} from '../../core/gr-navigation/gr-navigation'; +import {appContext} from '../../../services/app-context'; import { computeAllPatchSets, computeLatestPatchNum, patchNumEquals, - SPECIAL_PATCH_SET_NUM, -} from '../../../utils/patch-set-util.js'; + PatchSet, +} from '../../../utils/patch-set-util'; +import { + addUnmodifiedFiles, + computeDisplayPath, + computeTruncatedPath, + isMagicPath, + specialFilePathCompare, +} from '../../../utils/path-list-util'; +import {changeBaseURL, changeIsOpen} from '../../../utils/change-util'; +import {customElement, observe, property} from '@polymer/decorators'; +import {RestApiService} from '../../../services/services/gr-rest-api/gr-rest-api'; +import {GrDiffHost} from '../gr-diff-host/gr-diff-host'; +import { + DropdownItem, + GrDropdownList, +} from '../../shared/gr-dropdown-list/gr-dropdown-list'; +import {GrOverlay} from '../../shared/gr-overlay/gr-overlay'; import { - addUnmodifiedFiles, computeDisplayPath, computeTruncatedPath, - isMagicPath, specialFilePathCompare, -} from '../../../utils/path-list-util.js'; -import {changeBaseURL, changeIsOpen} from '../../../utils/change-util.js'; -import {Side} from '../../../constants/constants.js'; + ChangeComments, + GrCommentApi, + TwoSidesComments, +} from '../gr-comment-api/gr-comment-api'; +import {GrDiffModeSelector} from '../gr-diff-mode-selector/gr-diff-mode-selector'; +import { + ChangeInfo, + CommitId, + ConfigInfo, + DiffInfo, + DiffPreferencesInfo, + EditInfo, + EditPatchSetNum, + ElementPropertyDeepChange, + FileInfo, + NumericChangeId, + ParentPatchSetNum, + PatchRange, + PatchSetNum, + PreferencesInfo, + RepoName, + RevisionInfo, +} from '../../../types/common'; +import {ChangeViewState, CommitRange, FileRange} from '../../../types/types'; +import {FilesWebLinks} from '../gr-patch-range-select/gr-patch-range-select'; +import {PolymerDeepPropertyChange} from '@polymer/polymer/interfaces'; +import {GrDiffCursor} from '../gr-diff-cursor/gr-diff-cursor'; +import {CommentSide, DiffViewMode, Side} from '../../../constants/constants'; +import {hasOwnProperty} from '../../../utils/common-util'; +import {GrApplyFixDialog} from '../gr-apply-fix-dialog/gr-apply-fix-dialog'; +import {LineOfInterest} from '../gr-diff/gr-diff'; +import {CommentEventDetail} from '../../shared/gr-comment/gr-comment'; +import {RevisionInfo as RevisionInfoObj} from '../../shared/revision-info/revision-info'; +import {CommentMap} from '../../../utils/comment-util'; +import {AppElementParams} from '../../gr-app-types'; +import {FetchParams} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper'; const ERR_REVIEW_STATUS = 'Couldn’t change file review status.'; const MSG_LOADING_BLAME = 'Loading blame...'; const MSG_LOADED_BLAME = 'Blame loaded'; -const PARENT = 'PARENT'; +interface Files { + sortedFileList: string[]; + changeFilesByPath: {[path: string]: FileInfo}; +} -const DiffSides = { - LEFT: 'left', - RIGHT: 'right', -}; +interface CommentSkips { + previous: string | null; + next: string | null; +} -const DiffViewMode = { - SIDE_BY_SIDE: 'SIDE_BY_SIDE', - UNIFIED: 'UNIFIED_DIFF', -}; +export interface GrDiffView { + $: { + restAPI: RestApiService & Element; + commentAPI: GrCommentApi; + cursor: GrDiffCursor; + diffHost: GrDiffHost; + reviewed: HTMLInputElement; + dropdown: GrDropdownList; + diffPreferencesDialog: GrOverlay; + applyFixDialog: GrApplyFixDialog; + modeSelect: GrDiffModeSelector; + }; +} -/** - * @extends PolymerElement - */ -class GrDiffView extends KeyboardShortcutMixin( - GestureEventListeners(LegacyElementMixin(PolymerElement))) { - static get template() { return htmlTemplate; } +@customElement('gr-diff-view') +export class GrDiffView extends KeyboardShortcutMixin( + GestureEventListeners(LegacyElementMixin(PolymerElement)) +) { + static get template() { + return htmlTemplate; + } - static get is() { return 'gr-diff-view'; } /** * Fired when the title of the page should change. * @@ -90,162 +152,127 @@ class GrDiffView extends KeyboardShortcutMixin( * @event show-alert */ - static get properties() { - return { - /** - * URL params passed from the router. - */ - params: { - type: Object, - observer: '_paramsChanged', - }, - keyEventTarget: { - type: Object, - value() { return document.body; }, - }, - /** - * @type {{ diffMode: (string|undefined) }} - */ - changeViewState: { - type: Object, - notify: true, - value() { return {}; }, - observer: '_changeViewStateChanged', - }, - disableDiffPrefs: { - type: Boolean, - value: false, - }, - _diffPrefsDisabled: { - type: Boolean, - computed: '_computeDiffPrefsDisabled(disableDiffPrefs, _loggedIn)', - }, - /** @type {?} */ - _patchRange: Object, - /** @type {?} */ - _commitRange: Object, - /** - * @type {{ - * subject: string, - * project: string, - * revisions: string, - * }} - */ - _change: Object, - /** @type {?} */ - _changeComments: Object, - _changeNum: String, - /** - * This is a DiffInfo object. - * This is retrieved and owned by a child component. - */ - _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(_files, ' + - '_patchRange.patchNum, _changeComments)', - }, - // An sorted array of files, as returned by the rest API. - _fileList: { - type: Array, - computed: '_getSortedFileList(_files)', - }, - /** - * Contains information about files as returned by the rest API. - * - * @type {{ sortedFileList: Array, changeFilesByPath: Object }} - */ - _files: { - type: Object, - value() { return {sortedFileList: [], changeFilesByPath: {}}; }, - }, + @property({type: Object, observer: '_paramsChanged'}) + params?: AppElementParams; - /** @type {Gerrit.FileRange} */ - _file: { - type: Object, - computed: '_getCurrentFile(_files, _path)', - }, + @property({type: Object}) + keyEventTarget: HTMLElement = document.body; - _path: { - type: String, - observer: '_pathChanged', - }, - _fileNum: { - type: Number, - computed: '_computeFileNum(_path, _formattedFiles)', - }, - _loggedIn: { - type: Boolean, - value: false, - }, - _loading: { - type: Boolean, - value: true, - }, - _prefs: Object, - _projectConfig: Object, - _userPrefs: Object, - _diffMode: { - type: String, - computed: '_getDiffViewMode(changeViewState.diffMode, _userPrefs)', - }, - _isImageDiff: Boolean, - // The return type is FilesWebLinks from gr-patch-range-select. - _filesWeblinks: Object, - - /** - * Map of paths in the current change and patch range that have comments - * or drafts or robot comments. - */ - _commentMap: Object, - - _commentsForDiff: Object, - - /** - * Object to contain the path of the next and previous file in the current - * change and patch range that has comments. - */ - _commentSkips: { - type: Object, - computed: '_computeCommentSkips(_commentMap, _fileList, _path)', - }, - _editMode: { - type: Boolean, - computed: '_computeEditMode(_patchRange.*)', - }, - _isBlameLoaded: Boolean, - _isBlameLoading: { - type: Boolean, - value: false, - }, - _allPatchSets: { - type: Array, - computed: '_computeAllPatchSets(_change, _change.revisions.*)', - }, - _revisionInfo: { - type: Object, - computed: '_getRevisionInfo(_change)', - }, - _reviewedFiles: { - type: Object, - value: () => new Set(), - }, - // line number on the diff which should be scrolled to upon loading - _focusLineNum: Number, - }; - } + @property({type: Object, notify: true, observer: '_changeViewStateChanged'}) + changeViewState: Partial = {}; - static get observers() { - return [ - '_getProjectConfig(_change.project)', - '_getFiles(_changeNum, _patchRange.*, _changeComments)', - '_setReviewedObserver(_loggedIn, params.*, _prefs, _patchRange.*)', - '_recomputeComments(_files.changeFilesByPath,' + - '_path, _patchRange, _projectConfig)', - ]; - } + @property({type: Boolean}) + disableDiffPrefs = false; + + @property({ + type: Boolean, + computed: '_computeDiffPrefsDisabled(disableDiffPrefs, _loggedIn)', + }) + _diffPrefsDisabled?: boolean; + + @property({type: Object}) + _patchRange?: PatchRange; + + @property({type: Object}) + _commitRange?: CommitRange; + + @property({type: Object}) + _change?: ChangeInfo; + + @property({type: Object}) + _changeComments?: ChangeComments; + + @property({type: String}) + _changeNum?: NumericChangeId; + + @property({type: Object}) + _diff?: DiffInfo; + + @property({ + type: Array, + computed: + '_formatFilesForDropdown(_files, ' + + '_patchRange.patchNum, _changeComments)', + }) + _formattedFiles?: DropdownItem[]; + + @property({type: Array, computed: '_getSortedFileList(_files)'}) + _fileList?: string[]; + + @property({type: Object}) + _files: Files = {sortedFileList: [], changeFilesByPath: {}}; + + @property({type: Object, computed: '_getCurrentFile(_files, _path)'}) + _file?: FileInfo; + + @property({type: String, observer: '_pathChanged'}) + _path?: string; + + @property({type: Number, computed: '_computeFileNum(_path, _formattedFiles)'}) + _fileNum?: number; + + @property({type: Boolean}) + _loggedIn = false; + + @property({type: Boolean}) + _loading = true; + + @property({type: Object}) + _prefs?: DiffPreferencesInfo; + + @property({type: Object}) + _projectConfig?: ConfigInfo; + + @property({type: Object}) + _userPrefs?: PreferencesInfo; + + @property({ + type: String, + computed: '_getDiffViewMode(changeViewState.diffMode, _userPrefs)', + }) + _diffMode?: string; + + @property({type: Boolean}) + _isImageDiff?: boolean; + + @property({type: Object}) + _filesWeblinks?: FilesWebLinks; + + @property({type: Object}) + _commentMap?: CommentMap; + + @property({type: Object}) + _commentsForDiff?: TwoSidesComments; + + @property({ + type: Object, + computed: '_computeCommentSkips(_commentMap, _fileList, _path)', + }) + _commentSkips?: CommentSkips; + + @property({type: Boolean, computed: '_computeEditMode(_patchRange.*)'}) + _editMode?: boolean; + + @property({type: Boolean}) + _isBlameLoaded?: boolean; + + @property({type: Boolean}) + _isBlameLoading = false; + + @property({ + type: Array, + computed: '_computeAllPatchSets(_change, _change.revisions.*)', + }) + _allPatchSets?: PatchSet[] = []; + + @property({type: Object, computed: '_getRevisionInfo(_change)'}) + _revisionInfo?: RevisionInfoObj; + + @property({type: Object}) + _reviewedFiles = new Set(); + + @property({type: Number}) + _focusLineNum?: number; get keyBindings() { return { @@ -260,10 +287,8 @@ class GrDiffView extends KeyboardShortcutMixin( [Shortcut.NEXT_LINE]: '_handleNextLineOrFileWithComments', [Shortcut.PREV_LINE]: '_handlePrevLineOrFileWithComments', [Shortcut.VISIBLE_LINE]: '_handleVisibleLine', - [Shortcut.NEXT_FILE_WITH_COMMENTS]: - '_handleNextLineOrFileWithComments', - [Shortcut.PREV_FILE_WITH_COMMENTS]: - '_handlePrevLineOrFileWithComments', + [Shortcut.NEXT_FILE_WITH_COMMENTS]: '_handleNextLineOrFileWithComments', + [Shortcut.PREV_FILE_WITH_COMMENTS]: '_handlePrevLineOrFileWithComments', [Shortcut.NEW_COMMENT]: '_handleNewComment', [Shortcut.SAVE_COMMENT]: null, // DOC_ONLY binding [Shortcut.NEXT_FILE]: '_handleNextFile', @@ -272,12 +297,9 @@ class GrDiffView extends KeyboardShortcutMixin( [Shortcut.NEXT_COMMENT_THREAD]: '_handleNextChunkOrCommentThread', [Shortcut.PREV_CHUNK]: '_handlePrevChunkOrCommentThread', [Shortcut.PREV_COMMENT_THREAD]: '_handlePrevChunkOrCommentThread', - [Shortcut.OPEN_REPLY_DIALOG]: - '_handleOpenReplyDialogOrToggleLeftPane', - [Shortcut.TOGGLE_LEFT_PANE]: - '_handleOpenReplyDialogOrToggleLeftPane', - [Shortcut.OPEN_DOWNLOAD_DIALOG]: - '_handleOpenDownloadDialog', + [Shortcut.OPEN_REPLY_DIALOG]: '_handleOpenReplyDialogOrToggleLeftPane', + [Shortcut.TOGGLE_LEFT_PANE]: '_handleOpenReplyDialogOrToggleLeftPane', + [Shortcut.OPEN_DOWNLOAD_DIALOG]: '_handleOpenDownloadDialog', [Shortcut.UP_TO_CHANGE]: '_handleUpToChange', [Shortcut.OPEN_DIFF_PREFS]: '_handleCommaKey', [Shortcut.TOGGLE_DIFF_MODE]: '_handleToggleDiffMode', @@ -286,14 +308,12 @@ class GrDiffView extends KeyboardShortcutMixin( [Shortcut.NEXT_UNREVIEWED_FILE]: '_handleNextUnreviewedFile', [Shortcut.TOGGLE_BLAME]: '_handleToggleBlame', [Shortcut.TOGGLE_HIDE_ALL_COMMENT_THREADS]: - '_handleToggleHideAllCommentThreads', + '_handleToggleHideAllCommentThreads', [Shortcut.DIFF_AGAINST_BASE]: '_handleDiffAgainstBase', [Shortcut.DIFF_AGAINST_LATEST]: '_handleDiffAgainstLatest', [Shortcut.DIFF_BASE_AGAINST_LEFT]: '_handleDiffBaseAgainstLeft', - [Shortcut.DIFF_RIGHT_AGAINST_LATEST]: - '_handleDiffRightAgainstLatest', - [Shortcut.DIFF_BASE_AGAINST_LATEST]: - '_handleDiffBaseAgainstLatest', + [Shortcut.DIFF_RIGHT_AGAINST_LATEST]: '_handleDiffRightAgainstLatest', + [Shortcut.DIFF_BASE_AGAINST_LATEST]: '_handleDiffBaseAgainstLatest', // Final two are actually handled by gr-comment-thread. [Shortcut.EXPAND_ALL_COMMENT_THREADS]: null, @@ -301,16 +321,20 @@ class GrDiffView extends KeyboardShortcutMixin( }; } - constructor() { - super(); - this.reporting = appContext.reportingService; - this.flagsService = appContext.flagsService; - } + reporting = appContext.reportingService; + + flagsService = appContext.flagsService; + + _throttledToggleFileReviewed?: EventListener; + _onRenderHandler?: EventListener; + + /** @override */ connectedCallback() { super.connectedCallback(); this._throttledToggleFileReviewed = this._throttleWrap(e => - this._handleToggleFileReviewed(e)); + this._handleToggleFileReviewed(e as CustomKeyboardEvent) + ); } /** @override */ @@ -320,66 +344,75 @@ class GrDiffView extends KeyboardShortcutMixin( this._loggedIn = loggedIn; }); - this.addEventListener('open-fix-preview', - e => this._onOpenFixPreview(e)); + this.addEventListener('open-fix-preview', e => + this._onOpenFixPreview(e as CustomEvent) + ); this.$.cursor.push('diffs', this.$.diffHost); - this._onRenderHandler = () => { + this._onRenderHandler = (_: Event) => { this.$.cursor.reInitCursor(); }; this.$.diffHost.addEventListener('render', this._onRenderHandler); } + /** @override */ detached() { - this.$.diffHost.removeEventListener('render', this._onRenderHandler); + if (this._onRenderHandler) { + this.$.diffHost.removeEventListener('render', this._onRenderHandler); + } } _getLoggedIn() { return this.$.restAPI.getLoggedIn(); } - _getProjectConfig(project) { + @observe('_change.project') + _getProjectConfig(project?: RepoName) { if (!project) return; - return this.$.restAPI.getProjectConfig(project).then( - config => { - this._projectConfig = config; - }); + return this.$.restAPI.getProjectConfig(project).then(config => { + this._projectConfig = config; + }); } - _getChangeDetail(changeNum) { + _getChangeDetail(changeNum: NumericChangeId) { return this.$.restAPI.getDiffChangeDetail(changeNum).then(change => { + if (!change) throw new Error('Missing "change" in API response.'); this._change = change; return change; }); } - _getChangeEdit(changeNum) { + _getChangeEdit() { + if (!this._changeNum) throw new Error('Missing this._changeNum'); return this.$.restAPI.getChangeEdit(this._changeNum); } - _getSortedFileList(files) { + _getSortedFileList(files?: Files) { if (!files) return []; return files.sortedFileList; } - /** - * @param {!Object} files - * @param {string} path - * @returns {!Gerrit.FileRange} - */ - _getCurrentFile(files, path) { - if ([files, path].includes(undefined)) return; + _getCurrentFile(files?: Files, path?: string) { + if (!files || !path) return; const fileInfo = files.changeFilesByPath[path]; - const fileRange = {path}; + const fileRange: FileRange = {path}; if (fileInfo && fileInfo.old_path) { fileRange.basePath = fileInfo.old_path; } return fileRange; } - _getFiles(changeNum, patchRangeRecord, changeComments) { + @observe('_changeNum', '_patchRange.*', '_changeComments') + _getFiles( + changeNum: NumericChangeId, + patchRangeRecord: PolymerDeepPropertyChange, + changeComments: ChangeComments + ) { // Polymer 2: check for undefined - if ([changeNum, patchRangeRecord, patchRangeRecord.base, changeComments] - .some(arg => arg === undefined)) { + if ( + [changeNum, patchRangeRecord, patchRangeRecord.base, changeComments].some( + arg => arg === undefined + ) + ) { return Promise.resolve(); } @@ -388,17 +421,18 @@ class GrDiffView extends KeyboardShortcutMixin( } const patchRange = patchRangeRecord.base; - return this.$.restAPI.getChangeFiles( - changeNum, patchRange).then(changeFiles => { - if (!changeFiles) return; - const commentedPaths = changeComments.getPaths(patchRange); - const files = {...changeFiles}; - addUnmodifiedFiles(files, commentedPaths); - this._files = { - sortedFileList: Object.keys(files).sort(specialFilePathCompare), - changeFilesByPath: files, - }; - }); + return this.$.restAPI + .getChangeFiles(changeNum, patchRange) + .then(changeFiles => { + if (!changeFiles) return; + const commentedPaths = changeComments.getPaths(patchRange); + const files = {...changeFiles}; + addUnmodifiedFiles(files, commentedPaths); + this._files = { + sortedFileList: Object.keys(files).sort(specialFilePathCompare), + changeFilesByPath: files, + }; + }); } _getDiffPreferences() { @@ -415,91 +449,115 @@ class GrDiffView extends KeyboardShortcutMixin( return window.innerWidth; } - _handleReviewedChange(e) { - this._setReviewed(dom(e).rootTarget.checked); + _handleReviewedChange(e: Event) { + this._setReviewed( + ((dom(e) as EventApi).rootTarget as HTMLInputElement).checked + ); } - _setReviewed(reviewed) { - if (this._editMode) { return; } + _setReviewed(reviewed: boolean) { + if (this._editMode) return; this.$.reviewed.checked = reviewed; - if (!this._patchRange.patchNum) return; + if (!this._patchRange?.patchNum) return; this._saveReviewedState(reviewed).catch(err => { - this.dispatchEvent(new CustomEvent('show-alert', { - detail: {message: ERR_REVIEW_STATUS}, - composed: true, bubbles: true, - })); + this.dispatchEvent( + new CustomEvent('show-alert', { + detail: {message: ERR_REVIEW_STATUS}, + composed: true, + bubbles: true, + }) + ); throw err; }); } - _saveReviewedState(reviewed) { - return this.$.restAPI.saveFileReviewed(this._changeNum, - this._patchRange.patchNum, this._path, reviewed); + _saveReviewedState(reviewed: boolean): Promise { + if (!this._changeNum) return Promise.resolve(undefined); + if (!this._patchRange?.patchNum) return Promise.resolve(undefined); + if (!this._path) return Promise.resolve(undefined); + return this.$.restAPI.saveFileReviewed( + this._changeNum, + this._patchRange?.patchNum, + this._path, + reviewed + ); } - _handleToggleFileReviewed(e) { - if (this.shouldSuppressKeyboardShortcut(e) || - this.modifierPressed(e)) { return; } + _handleToggleFileReviewed(e: CustomKeyboardEvent) { + if (this.shouldSuppressKeyboardShortcut(e)) return; + if (this.modifierPressed(e)) return; e.preventDefault(); this._setReviewed(!this.$.reviewed.checked); } - _handleEscKey(e) { - if (this.shouldSuppressKeyboardShortcut(e) || - this.modifierPressed(e)) { return; } + _handleEscKey(e: CustomKeyboardEvent) { + if (this.shouldSuppressKeyboardShortcut(e)) return; + if (this.modifierPressed(e)) return; e.preventDefault(); this.$.diffHost.displayLine = false; } - _handleLeftPane(e) { - if (this.shouldSuppressKeyboardShortcut(e)) { return; } + _handleLeftPane(e: CustomKeyboardEvent) { + if (this.shouldSuppressKeyboardShortcut(e)) return; e.preventDefault(); this.$.cursor.moveLeft(); } - _handleRightPane(e) { - if (this.shouldSuppressKeyboardShortcut(e)) { return; } + _handleRightPane(e: CustomKeyboardEvent) { + if (this.shouldSuppressKeyboardShortcut(e)) return; e.preventDefault(); this.$.cursor.moveRight(); } - _handlePrevLineOrFileWithComments(e) { - if (this.shouldSuppressKeyboardShortcut(e)) { return; } - if (e.detail.keyboardEvent.shiftKey && - e.detail.keyboardEvent.keyCode === 75) { // 'K' + _handlePrevLineOrFileWithComments(e: CustomKeyboardEvent) { + if (this.shouldSuppressKeyboardShortcut(e)) return; + + if ( + e.detail.keyboardEvent?.shiftKey && + e.detail.keyboardEvent?.keyCode === 75 + ) { + // 'K' this._moveToPreviousFileWithComment(); return; } - if (this.modifierPressed(e)) { return; } + if (this.modifierPressed(e)) { + return; + } e.preventDefault(); this.$.diffHost.displayLine = true; this.$.cursor.moveUp(); } - _handleVisibleLine(e) { - if (this.shouldSuppressKeyboardShortcut(e)) { return; } + _handleVisibleLine(e: CustomKeyboardEvent) { + if (this.shouldSuppressKeyboardShortcut(e)) return; e.preventDefault(); this.$.cursor.moveToVisibleArea(); } - _onOpenFixPreview(e) { + _onOpenFixPreview(e: CustomEvent) { this.$.applyFixDialog.open(e); } - _handleNextLineOrFileWithComments(e) { - if (this.shouldSuppressKeyboardShortcut(e)) { return; } - if (e.detail.keyboardEvent.shiftKey && - e.detail.keyboardEvent.keyCode === 74) { // 'J' + _handleNextLineOrFileWithComments(e: CustomKeyboardEvent) { + if (this.shouldSuppressKeyboardShortcut(e)) return; + + if ( + e.detail.keyboardEvent?.shiftKey && + e.detail.keyboardEvent?.keyCode === 74 + ) { + // 'J' this._moveToNextFileWithComment(); return; } - if (this.modifierPressed(e)) { return; } + if (this.modifierPressed(e)) { + return; + } e.preventDefault(); this.$.diffHost.displayLine = true; @@ -507,7 +565,9 @@ class GrDiffView extends KeyboardShortcutMixin( } _moveToPreviousFileWithComment() { - if (!this._commentSkips) { return; } + if (!this._commentSkips) return; + if (!this._change) return; + if (!this._patchRange?.patchNum) return; // If there is no previous diff with comments, then return to the change // view. @@ -516,12 +576,18 @@ class GrDiffView extends KeyboardShortcutMixin( return; } - GerritNav.navigateToDiff(this._change, this._commentSkips.previous, - this._patchRange.patchNum, this._patchRange.basePatchNum); + GerritNav.navigateToDiff( + this._change, + this._commentSkips.previous, + this._patchRange.patchNum, + this._patchRange.basePatchNum + ); } _moveToNextFileWithComment() { - if (!this._commentSkips) { return; } + if (!this._commentSkips) return; + if (!this._change) return; + if (!this._patchRange?.patchNum) return; // If there is no next diff with comments, then return to the change view. if (!this._commentSkips.next) { @@ -529,107 +595,119 @@ class GrDiffView extends KeyboardShortcutMixin( return; } - GerritNav.navigateToDiff(this._change, this._commentSkips.next, - this._patchRange.patchNum, this._patchRange.basePatchNum); + GerritNav.navigateToDiff( + this._change, + this._commentSkips.next, + this._patchRange.patchNum, + this._patchRange.basePatchNum + ); } - _handleNewComment(e) { - if (this.shouldSuppressKeyboardShortcut(e) || - this.modifierPressed(e)) { return; } + _handleNewComment(e: CustomKeyboardEvent) { + if (this.shouldSuppressKeyboardShortcut(e)) return; + if (this.modifierPressed(e)) return; + e.preventDefault(); this.$.cursor.createCommentInPlace(); } - _handlePrevFile(e) { + _handlePrevFile(e: CustomKeyboardEvent) { + if (this.shouldSuppressKeyboardShortcut(e)) return; // Check for meta key to avoid overriding native chrome shortcut. - if (this.shouldSuppressKeyboardShortcut(e) || - this.getKeyboardEvent(e).metaKey) { return; } + if (this.getKeyboardEvent(e).metaKey) return; + if (!this._path) return; + if (!this._fileList) return; e.preventDefault(); this._navToFile(this._path, this._fileList, -1); } - _handleNextFile(e) { + _handleNextFile(e: CustomKeyboardEvent) { + if (this.shouldSuppressKeyboardShortcut(e)) return; // Check for meta key to avoid overriding native chrome shortcut. - if (this.shouldSuppressKeyboardShortcut(e) || - this.getKeyboardEvent(e).metaKey) { return; } + if (this.getKeyboardEvent(e).metaKey) return; + if (!this._path) return; + if (!this._fileList) return; e.preventDefault(); this._navToFile(this._path, this._fileList, 1); } - _handleNextChunkOrCommentThread(e) { - if (this.shouldSuppressKeyboardShortcut(e)) { return; } + _handleNextChunkOrCommentThread(e: CustomKeyboardEvent) { + if (this.shouldSuppressKeyboardShortcut(e)) return; e.preventDefault(); - if (e.detail.keyboardEvent.shiftKey) { + if (e.detail.keyboardEvent?.shiftKey) { this.$.cursor.moveToNextCommentThread(); } else { - if (this.modifierPressed(e)) { return; } + if (this.modifierPressed(e)) return; // navigate to next file if key is not being held down - this.$.cursor.moveToNextChunk(/* opt_clipToTop = */false, - /* opt_navigateToNextFile = */!e.detail.keyboardEvent.repeat); + this.$.cursor.moveToNextChunk( + /* opt_clipToTop = */ false, + /* opt_navigateToNextFile = */ !e.detail.keyboardEvent?.repeat + ); } } - _handlePrevChunkOrCommentThread(e) { - if (this.shouldSuppressKeyboardShortcut(e)) { return; } + _handlePrevChunkOrCommentThread(e: CustomKeyboardEvent) { + if (this.shouldSuppressKeyboardShortcut(e)) return; e.preventDefault(); - if (e.detail.keyboardEvent.shiftKey) { + if (e.detail.keyboardEvent?.shiftKey) { this.$.cursor.moveToPreviousCommentThread(); } else { - if (this.modifierPressed(e)) { return; } + if (this.modifierPressed(e)) return; this.$.cursor.moveToPreviousChunk(); } } - _handleOpenReplyDialogOrToggleLeftPane(e) { - if (this.shouldSuppressKeyboardShortcut(e)) { return; } + _handleOpenReplyDialogOrToggleLeftPane(e: CustomKeyboardEvent) { + if (this.shouldSuppressKeyboardShortcut(e)) return; - if (e.detail.keyboardEvent.shiftKey) { // Hide left diff. + if (e.detail.keyboardEvent?.shiftKey) { + // Hide left diff. e.preventDefault(); this.$.diffHost.toggleLeftDiff(); return; } - if (this.modifierPressed(e)) { return; } - - if (!this._loggedIn) { return; } + if (this.modifierPressed(e)) return; + if (!this._loggedIn) return; this.set('changeViewState.showReplyDialog', true); e.preventDefault(); this._navToChangeView(); } - _handleOpenDownloadDialog(e) { - if (this.shouldSuppressKeyboardShortcut(e)) { return; } - if (this.modifierPressed(e)) { return; } + _handleOpenDownloadDialog(e: CustomKeyboardEvent) { + if (this.shouldSuppressKeyboardShortcut(e)) return; + if (this.modifierPressed(e)) return; + this.set('changeViewState.showDownloadDialog', true); e.preventDefault(); this._navToChangeView(); } - _handleUpToChange(e) { - if (this.shouldSuppressKeyboardShortcut(e) || - this.modifierPressed(e)) { return; } + _handleUpToChange(e: CustomKeyboardEvent) { + if (this.shouldSuppressKeyboardShortcut(e)) return; + if (this.modifierPressed(e)) return; e.preventDefault(); this._navToChangeView(); } - _handleCommaKey(e) { - if (this.shouldSuppressKeyboardShortcut(e) || - this.modifierPressed(e)) { return; } - if (this._diffPrefsDisabled) { return; } + _handleCommaKey(e: CustomKeyboardEvent) { + if (this.shouldSuppressKeyboardShortcut(e)) return; + if (this.modifierPressed(e)) return; + if (this._diffPrefsDisabled) return; e.preventDefault(); this.$.diffPreferencesDialog.open(); } - _handleToggleDiffMode(e) { - if (this.shouldSuppressKeyboardShortcut(e) || - this.modifierPressed(e)) { return; } + _handleToggleDiffMode(e: CustomKeyboardEvent) { + if (this.shouldSuppressKeyboardShortcut(e)) return; + if (this.modifierPressed(e)) return; e.preventDefault(); if (this._getDiffViewMode() === DiffViewMode.SIDE_BY_SIDE) { @@ -640,91 +718,122 @@ class GrDiffView extends KeyboardShortcutMixin( } _navToChangeView() { - if (!this._changeNum || !this._patchRange.patchNum) { return; } + if (!this._changeNum || !this._patchRange?.patchNum) { + return; + } this._navigateToChange( - this._change, - this._patchRange, - this._change && this._change.revisions); + this._change, + this._patchRange, + this._change && this._change.revisions + ); } - _navToFile(path, fileList, direction) { + _navToFile(path: string, fileList: string[], direction: -1 | 1) { const newPath = this._getNavLinkPath(path, fileList, direction); - if (!newPath) { return; } + if (!newPath?.path) return; + if (!this._change) return; + if (!this._patchRange) return; if (newPath.up) { this._navigateToChange( - this._change, - this._patchRange, - this._change && this._change.revisions); + this._change, + this._patchRange, + this._change && this._change.revisions + ); return; } - GerritNav.navigateToDiff(this._change, newPath.path, - this._patchRange.patchNum, this._patchRange.basePatchNum); + GerritNav.navigateToDiff( + this._change, + newPath.path, + this._patchRange.patchNum, + this._patchRange.basePatchNum + ); } /** - * @param {?string} path The path of the current file being shown. - * @param {!Array} fileList The list of files in this change and - * patch range. - * @param {number} direction Either 1 (next file) or -1 (prev file). - * @param {(number|boolean)} opt_noUp Whether to return to the change view - * when advancing the file goes outside the bounds of fileList. - * - * @return {?string} The next URL when proceeding in the specified - * direction. + * @param path The path of the current file being shown. + * @param fileList The list of files in this change and + * patch range. + * @param direction Either 1 (next file) or -1 (prev file). + * @param opt_noUp Whether to return to the change view + * when advancing the file goes outside the bounds of fileList. + * @return The next URL when proceeding in the specified + * direction. */ - _computeNavLinkURL(change, path, fileList, direction, opt_noUp) { + _computeNavLinkURL( + change?: ChangeInfo, + path?: string, + fileList?: string[], + direction?: -1 | 1, + opt_noUp?: boolean + ) { + if (!change) return null; + if (!path) return null; + if (!fileList) return null; + if (!direction) return null; + const newPath = this._getNavLinkPath(path, fileList, direction, opt_noUp); - if (!newPath) { return null; } + if (!newPath) { + return null; + } if (newPath.up) { return this._getChangePath( - this._change, - this._patchRange, - this._change && this._change.revisions); + this._change, + this._patchRange, + this._change && this._change.revisions + ); } return this._getDiffUrl(this._change, this._patchRange, newPath.path); } _goToEditFile() { + if (!this._change) return; + if (!this._path) return; + if (!this._patchRange) return; + // TODO(taoalpha): add a shortcut for editing const cursorAddress = this.$.cursor.getAddress(); const editUrl = GerritNav.getEditUrlForDiff( - this._change, - this._path, - this._patchRange.patchNum, - cursorAddress && cursorAddress.number + this._change, + this._path, + this._patchRange.patchNum, + cursorAddress?.number ); - return GerritNav.navigateToRelativeUrl(editUrl); + GerritNav.navigateToRelativeUrl(editUrl); } /** * Gives an object representing the target of navigating either left or * right through the change. The resulting object will have one of the * following forms: - * * {path: ""} - When another file path should be the - * result of the navigation. - * * {up: true} - When the result of navigating should go back to the - * change view. - * * null - When no navigation is possible for the given direction. + * * {path: ""} - When another file path should be the + * result of the navigation. + * * {up: true} - When the result of navigating should go back to the + * change view. + * * null - When no navigation is possible for the given direction. * - * @param {?string} path The path of the current file being shown. - * @param {!Array} fileList The list of files in this change and - * patch range. - * @param {number} direction Either 1 (next file) or -1 (prev file). - * @param {?number|boolean=} opt_noUp Whether to return to the change view - * when advancing the file goes outside the bounds of fileList. - * @return {?Object} + * @param path The path of the current file being shown. + * @param fileList The list of files in this change and + * patch range. + * @param direction Either 1 (next file) or -1 (prev file). + * @param opt_noUp Whether to return to the change view + * when advancing the file goes outside the bounds of fileList. */ - _getNavLinkPath(path, fileList, direction, opt_noUp) { - if (!path || !fileList || fileList.length === 0) { return null; } + _getNavLinkPath( + path: string, + fileList: string[], + direction: -1 | 1, + opt_noUp?: boolean + ) { + if (!path || !fileList || fileList.length === 0) { + return null; + } let idx = fileList.indexOf(path); if (idx === -1) { - const file = direction > 0 ? - fileList[0] : - fileList[fileList.length - 1]; + const file = direction > 0 ? fileList[0] : fileList[fileList.length - 1]; return {path: file}; } @@ -732,66 +841,91 @@ class GrDiffView extends KeyboardShortcutMixin( // Redirect to the change view if opt_noUp isn’t truthy and idx falls // outside the bounds of [0, fileList.length). if (idx < 0 || idx > fileList.length - 1) { - if (opt_noUp) { return null; } + if (opt_noUp) { + return null; + } return {up: true}; } return {path: fileList[idx]}; } - _getReviewedFiles(changeNum, patchNum) { - return this.$.restAPI.getReviewedFiles(changeNum, patchNum) - .then(files => { - this._reviewedFiles = new Set(files); - return this._reviewedFiles; - }); + _getReviewedFiles( + changeNum?: NumericChangeId, + patchNum?: PatchSetNum + ): Promise> { + if (!changeNum || !patchNum) return Promise.resolve(new Set()); + return this.$.restAPI.getReviewedFiles(changeNum, patchNum).then(files => { + this._reviewedFiles = new Set(files); + return this._reviewedFiles; + }); } - _getReviewedStatus(editMode, changeNum, patchNum, path) { - if (editMode) { return Promise.resolve(false); } - return this._getReviewedFiles(changeNum, patchNum) - .then(files => files.has(path)); + _getReviewedStatus( + editMode?: boolean, + changeNum?: NumericChangeId, + patchNum?: PatchSetNum, + path?: string + ) { + if (editMode || !path) { + return Promise.resolve(false); + } + return this._getReviewedFiles(changeNum, patchNum).then(files => + files.has(path) + ); } - _initLineOfInterestAndCursor(leftSide) { - this.$.diffHost.lineOfInterest = - this._getLineOfInterest({ - leftSide, - }); + _initLineOfInterestAndCursor(leftSide: boolean) { + this.$.diffHost.lineOfInterest = this._getLineOfInterest({ + leftSide, + }); this._initCursor({ leftSide, }); } _displayDiffBaseAgainstLeftToast() { - this.dispatchEvent(new CustomEvent('show-alert', { - detail: { - // \u2190 = ← - message: `Patchset ${this._patchRange.basePatchNum} vs ` + - `${this._patchRange.patchNum} selected. Press v + \u2190 to view ` - + `Base vs ${this._patchRange.basePatchNum}`, - }, - composed: true, bubbles: true, - })); + if (!this._patchRange) return; + this.dispatchEvent( + new CustomEvent('show-alert', { + detail: { + // \u2190 = ← + message: + `Patchset ${this._patchRange.basePatchNum} vs ` + + `${this._patchRange.patchNum} selected. Press v + \u2190 to view ` + + `Base vs ${this._patchRange.basePatchNum}`, + }, + composed: true, + bubbles: true, + }) + ); } - _displayDiffAgainstLatestToast(latestPatchNum) { + _displayDiffAgainstLatestToast(latestPatchNum?: PatchSetNum) { + if (!this._patchRange) return; const leftPatchset = patchNumEquals( - this._patchRange.basePatchNum, 'PARENT') - ? 'Base' : `Patchset ${this._patchRange.basePatchNum}`; - this.dispatchEvent(new CustomEvent('show-alert', { - detail: { - // \u2191 = ↑ - message: `${leftPatchset} vs + this._patchRange.basePatchNum, + ParentPatchSetNum + ) + ? 'Base' + : `Patchset ${this._patchRange.basePatchNum}`; + this.dispatchEvent( + new CustomEvent('show-alert', { + detail: { + // \u2191 = ↑ + message: `${leftPatchset} vs ${this._patchRange.patchNum} selected\n. Press v + \u2191 to view ${leftPatchset} vs Patchset ${latestPatchNum}`, - }, - composed: true, bubbles: true, - })); + }, + composed: true, + bubbles: true, + }) + ); } _displayToasts() { - if (!patchNumEquals(this._patchRange.basePatchNum, 'PARENT')) { + if (!this._patchRange) return; + if (!patchNumEquals(this._patchRange.basePatchNum, ParentPatchSetNum)) { this._displayDiffBaseAgainstLeftToast(); return; } @@ -803,48 +937,60 @@ class GrDiffView extends KeyboardShortcutMixin( } _initCommitRange() { - let commit; - let baseCommit; + let commit: CommitId | undefined; + let baseCommit: CommitId | undefined; + if (!this._change) return; if (!this._patchRange || !this._patchRange.patchNum) return; for (const commitSha in this._change.revisions) { - if (!this._change.revisions.hasOwnProperty(commitSha)) continue; + if (!hasOwnProperty(this._change.revisions, commitSha)) continue; const revision = this._change.revisions[commitSha]; const patchNum = revision._number.toString(); if (patchNum === this._patchRange.patchNum) { - commit = commitSha; - const commitObj = revision.commit || {}; - const parents = commitObj.parents || []; - if (this._patchRange.basePatchNum === PARENT && parents.length) { + commit = commitSha as CommitId; + const commitObj = revision.commit; + const parents = commitObj?.parents || []; + if ( + this._patchRange.basePatchNum === ParentPatchSetNum && + parents.length + ) { baseCommit = parents[parents.length - 1].commit; } } else if (patchNum === this._patchRange.basePatchNum) { - baseCommit = commitSha; + baseCommit = commitSha as CommitId; } } - this._commitRange = {commit, baseCommit}; + this._commitRange = commit && baseCommit ? {commit, baseCommit} : undefined; } _initPatchRange() { - let leftSide; - if (this.params.commentId) { - const comment = this._changeComments.findCommentById( - this.params.commentId); + let leftSide = false; + if (!this._change) return; + if (this.params?.view !== GerritView.DIFF) return; + if (this.params?.commentId) { + const comment = this._changeComments?.findCommentById( + this.params.commentId + ); if (!comment) { - this.dispatchEvent(new CustomEvent('show-alert', { - detail: { - message: 'comment not found', - }, - composed: true, bubbles: true, - })); + this.dispatchEvent( + new CustomEvent('show-alert', { + detail: { + message: 'comment not found', + }, + composed: true, + bubbles: true, + }) + ); GerritNav.navigateToChange(this._change); return; } this._path = comment.path; const latestPatchNum = computeLatestPatchNum(this._allPatchSets); + if (!comment.patch_set) throw new Error('Missing comment.patch_set'); + if (!latestPatchNum) throw new Error('Missing _allPatchSets'); if (patchNumEquals(latestPatchNum, comment.patch_set)) { this._patchRange = { patchNum: latestPatchNum, - basePatchNum: PARENT, + basePatchNum: ParentPatchSetNum, }; leftSide = comment.__commentSide === 'left'; } else { @@ -864,34 +1010,40 @@ class GrDiffView extends KeyboardShortcutMixin( if (this.params.patchNum) { this._patchRange = { patchNum: this.params.patchNum, - basePatchNum: this.params.basePatchNum || PARENT, + basePatchNum: this.params.basePatchNum || ParentPatchSetNum, }; } if (this.params.lineNum) { this._focusLineNum = this.params.lineNum; - leftSide = this.params.leftSide; + leftSide = !!this.params.leftSide; } } + if (!this._patchRange) throw new Error('Failed to initialize patchRange.'); this._initLineOfInterestAndCursor(leftSide); this._commentMap = this._getPaths(this._patchRange); - this._commentsForDiff = this._getCommentsForPath(this._path, - this._patchRange, this._projectConfig); + this._commentsForDiff = this._getCommentsForPath( + this._path, + this._patchRange, + this._projectConfig + ); } - _isFileUnchanged(diff) { + _isFileUnchanged(diff: DiffInfo) { if (!diff || !diff.content) return false; - return !diff.content.some(content => - (content.a && !content.common) || - (content.b && !content.common) + return !diff.content.some( + content => + (content.a && !content.common) || (content.b && !content.common) ); } - _paramsChanged(value) { - if (value.view !== GerritNav.View.DIFF) { return; } + _paramsChanged(value: AppElementParams) { + if (value.view !== GerritView.DIFF) { + return; + } this._change = undefined; - this._files = undefined; + this._files = {sortedFileList: [], changeFilesByPath: {}}; this._path = undefined; this._patchRange = undefined; this._commitRange = undefined; @@ -914,103 +1066,130 @@ class GrDiffView extends KeyboardShortcutMixin( return; } - const promises = []; + const promises: Promise[] = []; promises.push(this._getDiffPreferences()); - promises.push(this._getPreferences().then(prefs => { - this._userPrefs = prefs; - })); + promises.push( + this._getPreferences().then(prefs => { + this._userPrefs = prefs; + }) + ); promises.push(this._getChangeDetail(this._changeNum)); promises.push(this._loadComments()); - promises.push(this._getChangeEdit(this._changeNum)); + promises.push(this._getChangeEdit()); this.$.diffHost.cancel(); this.$.diffHost.clearDiffContent(); this._loading = true; return Promise.all(promises) - .then(r => { - this._loading = false; - this._initPatchRange(); - this._initCommitRange(); - this.$.diffHost.comments = this._commentsForDiff; - const edit = r[4]; - if (edit) { - this.set('_change.revisions.' + edit.commit.commit, { - _number: SPECIAL_PATCH_SET_NUM.EDIT, - basePatchNum: edit.base_patch_set_number, - commit: edit.commit, - }); - } - return this.$.diffHost.reload(true); - }) - .then(() => { - this.reporting.diffViewFullyLoaded(); - // If diff view displayed has not ended yet, it ends here. - this.reporting.diffViewDisplayed(); - }) - .then(() => { - const fileUnchanged = this._isFileUnchanged(this._diff); - if (fileUnchanged && value.commentLink) { - this.dispatchEvent(new CustomEvent('show-alert', { + .then(r => { + this._loading = false; + this._initPatchRange(); + this._initCommitRange(); + this.$.diffHost.comments = this._commentsForDiff; + const edit = r[4] as EditInfo | undefined; + if (edit) { + this.set(`_change.revisions.${edit.commit.commit}`, { + _number: EditPatchSetNum, + basePatchNum: edit.base_patch_set_number, + commit: edit.commit, + }); + } + return this.$.diffHost.reload(true); + }) + .then(() => { + this.reporting.diffViewFullyLoaded(); + // If diff view displayed has not ended yet, it ends here. + this.reporting.diffViewDisplayed(); + }) + .then(() => { + if (!this._diff) throw new Error('Missing this._diff'); + const fileUnchanged = this._isFileUnchanged(this._diff); + if (fileUnchanged && value.commentLink) { + if (!this._change) throw new Error('Missing this._change'); + if (!this._path) throw new Error('Missing this._path'); + if (!this._patchRange) throw new Error('Missing this._patchRange'); + + this.dispatchEvent( + new CustomEvent('show-alert', { detail: { message: `File is unchanged between Patchset ${this._patchRange.basePatchNum} and ${this._patchRange.patchNum}. Showing diff of Base vs ${this._patchRange.basePatchNum}`, }, - composed: true, bubbles: true, - })); - GerritNav.navigateToDiff( - this._change, this._path, this._patchRange.basePatchNum, - 'PARENT', this._focusLineNum); - return; - } - if (value.commentLink) { - this._displayToasts(); - } - // If the blame was loaded for a previous file and user navigates to - // another file, then we load the blame for this file too - if (this._isBlameLoaded) this._loadBlame(); - }); + composed: true, + bubbles: true, + }) + ); + GerritNav.navigateToDiff( + this._change, + this._path, + this._patchRange.basePatchNum, + ParentPatchSetNum, + this._focusLineNum + ); + return; + } + if (value.commentLink) { + this._displayToasts(); + } + // If the blame was loaded for a previous file and user navigates to + // another file, then we load the blame for this file too + if (this._isBlameLoaded) this._loadBlame(); + }); } - _changeViewStateChanged(changeViewState) { + _changeViewStateChanged(changeViewState: Partial) { if (changeViewState.diffMode === null) { // If screen size is small, always default to unified view. this.$.restAPI.getPreferences().then(prefs => { - this.set('changeViewState.diffMode', prefs.default_diff_view); + if (prefs) { + this.set('changeViewState.diffMode', prefs.default_diff_view); + } }); } } - _setReviewedObserver(_loggedIn, paramsRecord, _prefs, patchRangeRecord) { - // Polymer 2: check for undefined - if ([_loggedIn, paramsRecord, _prefs, patchRangeRecord, - patchRangeRecord.base].includes( - undefined)) { + @observe('_loggedIn', 'params.*', '_prefs', '_patchRange.*') + _setReviewedObserver( + _loggedIn?: boolean, + paramsRecord?: ElementPropertyDeepChange, + _prefs?: DiffPreferencesInfo, + patchRangeRecord?: ElementPropertyDeepChange + ) { + if (_loggedIn === undefined) return; + if (paramsRecord === undefined) return; + if (_prefs === undefined) return; + if (patchRangeRecord === undefined) return; + if (patchRangeRecord.base === undefined) return; + + const patchRange = patchRangeRecord.base; + if (!_loggedIn) { return; } - const patchRange = patchRangeRecord.base; - const params = paramsRecord.base || {}; - if (!_loggedIn) { return; } if (_prefs.manual_review) { // Checkbox state needs to be set explicitly only when manual_review // is specified. if (patchRange.patchNum) { - this._getReviewedStatus(this.editMode, this._changeNum, - patchRange.patchNum, this._path).then(status => { + this._getReviewedStatus( + this._editMode, + this._changeNum, + patchRange.patchNum, + this._path + ).then((status: boolean) => { this.$.reviewed.checked = status; }); } return; } - if (params.view === GerritNav.View.DIFF) { + if (paramsRecord.base?.view === GerritNav.View.DIFF) { this._setReviewed(true); } } @@ -1018,50 +1197,61 @@ class GrDiffView extends KeyboardShortcutMixin( /** * If the params specify a diff address then configure the diff cursor. */ - _initCursor(params) { - if (this._focusLineNum === undefined) { return; } + _initCursor(params: FetchParams) { + if (this._focusLineNum === undefined) { + return; + } if (params.leftSide) { - this.$.cursor.side = DiffSides.LEFT; + this.$.cursor.side = Side.LEFT; } else { - this.$.cursor.side = DiffSides.RIGHT; + this.$.cursor.side = Side.RIGHT; } this.$.cursor.initialLineNumber = this._focusLineNum; } - _getLineOfInterest(params) { + _getLineOfInterest(params: FetchParams): LineOfInterest | undefined { // If there is a line number specified, pass it along to the diff so that // it will not get collapsed. - if (!this._focusLineNum) { return null; } - return {number: this._focusLineNum, leftSide: params.leftSide}; + if (!this._focusLineNum) { + return undefined; + } + + return {number: this._focusLineNum, leftSide: !!params.leftSide}; } - _pathChanged(path) { + _pathChanged(path: string) { if (path) { - this.dispatchEvent(new CustomEvent('title-change', { - detail: {title: computeTruncatedPath(path)}, - composed: true, bubbles: true, - })); + this.dispatchEvent( + new CustomEvent('title-change', { + detail: {title: computeTruncatedPath(path)}, + composed: true, + bubbles: true, + }) + ); } - if (!this._fileList || this._fileList.length == 0) { return; } + if (!this._fileList || this._fileList.length === 0) return; - this.set('changeViewState.selectedFileIndex', - this._fileList.indexOf(path)); + this.set('changeViewState.selectedFileIndex', this._fileList.indexOf(path)); } - _getDiffUrl(change, patchRange, path) { - if ([change, patchRange, path].includes(undefined)) { - return ''; - } - return GerritNav.getUrlForDiff(change, path, patchRange.patchNum, - patchRange.basePatchNum); + _getDiffUrl(change?: ChangeInfo, patchRange?: PatchRange, path?: string) { + if (!change || !patchRange || !path) return ''; + return GerritNav.getUrlForDiff( + change, + path, + patchRange.patchNum, + patchRange.basePatchNum + ); } - _patchRangeStr(patchRange) { - let patchStr = patchRange.patchNum; - if (patchRange.basePatchNum != null && - patchRange.basePatchNum != PARENT) { - patchStr = patchRange.basePatchNum + '..' + patchRange.patchNum; + _patchRangeStr(patchRange: PatchRange) { + let patchStr = `${patchRange.patchNum}`; + if ( + patchRange.basePatchNum && + patchRange.basePatchNum !== ParentPatchSetNum + ) { + patchStr = `${patchRange.basePatchNum}..${patchRange.patchNum}`; } return patchStr; } @@ -1070,118 +1260,163 @@ class GrDiffView extends KeyboardShortcutMixin( * When the latest patch of the change is selected (and there is no base * patch) then the patch range need not appear in the URL. Return a patch * range object with undefined values when a range is not needed. - * - * @param {!Object} patchRange - * @param {!Object} revisions - * @return {!Object} */ - _getChangeUrlRange(patchRange, revisions) { + _getChangeUrlRange( + patchRange?: PatchRange, + revisions?: {[revisionId: string]: RevisionInfo} + ) { let patchNum = undefined; let basePatchNum = undefined; let latestPatchNum = -1; for (const rev of Object.values(revisions || {})) { - latestPatchNum = Math.max(latestPatchNum, rev._number); + if (typeof rev._number === 'number') { + latestPatchNum = Math.max(latestPatchNum, rev._number); + } } - if (patchRange.basePatchNum !== PARENT || - parseInt(patchRange.patchNum, 10) !== latestPatchNum) { + if (!patchRange) return {patchNum, basePatchNum}; + if ( + patchRange.basePatchNum !== ParentPatchSetNum || + !patchNumEquals(patchRange.patchNum, latestPatchNum as PatchSetNum) + ) { patchNum = patchRange.patchNum; basePatchNum = patchRange.basePatchNum; } return {patchNum, basePatchNum}; } - _getChangePath(change, patchRange, revisions) { - if ([change, patchRange].includes(undefined)) { - return ''; - } + _getChangePath( + change?: ChangeInfo, + patchRange?: PatchRange, + revisions?: {[revisionId: string]: RevisionInfo} + ) { + if (!change) return ''; + if (!patchRange) return ''; + const range = this._getChangeUrlRange(patchRange, revisions); - return GerritNav.getUrlForChange(change, range.patchNum, - range.basePatchNum); + return GerritNav.getUrlForChange( + change, + range.patchNum, + range.basePatchNum + ); } - _navigateToChange(change, patchRange, revisions) { + _navigateToChange( + change?: ChangeInfo, + patchRange?: PatchRange, + revisions?: {[revisionId: string]: RevisionInfo} + ) { + if (!change) return; const range = this._getChangeUrlRange(patchRange, revisions); GerritNav.navigateToChange(change, range.patchNum, range.basePatchNum); } - _computeChangePath(change, patchRangeRecord, revisions) { + _computeChangePath( + change?: ChangeInfo, + patchRangeRecord?: PolymerDeepPropertyChange, + revisions?: {[revisionId: string]: RevisionInfo} + ) { + if (!patchRangeRecord) return ''; return this._getChangePath(change, patchRangeRecord.base, revisions); } - _formatFilesForDropdown(files, patchNum, changeComments) { - // Polymer 2: check for undefined - if ([ - files, - patchNum, - changeComments, - ].includes(undefined)) { - return; - } + _formatFilesForDropdown( + files?: Files, + patchNum?: PatchSetNum, + changeComments?: ChangeComments + ): DropdownItem[] { + if (!files) return []; + if (!patchNum) return []; + if (!changeComments) return []; - if (!files) { return; } - const dropdownContent = []; + const dropdownContent: DropdownItem[] = []; for (const path of files.sortedFileList) { dropdownContent.push({ text: computeDisplayPath(path), mobileText: computeTruncatedPath(path), value: path, - bottomText: this._computeCommentString(changeComments, patchNum, - path, files.changeFilesByPath[path]), + bottomText: this._computeCommentString( + changeComments, + patchNum, + path, + files.changeFilesByPath[path] + ), }); } return dropdownContent; } - _computeCommentString(changeComments, patchNum, path, changeFileInfo) { - const unresolvedCount = changeComments.computeUnresolvedNum({patchNum, - path}); + _computeCommentString( + changeComments?: ChangeComments, + patchNum?: PatchSetNum, + path?: string, + changeFileInfo?: FileInfo + ) { + if (!changeComments) return ''; + if (!path) return ''; + if (!changeFileInfo) return ''; + + const unresolvedCount = changeComments.computeUnresolvedNum({ + patchNum, + path, + }); const commentCount = changeComments.computeCommentCount({patchNum, path}); const commentString = GrCountStringFormatter.computePluralString( - commentCount, 'comment'); + commentCount, + 'comment' + ); const unresolvedString = GrCountStringFormatter.computeString( - unresolvedCount, 'unresolved'); + unresolvedCount, + 'unresolved' + ); - const unmodifiedString = changeFileInfo.status === 'U' ? 'no changes': ''; + const unmodifiedString = changeFileInfo.status === 'U' ? 'no changes' : ''; - return [ - unmodifiedString, - commentString, - unresolvedString] - .filter(v => v && v.length > 0).join(', '); + return [unmodifiedString, commentString, unresolvedString] + .filter(v => v && v.length > 0) + .join(', '); } - _computePrefsButtonHidden(prefs, prefsDisabled) { + _computePrefsButtonHidden( + prefs?: DiffPreferencesInfo, + prefsDisabled?: boolean + ) { return prefsDisabled || !prefs; } - _handleFileChange(e) { + _handleFileChange(e: CustomEvent) { + if (!this._change) return; + if (!this._patchRange) return; + // This is when it gets set initially. const path = e.detail.value; if (path === this._path) { return; } - GerritNav.navigateToDiff(this._change, path, this._patchRange.patchNum, - this._patchRange.basePatchNum); + GerritNav.navigateToDiff( + this._change, + path, + this._patchRange.patchNum, + this._patchRange.basePatchNum + ); } - _handleFileTap(e) { - // async is needed so that that the click event is fired before the - // dropdown closes (This was a bug for touch devices). - this.async(() => { - this.$.dropdown.close(); - }, 1); - } + _handlePatchChange(e: CustomEvent) { + if (!this._change) return; + if (!this._path) return; + if (!this._patchRange) return; - _handlePatchChange(e) { const {basePatchNum, patchNum} = e.detail; - if (patchNumEquals(basePatchNum, this._patchRange.basePatchNum) && - patchNumEquals(patchNum, this._patchRange.patchNum)) { return; } - GerritNav.navigateToDiff( - this._change, this._path, patchNum, basePatchNum); + if ( + patchNumEquals(basePatchNum, this._patchRange.basePatchNum) && + patchNumEquals(patchNum, this._patchRange.patchNum) + ) { + return; + } + GerritNav.navigateToDiff(this._change, this._path, patchNum, basePatchNum); } - _handlePrefsTap(e) { + _handlePrefsTap(e: Event) { e.preventDefault(); this.$.diffPreferencesDialog.open(); } @@ -1197,8 +1432,6 @@ class GrDiffView extends KeyboardShortcutMixin( * diff is viewed. * * Use side-by-side if the user is not logged in. - * - * @return {string} */ _getDiffViewMode() { if (this.changeViewState.diffMode) { @@ -1211,30 +1444,56 @@ class GrDiffView extends KeyboardShortcutMixin( } } - _computeModeSelectHideClass(_diff) { - return (!_diff || _diff.binary) ? 'hide' : ''; + _computeModeSelectHideClass(diff?: DiffInfo) { + return !diff || diff.binary ? 'hide' : ''; } - _onLineSelected(e, detail) { - if (!this._change) { return; } + _onLineSelected( + _: Event, + detail: {side: Side | CommentSide; number: number} + ) { + if (!this._change) return; + if (!this._path) return; + if (!this._changeNum) return; + if (!this._patchRange) return; + const number = detail.number; // for on-comment-anchor-tap side can be PARENT/REVISIONS // for on-line-selected side can be left/right - const leftSide = detail.side === Side.LEFT || detail.side === 'PARENT'; - const url = GerritNav.getUrlForDiffById(this._changeNum, - this._change.project, this._path, this._patchRange.patchNum, - this._patchRange.basePatchNum, number, leftSide); + const leftSide = + detail.side === Side.LEFT || detail.side === CommentSide.PARENT; + const url = GerritNav.getUrlForDiffById( + this._changeNum, + this._change.project, + this._path, + this._patchRange.patchNum, + this._patchRange.basePatchNum, + number, + leftSide + ); history.replaceState(null, '', url); } _computeDownloadDropdownLinks( - project, changeNum, patchRange, path, diff) { - if (!patchRange || !patchRange.patchNum) { return []; } + project?: RepoName, + changeNum?: NumericChangeId, + patchRange?: PatchRange, + path?: string, + diff?: DiffInfo + ) { + if (!project) return []; + if (!changeNum) return []; + if (!patchRange || !patchRange.patchNum) return []; + if (!path) return []; const links = [ { url: this._computeDownloadPatchLink( - project, changeNum, patchRange, path), + project, + changeNum, + patchRange, + path + ), name: 'Patch', }, ]; @@ -1244,30 +1503,41 @@ class GrDiffView extends KeyboardShortcutMixin( if (diff.change_type === 'RENAMED') { leftPath = diff.meta_a.name; } - links.push( - { - url: this._computeDownloadFileLink( - project, changeNum, patchRange, leftPath, true), - name: 'Left Content', - } - ); + links.push({ + url: this._computeDownloadFileLink( + project, + changeNum, + patchRange, + leftPath, + true + ), + name: 'Left Content', + }); } if (diff && diff.meta_b) { - links.push( - { - url: this._computeDownloadFileLink( - project, changeNum, patchRange, path, false), - name: 'Right Content', - } - ); + links.push({ + url: this._computeDownloadFileLink( + project, + changeNum, + patchRange, + path, + false + ), + name: 'Right Content', + }); } return links; } _computeDownloadFileLink( - project, changeNum, patchRange, path, isBase) { + project: RepoName, + changeNum: NumericChangeId, + patchRange: PatchRange, + path: string, + isBase?: boolean + ) { let patchNum = patchRange.patchNum; const comparedAgainsParent = patchRange.basePatchNum === 'PARENT'; @@ -1276,8 +1546,9 @@ class GrDiffView extends KeyboardShortcutMixin( patchNum = patchRange.basePatchNum; } - let url = changeBaseURL(project, changeNum, patchNum) + - `/files/${encodeURIComponent(path)}/download`; + let url = + changeBaseURL(project, changeNum, patchNum) + + `/files/${encodeURIComponent(path)}/download`; if (isBase && comparedAgainsParent) { url += '?parent=1'; @@ -1286,65 +1557,89 @@ class GrDiffView extends KeyboardShortcutMixin( return url; } - _computeDownloadPatchLink(project, changeNum, patchRange, path) { + _computeDownloadPatchLink( + project: RepoName, + changeNum: NumericChangeId, + patchRange: PatchRange, + path: string + ) { let url = changeBaseURL(project, changeNum, patchRange.patchNum); url += '/patch?zip&path=' + encodeURIComponent(path); return url; } _loadComments() { + if (!this._changeNum) throw new Error('Missing this._changeNum'); return this.$.commentAPI.loadAll(this._changeNum).then(comments => { this._changeComments = comments; }); } - _recomputeComments(files, path, patchRange, projectConfig) { - // Polymer 2: check for undefined - if ([ - files, - path, - patchRange, - projectConfig, - ].includes(undefined)) { - return undefined; - } + @observe('_files.changeFilesByPath', '_path', '_patchRange', '_projectConfig') + _recomputeComments( + files?: {[path: string]: FileInfo}, + path?: string, + patchRange?: PatchRange, + projectConfig?: ConfigInfo + ) { + if (!files) return; + if (!path) return; + if (!patchRange) return; + if (!projectConfig) return; + if (!this._changeComments) return; const file = files[path]; if (file && file.old_path) { this._commentsForDiff = this._changeComments.getCommentsBySideForFile( - {path, basePath: file.old_path}, - patchRange, - projectConfig); + {path, basePath: file.old_path}, + patchRange, + projectConfig + ); this.$.diffHost.comments = this._commentsForDiff; } } - _getPaths(patchRange) { + _getPaths(patchRange: PatchRange) { + if (!this._changeComments) return {}; return this._changeComments.getPaths(patchRange); } - _getCommentsForPath(path, patchRange, projectConfig) { - return this._changeComments.getCommentsBySideForPath(path, patchRange, - projectConfig); + _getCommentsForPath( + path?: string, + patchRange?: PatchRange, + projectConfig?: ConfigInfo + ) { + if (!path) return undefined; + if (!patchRange) return undefined; + if (!this._changeComments) return undefined; + + return this._changeComments.getCommentsBySideForPath( + path, + patchRange, + projectConfig + ); } _getDiffDrafts() { + if (!this._changeNum) throw new Error('Missing this._changeNum'); + return this.$.restAPI.getDiffDrafts(this._changeNum); } - _computeCommentSkips(commentMap, fileList, path) { - // Polymer 2: check for undefined - if ([ - commentMap, - fileList, - path, - ].includes(undefined)) { - return undefined; - } + _computeCommentSkips( + commentMap?: CommentMap, + fileList?: string[], + path?: string + ) { + if (!commentMap) return undefined; + if (!fileList) return undefined; + if (!path) return undefined; - const skips = {previous: null, next: null}; - if (!fileList.length) { return skips; } + const skips: CommentSkips = {previous: null, next: null}; + if (!fileList.length) { + return skips; + } const pathIndex = fileList.indexOf(path); // Scan backward for the previous file. @@ -1366,40 +1661,45 @@ class GrDiffView extends KeyboardShortcutMixin( return skips; } - _computeContainerClass(editMode) { + _computeContainerClass(editMode: boolean) { return editMode ? 'editMode' : ''; } - /** - * @param {!Object} patchRangeRecord - */ - _computeEditMode(patchRangeRecord) { + _computeEditMode( + patchRangeRecord: PolymerDeepPropertyChange + ) { const patchRange = patchRangeRecord.base || {}; - return patchNumEquals(patchRange.patchNum, SPECIAL_PATCH_SET_NUM.EDIT); + return patchNumEquals(patchRange.patchNum, EditPatchSetNum); } - _computeBlameToggleLabel(loaded, loading) { - if (loaded) { return 'Hide blame'; } - return 'Show blame'; + _computeBlameToggleLabel(loaded?: boolean, loading?: boolean) { + return loaded && !loading ? 'Hide blame' : 'Show blame'; } _loadBlame() { this._isBlameLoading = true; - this.dispatchEvent(new CustomEvent('show-alert', { - detail: {message: MSG_LOADING_BLAME}, - composed: true, bubbles: true, - })); - this.$.diffHost.loadBlame() - .then(() => { - this._isBlameLoading = false; - this.dispatchEvent(new CustomEvent('show-alert', { + this.dispatchEvent( + new CustomEvent('show-alert', { + detail: {message: MSG_LOADING_BLAME}, + composed: true, + bubbles: true, + }) + ); + this.$.diffHost + .loadBlame() + .then(() => { + this._isBlameLoading = false; + this.dispatchEvent( + new CustomEvent('show-alert', { detail: {message: MSG_LOADED_BLAME}, - composed: true, bubbles: true, - })); - }) - .catch(() => { - this._isBlameLoading = false; - }); + composed: true, + bubbles: true, + }) + ); + }) + .catch(() => { + this._isBlameLoading = false; + }); } /** @@ -1414,148 +1714,193 @@ class GrDiffView extends KeyboardShortcutMixin( this._loadBlame(); } - _handleToggleBlame(e) { - if (this.shouldSuppressKeyboardShortcut(e) || - this.modifierPressed(e)) { return; } + _handleToggleBlame(e: CustomKeyboardEvent) { + if (this.shouldSuppressKeyboardShortcut(e)) return; + if (this.modifierPressed(e)) return; + this._toggleBlame(); } - _handleToggleHideAllCommentThreads(e) { - if (this.shouldSuppressKeyboardShortcut(e) || - this.modifierPressed(e)) { return; } + _handleToggleHideAllCommentThreads(e: CustomKeyboardEvent) { + if (this.shouldSuppressKeyboardShortcut(e)) return; + if (this.modifierPressed(e)) return; + this.toggleClass('hideComments'); } - _handleDiffAgainstBase(e) { - if (this.shouldSuppressKeyboardShortcut(e)) { return; } - if (patchNumEquals(this._patchRange.basePatchNum, - SPECIAL_PATCH_SET_NUM.PARENT)) { - this.dispatchEvent(new CustomEvent('show-alert', { - detail: { - message: 'Base is already selected.', - }, - composed: true, bubbles: true, - })); + _handleDiffAgainstBase(e: CustomKeyboardEvent) { + if (this.shouldSuppressKeyboardShortcut(e)) return; + if (!this._change) return; + if (!this._path) return; + if (!this._patchRange) return; + + if (patchNumEquals(this._patchRange.basePatchNum, ParentPatchSetNum)) { + this.dispatchEvent( + new CustomEvent('show-alert', { + detail: { + message: 'Base is already selected.', + }, + composed: true, + bubbles: true, + }) + ); return; } GerritNav.navigateToDiff( - this._change, this._path, this._patchRange.patchNum); + this._change, + this._path, + this._patchRange.patchNum + ); } - _handleDiffBaseAgainstLeft(e) { - if (this.shouldSuppressKeyboardShortcut(e)) { return; } - if (patchNumEquals(this._patchRange.basePatchNum, - SPECIAL_PATCH_SET_NUM.PARENT)) { - this.dispatchEvent(new CustomEvent('show-alert', { - detail: { - message: 'Left is already base.', - }, - composed: true, bubbles: true, - })); + _handleDiffBaseAgainstLeft(e: CustomKeyboardEvent) { + if (this.shouldSuppressKeyboardShortcut(e)) return; + if (!this._change) return; + if (!this._path) return; + if (!this._patchRange) return; + + if (patchNumEquals(this._patchRange.basePatchNum, ParentPatchSetNum)) { + this.dispatchEvent( + new CustomEvent('show-alert', { + detail: { + message: 'Left is already base.', + }, + composed: true, + bubbles: true, + }) + ); return; } - GerritNav.navigateToDiff(this._change, this._path, - this._patchRange.basePatchNum); + GerritNav.navigateToDiff( + this._change, + this._path, + this._patchRange.basePatchNum + ); } - _handleDiffAgainstLatest(e) { - if (this.shouldSuppressKeyboardShortcut(e)) { return; } + _handleDiffAgainstLatest(e: CustomKeyboardEvent) { + if (this.shouldSuppressKeyboardShortcut(e)) return; + if (!this._change) return; + if (!this._path) return; + if (!this._patchRange) return; const latestPatchNum = computeLatestPatchNum(this._allPatchSets); if (patchNumEquals(this._patchRange.patchNum, latestPatchNum)) { - this.dispatchEvent(new CustomEvent('show-alert', { - detail: { - message: 'Latest is already selected.', - }, - composed: true, bubbles: true, - })); + this.dispatchEvent( + new CustomEvent('show-alert', { + detail: { + message: 'Latest is already selected.', + }, + composed: true, + bubbles: true, + }) + ); return; } GerritNav.navigateToDiff( - this._change, this._path, latestPatchNum, - this._patchRange.basePatchNum); + this._change, + this._path, + latestPatchNum, + this._patchRange.basePatchNum + ); } - _handleDiffRightAgainstLatest(e) { - if (this.shouldSuppressKeyboardShortcut(e)) { return; } + _handleDiffRightAgainstLatest(e: CustomKeyboardEvent) { + if (this.shouldSuppressKeyboardShortcut(e)) return; + if (!this._change) return; + if (!this._path) return; + if (!this._patchRange) return; + const latestPatchNum = computeLatestPatchNum(this._allPatchSets); if (patchNumEquals(this._patchRange.patchNum, latestPatchNum)) { - this.dispatchEvent(new CustomEvent('show-alert', { - detail: { - message: 'Right is already latest.', - }, - composed: true, bubbles: true, - })); + this.dispatchEvent( + new CustomEvent('show-alert', { + detail: { + message: 'Right is already latest.', + }, + composed: true, + bubbles: true, + }) + ); return; } - GerritNav.navigateToDiff(this._change, this._path, latestPatchNum, - this._patchRange.patchNum); + GerritNav.navigateToDiff( + this._change, + this._path, + latestPatchNum, + this._patchRange.patchNum + ); } - _handleDiffBaseAgainstLatest(e) { - if (this.shouldSuppressKeyboardShortcut(e)) { return; } + _handleDiffBaseAgainstLatest(e: CustomKeyboardEvent) { + if (this.shouldSuppressKeyboardShortcut(e)) return; + if (!this._change) return; + if (!this._path) return; + if (!this._patchRange) return; + const latestPatchNum = computeLatestPatchNum(this._allPatchSets); - if (patchNumEquals(this._patchRange.patchNum, latestPatchNum) && - patchNumEquals(this._patchRange.basePatchNum, - SPECIAL_PATCH_SET_NUM.PARENT)) { - this.dispatchEvent(new CustomEvent('show-alert', { - detail: { - message: 'Already diffing base against latest.', - }, - composed: true, bubbles: true, - })); + if ( + patchNumEquals(this._patchRange.patchNum, latestPatchNum) && + patchNumEquals(this._patchRange.basePatchNum, ParentPatchSetNum) + ) { + this.dispatchEvent( + new CustomEvent('show-alert', { + detail: { + message: 'Already diffing base against latest.', + }, + composed: true, + bubbles: true, + }) + ); return; } GerritNav.navigateToDiff(this._change, this._path, latestPatchNum); } - _computeBlameLoaderClass(isImageDiff, path) { + _computeBlameLoaderClass(isImageDiff?: boolean, path?: string) { return !isMagicPath(path) && !isImageDiff ? 'show' : ''; } - _getRevisionInfo(change) { - return new RevisionInfo(change); + _getRevisionInfo(change: ChangeInfo) { + return new RevisionInfoObj(change); } - _computeFileNum(file, files) { - // Polymer 2: check for undefined - if ([file, files].includes(undefined)) { - return undefined; - } + _computeFileNum(file?: string, files?: DropdownItem[]) { + if (!file || !files) return undefined; return files.findIndex(({value}) => value === file) + 1; } - /** - * @param {number} fileNum - * @param {!Array} files - * @return {string} - */ - _computeFileNumClass(fileNum, files) { - if (files && fileNum > 0) { + _computeFileNumClass(fileNum?: number, files?: DropdownItem[]) { + if (files && fileNum && fileNum > 0) { return 'show'; } return ''; } - _handleExpandAllDiffContext(e) { - if (this.shouldSuppressKeyboardShortcut(e)) { return; } + _handleExpandAllDiffContext(e: CustomKeyboardEvent) { + if (this.shouldSuppressKeyboardShortcut(e)) return; + this.$.diffHost.expandAllContext(); } - _computeDiffPrefsDisabled(disableDiffPrefs, loggedIn) { + _computeDiffPrefsDisabled(disableDiffPrefs?: boolean, loggedIn?: boolean) { return disableDiffPrefs || !loggedIn; } - _handleNextUnreviewedFile(e) { - if (this.shouldSuppressKeyboardShortcut(e)) { return; } + _handleNextUnreviewedFile(e: CustomKeyboardEvent) { + if (this.shouldSuppressKeyboardShortcut(e)) return; + if (!this._path) return; + if (!this._fileList) return; + if (!this._reviewedFiles) return; + this._setReviewed(true); // Ensure that the currently viewed file always appears in unreviewedFiles // so we resolve the right "next" file. - const unreviewedFiles = this._fileList - .filter(file => - (file === this._path || !this._reviewedFiles.has(file))); + const unreviewedFiles = this._fileList.filter( + file => file === this._path || !this._reviewedFiles.has(file) + ); this._navToFile(this._path, unreviewedFiles, 1); } @@ -1563,31 +1908,35 @@ class GrDiffView extends KeyboardShortcutMixin( this._getDiffPreferences(); } - _computeCanEdit(loggedIn, changeChangeRecord) { - if ([changeChangeRecord, changeChangeRecord.base] - .some(arg => arg === undefined)) { - return false; - } + _computeCanEdit( + loggedIn?: boolean, + changeChangeRecord?: PolymerDeepPropertyChange + ) { + if (!changeChangeRecord?.base) return false; return loggedIn && changeIsOpen(changeChangeRecord.base); } - _computeIsLoggedIn(loggedIn) { + _computeIsLoggedIn(loggedIn: boolean) { return loggedIn ? true : false; } /** * Wrapper for using in the element template and computed properties */ - _computeAllPatchSets(change) { + _computeAllPatchSets(change: ChangeInfo) { return computeAllPatchSets(change); } /** * Wrapper for using in the element template and computed properties */ - _computeDisplayPath(path) { + _computeDisplayPath(path: string) { return computeDisplayPath(path); } } -customElements.define(GrDiffView.is, GrDiffView); +declare global { + interface HTMLElementTagNameMap { + 'gr-diff-view': GrDiffView; + } +} diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js index b63af54..7d18527 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js +++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js @@ -24,6 +24,7 @@ import {SPECIAL_PATCH_SET_NUM} from '../../../utils/patch-set-util.js'; import {Shortcut} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.js'; import {_testOnly_findCommentById} from '../gr-comment-api/gr-comment-api.js'; import {appContext} from '../../../services/app-context.js'; +import {GerritView} from '../../core/gr-navigation/gr-navigation.js'; const basicFixture = fixtureFromElement('gr-diff-view'); @@ -82,7 +83,7 @@ suite('gr-diff-view tests', () => { }; } - setup(() => { + setup(async () => { clock = sinon.useFakeTimers(); sinon.stub(appContext.flagsService, 'isEnabled').returns(true); stub('gr-rest-api-interface', { @@ -118,6 +119,14 @@ suite('gr-diff-view tests', () => { }, }); element = basicFixture.instantiate(); + element._changeNum = '42'; + element._path = 'some/path.txt'; + element._change = {}; + element._diff = {content: []}; + element._patchRange = { + patchNum: 77, + basePatchNum: 'PARENT', + }; sinon.stub(element.$.commentAPI, 'loadAll').returns(Promise.resolve({ _comments: {'/COMMIT_MSG': [{id: 'c1', line: 10, patch_set: 2, __commentSide: 'left', path: '/COMMIT_MSG'}]}, @@ -127,7 +136,8 @@ suite('gr-diff-view tests', () => { getCommentsBySideForPath: () => {}, findCommentById: _testOnly_findCommentById, })); - return element._loadComments(); + await element._loadComments(); + await flush(); }); teardown(() => { @@ -269,13 +279,22 @@ suite('gr-diff-view tests', () => { assert.equal(element._isFileUnchanged(diff), true); }); - test('diff toast to go to latest is shown and not base', () => { + test('diff toast to go to latest is shown and not base', async () => { sinon.stub(element.reporting, 'diffViewDisplayed'); sinon.stub(element, '_loadBlame'); sinon.stub(element.$.diffHost, 'reload').returns(Promise.resolve()); sinon.spy(element, '_paramsChanged'); - sinon.stub(element, '_getChangeDetail').returns(Promise.resolve( - generateChange({revisionsCount: 11}))); + element.$.restAPI.getDiffChangeDetail.restore(); + sinon.stub(element.$.restAPI, 'getDiffChangeDetail') + .returns( + Promise.resolve(generateChange({revisionsCount: 11}))); + element._patchRange = { + patchNum: '2', + basePatchNum: '1', + }; + sinon.stub(element, '_isFileUnchanged').returns(false); + const toastStub = + sinon.stub(element, '_displayDiffBaseAgainstLeftToast'); element.params = { view: GerritNav.View.DIFF, changeNum: '42', @@ -283,12 +302,8 @@ suite('gr-diff-view tests', () => { commentId: 'c1', commentLink: true, }; - element._change = generateChange({revisionsCount: 11}); - const toastStub = - sinon.stub(element, '_displayDiffBaseAgainstLeftToast'); - return element._paramsChanged.returnValues[0].then(() => { - assert.isTrue(toastStub.called); - }); + await element._paramsChanged.returnValues[0]; + assert.isTrue(toastStub.called); }); test('toggle left diff with a hotkey', () => { @@ -1251,7 +1266,7 @@ suite('gr-diff-view tests', () => { }); test('_getLineOfInterest', () => { - assert.isNull(element._getLineOfInterest({})); + assert.isUndefined(element._getLineOfInterest({})); element._focusLineNum = 12; let result = element._getLineOfInterest({}); @@ -1334,10 +1349,20 @@ suite('gr-diff-view tests', () => { }); suite('_initPatchRange', () => { + setup(async () => { + // const changeDetail = generateChange({revisionsCount: 5}); + // sinon.stub(element.$.restAPI, 'getDiffChangeDetail') + // .returns(Promise.resolve(changeDetail)); + element.params = { + view: GerritView.DIFF, + changeNum: '42', + patchNum: '3', + }; + await flush(); + }); test('empty', () => { sinon.stub(element, '_getCommentsForPath'); sinon.stub(element, '_getPaths').returns(new Map()); - element.params = {}; element._initPatchRange(); assert.equal(Object.keys(element._commentMap).length, 0); }); @@ -1354,7 +1379,6 @@ suite('gr-diff-view tests', () => { basePatchNum: '3', patchNum: '5', }; - element.params = {}; element._initPatchRange(); assert.deepEqual(Object.keys(element._commentMap), ['path/to/file/one.cpp', 'path-to/file/two.py']); @@ -1523,6 +1547,9 @@ suite('gr-diff-view tests', () => { .then(reviewed => assert.isFalse(reviewed))); promises.push(element._getReviewedStatus(false, null, null, 'path') + .then(reviewed => assert.isFalse(reviewed))); + + promises.push(element._getReviewedStatus(false, 3, 5, 'path') .then(reviewed => assert.isTrue(reviewed))); return Promise.all(promises); @@ -1613,6 +1640,7 @@ suite('gr-diff-view tests', () => { patchNum: 1, basePatchNum: 'PARENT', }; + element._change = generateChange({revisionsCount: 1}); flush(); assert.isTrue(GerritNav.navigateToDiff.notCalled); @@ -1754,6 +1782,7 @@ suite('gr-diff-view tests', () => { getReviewedFiles() { return Promise.resolve([]); }, }); element = basicFixture.instantiate(); + element._changeNum = '42'; return element._loadComments(); }); diff --git a/polygerrit-ui/app/elements/gr-app-types.ts b/polygerrit-ui/app/elements/gr-app-types.ts index aee5bcb..448b281 100644 --- a/polygerrit-ui/app/elements/gr-app-types.ts +++ b/polygerrit-ui/app/elements/gr-app-types.ts @@ -20,7 +20,14 @@ import { GroupDetailView, RepoDetailView, } from './core/gr-navigation/gr-navigation'; -import {DashboardId, GroupId, RepoName} from '../types/common'; +import { + DashboardId, + GroupId, + NumericChangeId, + PatchSetNum, + RepoName, + UrlEncodedCommentId, +} from '../types/common'; export interface AppElement extends HTMLElement { params: AppElementParams | GenerateUrlParameters; @@ -86,6 +93,19 @@ export interface AppElementAgreementParam { view: GerritView.AGREEMENTS; } +export interface AppElementDiffViewParam { + view: GerritView.DIFF; + changeNum: NumericChangeId; + project?: RepoName; + commentId?: UrlEncodedCommentId; + path?: string; + patchNum?: PatchSetNum; + basePatchNum?: PatchSetNum; + lineNum: number; + leftSide?: boolean; + commentLink?: boolean; +} + export interface AppElementJustRegisteredParams { // We use params.view === ... as a type guard. // The view?: never tells to the compiler that @@ -106,4 +126,5 @@ export type AppElementParams = | AppElementSearchParam | AppElementSettingsParam | AppElementAgreementParam - | AppElementJustRegisteredParams; + | AppElementJustRegisteredParams + | AppElementDiffViewParam; diff --git a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts index b255ea5..d915873 100644 --- a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts +++ b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts @@ -544,6 +544,7 @@ export interface CustomKeyboardEvent extends CustomEvent, EventApi { readonly metaKey: boolean; readonly shiftKey: boolean; readonly keyCode: number; + readonly repeat: boolean; } function getKeyboardEvent(e: CustomKeyboardEvent): CustomKeyboardEvent { const event = dom(e.detail ? e.detail.keyboardEvent : e); @@ -1091,6 +1092,8 @@ export interface KeyboardShortcutMixinInterface { getKeyboardEvent(e: CustomKeyboardEvent): CustomKeyboardEvent; addKeyboardShortcutDirectoryListener(listener: ShortcutListener): void; removeKeyboardShortcutDirectoryListener(listener: ShortcutListener): void; + // TODO(TS): Remove underscore. Apparently not a private method. + _throttleWrap(eventListener: EventListener): EventListener; } export function _testOnly_getShortcutManagerInstance() { diff --git a/polygerrit-ui/app/services/services/gr-rest-api/gr-rest-api.ts b/polygerrit-ui/app/services/services/gr-rest-api/gr-rest-api.ts index 6b93082..7ff65e2 100644 --- a/polygerrit-ui/app/services/services/gr-rest-api/gr-rest-api.ts +++ b/polygerrit-ui/app/services/services/gr-rest-api/gr-rest-api.ts @@ -818,6 +818,11 @@ export interface RestApiService { topic: string | null ): Promise; + getChangeFiles( + changeNum: NumericChangeId, + patchRange: PatchRange + ): Promise; + getChangeOrEditFiles( changeNum: NumericChangeId, patchRange: PatchRange @@ -844,4 +849,6 @@ export interface RestApiService { ): Promise; getTopMenus(errFn?: ErrorCallback): Promise; + + setInProjectLookup(changeNum: NumericChangeId, project: RepoName): void; } diff --git a/polygerrit-ui/app/types/common.ts b/polygerrit-ui/app/types/common.ts index 5bcf0b8..bd71f2c 100644 --- a/polygerrit-ui/app/types/common.ts +++ b/polygerrit-ui/app/types/common.ts @@ -1158,6 +1158,7 @@ export interface UserConfigInfo { * https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#comment-info */ export interface CommentInfo { + // TODO(TS): Make this required. patch_set?: PatchSetNum; id: UrlEncodedCommentId; path?: string; @@ -1441,7 +1442,7 @@ export interface CommentLinks { /** * The ConfigInfo entity contains information about the effective - * projectconfiguration. + * project configuration. * https://gerrit-review.googlesource.com/Documentation/rest-api-projects.html#config-info */ export interface ConfigInfo { @@ -1468,7 +1469,8 @@ export interface ConfigInfo { } /** - * The ProjectAccessInfo entity contains information about the access rights for a project + * The ProjectAccessInfo entity contains information about the access rights for + * a project. * https://gerrit-review.googlesource.com/Documentation/rest-api-access.html#project-access-info */ export interface ProjectAccessInfo { diff --git a/polygerrit-ui/app/types/types.ts b/polygerrit-ui/app/types/types.ts index 3768391..708b5d8 100644 --- a/polygerrit-ui/app/types/types.ts +++ b/polygerrit-ui/app/types/types.ts @@ -14,12 +14,12 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import {Side} from '../constants/constants'; +import {DiffViewMode, Side} from '../constants/constants'; import {IronA11yAnnouncer} from '@polymer/iron-a11y-announcer/iron-a11y-announcer'; import {GrDiffLine} from '../elements/diff/gr-diff/gr-diff-line'; import {FlattenedNodesObserver} from '@polymer/polymer/lib/utils/flattened-nodes-observer'; import {PaperInputElement} from '@polymer/paper-input/paper-input'; -import {CommitId} from './common'; +import {CommitId, NumericChangeId, PatchRange, PatchSetNum} from './common'; export function notUndefined(x: T | undefined): x is T { return x !== undefined; @@ -165,3 +165,51 @@ export interface DiffLayer { addListener?(listener: DiffLayerListener): void; removeListener?(listener: DiffLayerListener): void; } + +export interface ChangeViewState { + changeNum: NumericChangeId | null; + patchRange: PatchRange | null; + selectedFileIndex: number; + showReplyDialog: boolean; + showDownloadDialog: boolean; + diffMode: DiffViewMode | null; + numFilesShown: number | null; + scrollTop: number; +} + +export interface ChangeListViewState { + query: string | null; + offset: number; + selectedChangeIndex: number; +} + +export interface DashboardViewState { + selectedChangeIndex: number; +} + +export interface ViewState { + changeView: ChangeViewState; + changeListView: ChangeListViewState; + dashboardView: DashboardViewState; +} + +export interface PatchSetFile { + path: string; + basePath?: string; + patchNum?: PatchSetNum; +} + +export interface PatchNumOnly { + patchNum: PatchSetNum; +} + +export function isPatchSetFile( + x: PatchSetFile | PatchNumOnly +): x is PatchSetFile { + return !!(x as PatchSetFile).path; +} + +export interface FileRange { + basePath?: string; + path: string; +} diff --git a/polygerrit-ui/app/utils/path-list-util.ts b/polygerrit-ui/app/utils/path-list-util.ts index 008abd2..dda6031 100644 --- a/polygerrit-ui/app/utils/path-list-util.ts +++ b/polygerrit-ui/app/utils/path-list-util.ts @@ -90,7 +90,7 @@ export function computeDisplayPath(path: string) { return path; } -export function isMagicPath(path: string) { +export function isMagicPath(path?: string) { return ( !!path && (path === SpecialFilePath.COMMIT_MESSAGE ||