diff --git a/webviews/components/merge.tsx b/webviews/components/merge.tsx index 347d2299f7..b13faf6ca2 100644 --- a/webviews/components/merge.tsx +++ b/webviews/components/merge.tsx @@ -542,7 +542,7 @@ export const DeleteBranch = (pr: PullRequest) => { return
; } else { return ( -
+
{ event.preventDefault(); diff --git a/webviews/components/pullRequestStack.tsx b/webviews/components/pullRequestStack.tsx index 219e30e9a6..d5fba43a44 100644 --- a/webviews/components/pullRequestStack.tsx +++ b/webviews/components/pullRequestStack.tsx @@ -4,30 +4,30 @@ *--------------------------------------------------------------------------------------------*/ import * as React from 'react'; -import { checkIcon, chevronDownIcon, circleFilledIcon, closeIcon, layersIcon, warningIcon } from './icon'; +import { chevronDownIcon, circleFilledIcon, gitMergeIcon, gitPullRequestDraftIcon, layersIcon, passIcon, skipIcon } from './icon'; import { GithubItemStateEnum, PullRequestMergeability, PullRequestStack as Stack } from '../../src/github/interface'; import { PullRequest } from '../../src/github/views'; import PullRequestContext from '../common/context'; -function getReadiness(entry: Stack['pullRequests'][number]): { icon: JSX.Element; label: string; kind: string } { +function getReadiness(entry: Stack['pullRequests'][number], currentPosition: number): { icon: JSX.Element; label: string; kind: string } { if (entry.state === GithubItemStateEnum.Merged) { - return { icon: checkIcon, label: 'Already merged', kind: 'ready' }; + return { icon: gitMergeIcon, label: 'Already merged', kind: 'merged' }; } if (entry.state === GithubItemStateEnum.Closed) { - return { icon: closeIcon, label: 'Closed pull request cannot be merged', kind: 'blocked' }; + return { icon: skipIcon, label: 'Closed pull request cannot be merged', kind: 'blocked' }; } if (entry.isDraft) { - return { icon: warningIcon, label: 'Draft pull request cannot be merged', kind: 'waiting' }; + return { icon: gitPullRequestDraftIcon, label: 'Draft pull request cannot be merged', kind: 'draft' }; } switch (entry.mergeable) { case PullRequestMergeability.Mergeable: - return { icon: checkIcon, label: 'Ready to merge', kind: 'ready' }; + return { icon: entry.position > currentPosition ? circleFilledIcon : passIcon, label: 'Ready to merge', kind: 'ready' }; case PullRequestMergeability.Conflict: - return { icon: closeIcon, label: 'Merge conflicts', kind: 'blocked' }; + return { icon: circleFilledIcon, label: 'Merge conflicts', kind: 'waiting' }; case PullRequestMergeability.NotMergeable: - return { icon: closeIcon, label: 'Merge requirements not met', kind: 'blocked' }; + return { icon: circleFilledIcon, label: 'Merge requirements not met', kind: 'waiting' }; case PullRequestMergeability.Behind: - return { icon: warningIcon, label: 'Branch is behind its base', kind: 'waiting' }; + return { icon: circleFilledIcon, label: 'Branch is behind its base', kind: 'waiting' }; default: return { icon: circleFilledIcon, label: 'Mergeability is being checked', kind: 'waiting' }; } @@ -87,7 +87,7 @@ export const StackSection = ({ pr }: { pr: PullRequest }) => {
    {[...stack.pullRequests].reverse().map(entry => { const current = entry.number === pr.number; - const readiness = getReadiness(entry); + const readiness = getReadiness(entry, stack.position); return
  1. {readiness.icon} diff --git a/webviews/editorWebview/index.css b/webviews/editorWebview/index.css index fa0489db81..90b10eccd0 100644 --- a/webviews/editorWebview/index.css +++ b/webviews/editorWebview/index.css @@ -544,6 +544,15 @@ button.input-box { display: inline-block; } +#status-checks .stacked-delete-branch-container { + display: block; + padding: 12px 16px; +} + +#status-checks .stacked-delete-branch-container form { + margin: 0; +} + #status-checks .branch-status-message { display: inline-block; line-height: 100%; @@ -789,14 +798,52 @@ body button .icon { fill: var(--vscode-issues-open); } +.stack-entry-readiness.merged { + color: var(--vscode-pullRequests-merged); +} + +.stack-entry-readiness.merged svg path { + fill: var(--vscode-pullRequests-merged); +} + +.stack-entry-readiness.draft { + color: var(--vscode-pullRequests-draft); +} + +.stack-entry-readiness.draft svg path { + fill: var(--vscode-pullRequests-draft); +} + +.vscode-high-contrast .stack-entry-readiness.merged, +.vscode-high-contrast-light .stack-entry-readiness.merged, +.vscode-high-contrast .stack-entry-readiness.draft, +.vscode-high-contrast-light .stack-entry-readiness.draft { + color: var(--vscode-foreground); +} + +.vscode-high-contrast .stack-entry-readiness.merged svg path, +.vscode-high-contrast-light .stack-entry-readiness.merged svg path, +.vscode-high-contrast .stack-entry-readiness.draft svg path, +.vscode-high-contrast-light .stack-entry-readiness.draft svg path { + fill: var(--vscode-foreground); +} + .stack-entry-readiness.blocked { - color: var(--vscode-errorForeground); + color: var(--vscode-pullRequests-closed); +} + +.stack-entry-readiness.blocked svg path { + fill: var(--vscode-pullRequests-closed); } .stack-entry-readiness.waiting { color: var(--vscode-list-warningForeground); } +.stack-entry-readiness.waiting svg path { + fill: var(--vscode-list-warningForeground); +} + .stack-entry-details { min-width: 0; overflow-wrap: anywhere; diff --git a/webviews/editorWebview/test/overview.test.tsx b/webviews/editorWebview/test/overview.test.tsx index bd0a624280..b8d4243c8d 100644 --- a/webviews/editorWebview/test/overview.test.tsx +++ b/webviews/editorWebview/test/overview.test.tsx @@ -95,7 +95,7 @@ describe('Overview', function () { 'First Change#793 - D1', ]); assert.deepStrictEqual([...section.querySelectorAll('.stack-entry-readiness')].map(entry => [entry.classList[1], entry.getAttribute('aria-label')]), [ - ['waiting', 'Draft pull request cannot be merged'], + ['draft', 'Draft pull request cannot be merged'], ['ready', 'Ready to merge'], ['ready', 'Ready to merge'], ]); @@ -126,6 +126,54 @@ describe('Overview', function () { assert(openOnGitHub.notCalled); }); + it('shows stack state icons appropriate to each pull request and its position', function () { + const states = [ + { state: GithubItemStateEnum.Merged, isDraft: false, mergeable: PullRequestMergeability.Unknown }, + { state: GithubItemStateEnum.Closed, isDraft: false, mergeable: PullRequestMergeability.Unknown }, + { state: GithubItemStateEnum.Open, isDraft: false, mergeable: PullRequestMergeability.Mergeable }, + { state: GithubItemStateEnum.Open, isDraft: false, mergeable: PullRequestMergeability.Mergeable }, + { state: GithubItemStateEnum.Open, isDraft: false, mergeable: PullRequestMergeability.Mergeable }, + { state: GithubItemStateEnum.Open, isDraft: true, mergeable: PullRequestMergeability.Mergeable }, + { state: GithubItemStateEnum.Open, isDraft: false, mergeable: PullRequestMergeability.Conflict }, + { state: GithubItemStateEnum.Open, isDraft: false, mergeable: PullRequestMergeability.NotMergeable }, + { state: GithubItemStateEnum.Open, isDraft: false, mergeable: PullRequestMergeability.Behind }, + { state: GithubItemStateEnum.Open, isDraft: false, mergeable: PullRequestMergeability.Unknown }, + ]; + const pr = new PullRequestBuilder().number(4).stack({ + position: 4, size: states.length, base: 'main', + pullRequests: states.map((state, index) => ({ + ...state, position: index + 1, number: index + 1, title: `PR ${index + 1}`, + head: `D${index + 1}`, url: `https://example.com/${index + 1}`, + })), + }).build(); + const out = render( + + + , + ); + const iconPath = (svg: string) => { + const element = document.createElement('div'); + element.innerHTML = svg; + return element.querySelector('path')?.getAttribute('d'); + }; + const dot = iconPath(require('../../../resources/icons/codicons/circle-filled.svg')); + const pass = iconPath(require('../../../resources/icons/codicons/pass.svg')); + assert.deepStrictEqual([...out.container.querySelectorAll('.stack-entry-readiness')].map(entry => [ + entry.getAttribute('aria-label'), entry.classList[1], entry.querySelector('svg path')?.getAttribute('d'), + ]), [ + ['Mergeability is being checked', 'waiting', dot], + ['Branch is behind its base', 'waiting', dot], + ['Merge requirements not met', 'waiting', dot], + ['Merge conflicts', 'waiting', dot], + ['Draft pull request cannot be merged', 'draft', iconPath(require('../../../resources/icons/codicons/git-pull-request-draft.svg'))], + ['Ready to merge', 'ready', dot], + ['Ready to merge', 'ready', pass], + ['Ready to merge', 'ready', pass], + ['Closed pull request cannot be merged', 'blocked', iconPath(require('../../../resources/icons/codicons/skip.svg'))], + ['Already merged', 'merged', iconPath(require('../../../resources/icons/codicons/git-merge.svg'))], + ]); + }); + it('does not show a stack badge or section for an unstacked pull request', function () { const pr = new PullRequestBuilder().build(); const out = render( @@ -221,9 +269,23 @@ describe('Overview', function () { 'Mergeability is being checked', 'Closed pull request cannot be merged', ]); + assert(out.container.querySelector('.stack-entry-readiness.blocked .icon.skip')); + assert.strictEqual(out.container.querySelector('#status-checks > .stacked-delete-branch-container button')?.textContent?.trim(), 'Delete Branch...'); assert.strictEqual(out.container.querySelector('.automerge-section'), null); }); + it('keeps the original Delete Branch placement outside stacks', function () { + const pr = new PullRequestBuilder().state(GithubItemStateEnum.Closed).build(); + const out = render( + + + , + ); + + assert.strictEqual(out.container.querySelector('#pull-request-stack'), null); + assert.strictEqual(out.container.querySelector('#status-checks > .branch-status-container:not(.stacked-delete-branch-container) button')?.textContent?.trim(), 'Delete Branch...'); + }); + it('does not count already merged pull requests in the merge impact', function () { const pr = new PullRequestBuilder().number(795).stack({ position: 3,