Apply fix dialog UX improvements - Loading state when apply fix is loading - Show warning when user cannot apply fix - Fix removal of footer slot from gr-dialog - it's used by apply fix dialog. Screenshot: https://imgur.com/a/4s0fNWi Release-Notes: skip Google-Bug-Id: b/277080556 Change-Id: I11d10874fdf37256c1562b8d2adb9d01359c62d8 (cherry picked from commit f63fafdf5f82d1279bfbc26cefdb834b0f746892)
diff --git a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts index 9d29cea..b97f54f 100644 --- a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts +++ b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
@@ -23,7 +23,7 @@ import {DiffLayer, ParsedChangeInfo} from '../../../types/types'; import {GrButton} from '../../shared/gr-button/gr-button'; import {TokenHighlightLayer} from '../../../embed/diff/gr-diff-builder/token-highlight-layer'; -import {css, html, LitElement} from 'lit'; +import {css, html, LitElement, nothing} from 'lit'; import {customElement, property, query, state} from 'lit/decorators.js'; import {sharedStyles} from '../../../styles/shared-styles'; import {subscribe} from '../../lit/subscription-controller'; @@ -37,6 +37,7 @@ import {highlightServiceToken} from '../../../services/highlight/highlight-service'; import {anyLineTooLong} from '../../../embed/diff/gr-diff/gr-diff-utils'; import {fireReload} from '../../../utils/event-util'; +import {when} from 'lit/directives/when.js'; interface FilePreview { filepath: string; @@ -91,6 +92,9 @@ diffPrefs?: DiffPreferencesInfo; @state() + loading = false; + + @state() onCloseFixPreviewCallbacks: ((fixapplied: boolean) => void)[] = []; private readonly restApiService = getAppContext().restApiService; @@ -158,6 +162,8 @@ <dialog id="applyFixModal" tabindex="-1"> <gr-dialog id="applyFixDialog" + ?loading=${this.loading} + .loadingLabel=${'Creating preview ...'} .confirmLabel=${this.isApplyFixLoading ? 'Saving...' : 'Apply Fix'} .confirmTooltip=${this.computeTooltip()} ?disabled=${this.computeDisableApplyFixButton()} @@ -206,30 +212,43 @@ } private renderFooter() { - const id = this.selectedFixIdx; const fixCount = this.fixSuggestions?.length ?? 0; - if (fixCount < 2) return; + const reasonForDisabledApplyButton = this.computeTooltip(); + if (fixCount < 2 && !reasonForDisabledApplyButton) return nothing; + return html`<div slot="footer" class="fix-picker"> + ${when(fixCount >= 2, () => + this.renderNavForMultipleSuggestedFixes(fixCount) + )} + ${this.renderWarning(reasonForDisabledApplyButton)} + </div>`; + } + + private renderNavForMultipleSuggestedFixes(fixCount: number) { + const id = this.selectedFixIdx; return html` - <div slot="footer" class="fix-picker"> - <span>Suggested fix ${id + 1} of ${fixCount}</span> - <gr-button - id="prevFix" - @click=${this.onPrevFixClick} - ?disabled=${id === 0} - > - <gr-icon icon="chevron_left"></gr-icon> - </gr-button> - <gr-button - id="nextFix" - @click=${this.onNextFixClick} - ?disabled=${id === fixCount - 1} - > - <gr-icon icon="chevron_right"></gr-icon> - </gr-button> - </div> + <span>Suggested fix ${id + 1} of ${fixCount}</span> + <gr-button + id="prevFix" + @click=${this.onPrevFixClick} + ?disabled=${id === 0} + > + <gr-icon icon="chevron_left"></gr-icon> + </gr-button> + <gr-button + id="nextFix" + @click=${this.onNextFixClick} + ?disabled=${id === fixCount - 1} + > + <gr-icon icon="chevron_right"></gr-icon> + </gr-button> `; } + private renderWarning(message: string) { + if (!message) return nothing; + return html`<span><gr-icon icon="info"></gr-icon>${message}</span>`; + } + /** * Given event with fixSuggestions, fetch diffs associated with first * suggested fix and open dialog. @@ -246,7 +265,9 @@ private async showSelectedFixSuggestion(fixSuggestion: FixSuggestionInfo) { this.currentFix = fixSuggestion; + this.loading = true; await this.fetchFixPreview(fixSuggestion); + this.loading = false; } private async fetchFixPreview(fixSuggestion: FixSuggestionInfo) { @@ -333,7 +354,7 @@ const latestPatchNum = this.change.revisions[this.change.current_revision]._number; return latestPatchNum !== this.patchNum - ? 'Fix can only be applied to the latest patchset' + ? 'You cannot apply this fix because it is from a previous patchset' : ''; }
diff --git a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.ts b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.ts index 36fadff..267c569 100644 --- a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.ts +++ b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.ts
@@ -174,7 +174,7 @@ assert.isTrue(button.hasAttribute('disabled')); assert.equal( button.getAttribute('title'), - 'Fix can only be applied to the latest patchset' + 'You cannot apply this fix because it is from a previous patchset' ); }); }); @@ -185,7 +185,7 @@ element, /* HTML */ ` <dialog id="applyFixModal" tabindex="-1" open=""> - <gr-dialog id="applyFixDialog" role="dialog"> + <gr-dialog id="applyFixDialog" role="dialog" loading=""> <div slot="header">Fix fix_1</div> <div slot="main"></div> <div class="fix-picker" slot="footer">
diff --git a/polygerrit-ui/app/elements/shared/gr-dialog/gr-dialog.ts b/polygerrit-ui/app/elements/shared/gr-dialog/gr-dialog.ts index 6536806..93201c8 100644 --- a/polygerrit-ui/app/elements/shared/gr-dialog/gr-dialog.ts +++ b/polygerrit-ui/app/elements/shared/gr-dialog/gr-dialog.ts
@@ -152,6 +152,7 @@ <span class="loadingLabel"> ${this.loadingLabel} </span> ` )} + <slot name="footer"></slot> <div class="flex-space"></div> <gr-button id="cancel"
diff --git a/polygerrit-ui/app/elements/shared/gr-dialog/gr-dialog_test.ts b/polygerrit-ui/app/elements/shared/gr-dialog/gr-dialog_test.ts index d386c32..41fcfed 100644 --- a/polygerrit-ui/app/elements/shared/gr-dialog/gr-dialog_test.ts +++ b/polygerrit-ui/app/elements/shared/gr-dialog/gr-dialog_test.ts
@@ -36,6 +36,7 @@ </div> </main> <footer> + <slot name="footer"></slot> <div class="flex-space"></div> <gr-button aria-disabled="false" @@ -81,6 +82,7 @@ <span class="loadingSpin" aria-label="Loading!!" role="progressbar"> </span> <span class="loadingLabel"> Loading!! </span> + <slot name="footer"></slot> <div class="flex-space"></div> <gr-button aria-disabled="false"