diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts index 163a5285a4..6a40aa5c71 100644 --- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts +++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts @@ -163,7 +163,6 @@ import { fireDialogChange, fireTitleChange, } from '../../../utils/event-util'; -import {KnownExperimentId} from '../../../services/flags/flags'; import {GerritView} from '../../../services/router/router-model'; import {takeUntil} from 'rxjs/operators'; import {aPluginHasRegistered$} from '../../../services/checks/checks-model'; @@ -171,13 +170,7 @@ import {Subject} from 'rxjs'; import {debounce, DelayedTask} from '../../../utils/async-util'; import {Timing} from '../../../constants/reporting'; -const CHANGE_ID_ERROR = { - MISMATCH: 'mismatch', - MISSING: 'missing', -}; -const CHANGE_ID_REGEX_PATTERN = /^(Change-Id:\s|Link:.*\/id\/)(I[0-9a-f]{8,40})/gm; - -const MIN_LINES_FOR_COMMIT_COLLAPSE = 30; +const MIN_LINES_FOR_COMMIT_COLLAPSE = 17; const REVIEWERS_REGEX = /^(R|CC)=/gm; const MIN_CHECK_INTERVAL_SECS = 0; @@ -251,8 +244,6 @@ export class GrChangeView extends KeyboardShortcutMixin(PolymerElement) { private readonly reporting = appContext.reportingService; - private readonly flagsService = appContext.flagsService; - private readonly jsAPI = appContext.jsApiService; private readonly changeService = appContext.changeService; @@ -356,8 +347,7 @@ export class GrChangeView extends KeyboardShortcutMixin(PolymerElement) { type: Boolean, computed: '_computeHideEditCommitMessage(_loggedIn, ' + - '_editingCommitMessage, _change, _editMode, _commitCollapsed, ' + - '_commitCollapsible)', + '_editingCommitMessage, _change, _editMode)', }) _hideEditCommitMessage?: boolean; @@ -379,13 +369,6 @@ export class GrChangeView extends KeyboardShortcutMixin(PolymerElement) { @property({type: Number}) _lineHeight?: number; - @property({ - type: String, - computed: - '_computeChangeIdCommitMessageError(_latestCommitMessage, _change)', - }) - _changeIdCommitMessageError?: string; - @property({type: Object}) _patchRange?: ChangeViewPatchRange; @@ -525,9 +508,6 @@ export class GrChangeView extends KeyboardShortcutMixin(PolymerElement) { @property({type: Boolean}) _showChecksTab = false; - @property({type: Boolean}) - _isNewChangeSummaryUiEnabled = false; - @property({type: String}) _tabState?: TabState; @@ -569,9 +549,6 @@ export class GrChangeView extends KeyboardShortcutMixin(PolymerElement) { aPluginHasRegistered$.pipe(takeUntil(this.disconnected$)).subscribe(b => { this._showChecksTab = b; }); - this._isNewChangeSummaryUiEnabled = this.flagsService.isEnabled( - KnownExperimentId.NEW_CHANGE_SUMMARY_UI - ); } constructor() { @@ -841,11 +818,6 @@ export class GrChangeView extends KeyboardShortcutMixin(PolymerElement) { } } - _handleEditCommitMessage() { - this._editingCommitMessage = true; - this.$.commitMessageEditor.focusTextarea(); - } - _handleCommitMessageSave(e: EditableContentSaveEvent) { assertIsDefined(this._change, '_change'); if (!this._changeNum) @@ -907,19 +879,13 @@ export class GrChangeView extends KeyboardShortcutMixin(PolymerElement) { loggedIn: boolean, editing: boolean, change: ChangeInfo, - editMode?: boolean, - collapsed?: boolean, - collapsible?: boolean + editMode?: boolean ) { - const hideWhenCollapsed = this._isNewChangeSummaryUiEnabled - ? false - : collapsed && collapsible; if ( !loggedIn || editing || (change && change.status === ChangeStatus.MERGED) || - editMode || - hideWhenCollapsed + editMode ) { return true; } @@ -1540,53 +1506,6 @@ export class GrChangeView extends KeyboardShortcutMixin(PolymerElement) { ); } - _computeChangeIdClass(displayChangeId: string) { - return displayChangeId === CHANGE_ID_ERROR.MISMATCH ? 'warning' : ''; - } - - _computeTitleAttributeWarning(displayChangeId: string) { - if (displayChangeId === CHANGE_ID_ERROR.MISMATCH) { - return 'Change-Id mismatch'; - } else if (displayChangeId === CHANGE_ID_ERROR.MISSING) { - return 'No Change-Id in commit message'; - } - return undefined; - } - - _computeChangeIdCommitMessageError( - commitMessage?: string, - change?: ChangeInfo - ) { - if (change === undefined) { - return undefined; - } - - if (!commitMessage) { - return CHANGE_ID_ERROR.MISSING; - } - - // Find the last match in the commit message: - let changeId; - let changeIdArr; - - while ((changeIdArr = CHANGE_ID_REGEX_PATTERN.exec(commitMessage))) { - changeId = changeIdArr[2]; - } - - if (changeId) { - // A change-id is detected in the commit message. - - if (changeId === change.change_id) { - // The change-id found matches the real change-id. - return null; - } - // The change-id found does not match the change-id. - return CHANGE_ID_ERROR.MISMATCH; - } - // There is no change-id in the commit message. - return CHANGE_ID_ERROR.MISSING; - } - _computeReplyButtonLabel( changeRecord?: ElementPropertyDeepChange< GrChangeView, @@ -2347,13 +2266,6 @@ export class GrChangeView extends KeyboardShortcutMixin(PolymerElement) { return `Change ${changeNum}`; } - _computeCommitMessageCollapsed(collapsed?: boolean, collapsible?: boolean) { - if (this._isNewChangeSummaryUiEnabled) { - return false; - } - return collapsible && collapsed; - } - /** * Returns the text to be copied when * click the copy icon next to change subject @@ -2366,21 +2278,11 @@ export class GrChangeView extends KeyboardShortcutMixin(PolymerElement) { ); } - _toggleCommitCollapsed() { - this._commitCollapsed = !this._commitCollapsed; - if (this._commitCollapsed) { - window.scrollTo(0, 0); - } - } - _computeCommitCollapsible(commitMessage?: string) { if (!commitMessage) { return false; } - const MIN_LINES = this._isNewChangeSummaryUiEnabled - ? 17 - : MIN_LINES_FOR_COMMIT_COLLAPSE; - return commitMessage.split('\n').length >= MIN_LINES; + return commitMessage.split('\n').length >= MIN_LINES_FOR_COMMIT_COLLAPSE; } _startUpdateCheckTimer() { diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts index eb2561ff8e..0a8bf86a2b 100644 --- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts +++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts @@ -91,11 +91,6 @@ export const htmlTemplate = html` background-color: var(--view-background-color); box-shadow: var(--elevation-level-1); } - .changeId { - color: var(--deemphasized-text-color); - font-family: var(--font-family); - margin-top: var(--spacing-l); - } .changeMetadata { /* Limit meta section to half of the screen at max */ max-width: 50%; @@ -115,18 +110,8 @@ export const htmlTemplate = html` #commitMessageEditor { /* Account for border and padding and rounding errors. */ min-width: calc(72ch + 2px + 2 * var(--spacing-m) + 0.4px); - --collapsed-max-height: 36em; - } - .new-change-summary-true #commitMessageEditor { --collapsed-max-height: 300px; } - .editCommitMessage { - margin-top: var(--spacing-l); - - --gr-button: { - padding: 5px 0px; - } - } .changeStatuses, .commitActions, .statusText { @@ -156,9 +141,6 @@ export const htmlTemplate = html` .mobile { display: none; } - .warning { - color: var(--error-text-color); - } hr { border: 0; border-top: 1px solid var(--border-color); @@ -175,13 +157,6 @@ export const htmlTemplate = html` margin: var(--spacing-l) 0; padding: 0 var(--spacing-l); } - .collapseToggleContainer { - display: flex; - margin-bottom: 8px; - } - .collapseToggleContainer gr-button { - display: block; - } .showOnEdit { display: none; } @@ -223,7 +198,7 @@ export const htmlTemplate = html` padding-top: var(--spacing-l); width: 100%; } - gr-change-summary.new-change-summary-true { + gr-change-summary { /* temporary for old checks status */ margin-bottom: var(--spacing-m); } @@ -435,10 +410,7 @@ export const htmlTemplate = html` >[[_replyButtonLabel]] -
+
-
- { assert.equal(putStub.lastCall.args[1], '\n\n\n\n\n\n\n\n'); }); - test('_computeChangeIdCommitMessageError', () => { - let commitMessage = 'Change-Id: I4ce18b2395bca69d7a9aa48bf4554faa56282483'; - let change: ChangeInfo = { - ...createChangeViewChange(), - change_id: 'I4ce18b2395bca69d7a9aa48bf4554faa56282483' as ChangeId, - }; - assert.equal( - element._computeChangeIdCommitMessageError(commitMessage, change), - null - ); - - change = { - ...createChangeViewChange(), - change_id: 'I4ce18b2395bca69d7a9aa48bf4554faa56282484' as ChangeId, - }; - assert.equal( - element._computeChangeIdCommitMessageError(commitMessage, change), - 'mismatch' - ); - - commitMessage = 'This is the greatest change.'; - assert.equal( - element._computeChangeIdCommitMessageError(commitMessage, change), - 'missing' - ); - }); - - test('multiple change Ids in commit message picks last', () => { - const commitMessage = [ - 'Change-Id: I4ce18b2395bca69d7a9aa48bf4554faa56282484', - 'Change-Id: I4ce18b2395bca69d7a9aa48bf4554faa56282483', - ].join('\n'); - let change: ChangeInfo = { - ...createChangeViewChange(), - change_id: 'I4ce18b2395bca69d7a9aa48bf4554faa56282483' as ChangeId, - }; - assert.equal( - element._computeChangeIdCommitMessageError(commitMessage, change), - null - ); - change = { - ...createChangeViewChange(), - change_id: 'I4ce18b2395bca69d7a9aa48bf4554faa56282484' as ChangeId, - }; - assert.equal( - element._computeChangeIdCommitMessageError(commitMessage, change), - 'mismatch' - ); - }); - - test('does not count change Id that starts mid line', () => { - const commitMessage = [ - 'Change-Id: I4ce18b2395bca69d7a9aa48bf4554faa56282484', - 'Change-Id: I4ce18b2395bca69d7a9aa48bf4554faa56282483', - ].join(' and '); - let change: ChangeInfo = { - ...createChangeViewChange(), - change_id: 'I4ce18b2395bca69d7a9aa48bf4554faa56282484' as ChangeId, - }; - assert.equal( - element._computeChangeIdCommitMessageError(commitMessage, change), - null - ); - change = { - ...createChangeViewChange(), - change_id: 'I4ce18b2395bca69d7a9aa48bf4554faa56282483' as ChangeId, - }; - assert.equal( - element._computeChangeIdCommitMessageError(commitMessage, change), - 'mismatch' - ); - }); - - test('_computeTitleAttributeWarning', () => { - let changeIdCommitMessageError = 'missing'; - assert.equal( - element._computeTitleAttributeWarning(changeIdCommitMessageError), - 'No Change-Id in commit message' - ); - - changeIdCommitMessageError = 'mismatch'; - assert.equal( - element._computeTitleAttributeWarning(changeIdCommitMessageError), - 'Change-Id mismatch' - ); - }); - - test('_computeChangeIdClass', () => { - let changeIdCommitMessageError = 'missing'; - assert.equal(element._computeChangeIdClass(changeIdCommitMessageError), ''); - - changeIdCommitMessageError = 'mismatch'; - assert.equal( - element._computeChangeIdClass(changeIdCommitMessageError), - 'warning' - ); - }); - test('topic is coalesced to null', done => { sinon.stub(element, '_changeChanged'); stubRestApi('getChangeDetail').returns( @@ -2113,57 +2014,6 @@ suite('gr-change-view tests', () => { }); }); - suite('commit message expand/collapse', () => { - setup(() => { - element._change = { - ...createChangeViewChange(), - revisions: createRevisions(1), - messages: createChangeMessages(1), - }; - element._change.labels = {}; - stubRestApi('getChangeDetail').callsFake(() => - Promise.resolve({ - ...createChangeViewChange(), - // new patchset was uploaded - revisions: createRevisions(2), - current_revision: getCurrentRevision(2), - messages: createChangeMessages(1), - }) - ); - }); - - test('commitCollapseToggle hidden for short commit message', () => { - element._latestCommitMessage = ''; - flush(); - const commitCollapseToggle = element.shadowRoot!.querySelector( - '#commitCollapseToggle' - ); - assert.isTrue(commitCollapseToggle?.hasAttribute('hidden')); - }); - - test('commitCollapseToggle shown for long commit message', () => { - element._latestCommitMessage = _.times(31, String).join('\n'); - const commitCollapseToggle = element.shadowRoot!.querySelector( - '#commitCollapseToggle' - ); - assert.isFalse(commitCollapseToggle?.hasAttribute('hidden')); - }); - - test('commitCollapseToggle functions', () => { - element._latestCommitMessage = _.times(35, String).join('\n'); - assert.isTrue(element._commitCollapsed); - assert.isTrue(element._commitCollapsible); - assert.isTrue(element.$.commitMessageEditor.hasAttribute('collapsed')); - const commitCollapseToggleButton = element.shadowRoot!.querySelector( - '#commitCollapseToggleButton' - )!; - tap(commitCollapseToggleButton); - assert.isFalse(element._commitCollapsed); - assert.isTrue(element._commitCollapsible); - assert.isFalse(element.$.commitMessageEditor.hasAttribute('collapsed')); - }); - }); - test('header class computation', () => { assert.equal(element._computeHeaderClass(), 'header'); assert.equal(element._computeHeaderClass(true), 'header editMode'); diff --git a/polygerrit-ui/app/services/flags/flags.ts b/polygerrit-ui/app/services/flags/flags.ts index ff950b80b2..024a3a4576 100644 --- a/polygerrit-ui/app/services/flags/flags.ts +++ b/polygerrit-ui/app/services/flags/flags.ts @@ -28,7 +28,6 @@ export enum KnownExperimentId { // be used by plugins. The new Checks UI will show up, if a plugin registers // with the new Checks plugin API. CI_REBOOT_CHECKS = 'UiFeature__ci_reboot_checks', - NEW_CHANGE_SUMMARY_UI = 'UiFeature__new_change_summary_ui', NEW_IMAGE_DIFF_UI = 'UiFeature__new_image_diff_ui', COMMENT_CONTEXT = 'UiFeature__comment_context', }