Polish stack view

webviews/components/merge.tsx

@@ -542,7 +542,7 @@ export const DeleteBranch = (pr: PullRequest) => {
 		return <div />;
 	} else {
 		return (
-			<div className="branch-status-container">
+			<div className={`branch-status-container${pr.stack ? ' stacked-delete-branch-container' : ''}`}>
 				<form
 					onSubmit={async event => {
 						event.preventDefault();

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 }) => {
 			<ol className="stack-entries" aria-label={`Pull requests merging down into ${stack.base}`}>
 				{[...stack.pullRequests].reverse().map(entry => {
 					const current = entry.number === pr.number;
-					const readiness = getReadiness(entry);
+					const readiness = getReadiness(entry, stack.position);
 
 					return <li key={entry.number} className={`stack-entry${current ? ' current' : ''}`} aria-current={current ? 'step' : undefined}>
 						<span className={`stack-entry-readiness ${readiness.kind}`} role="img" aria-label={readiness.label} title={readiness.label}>{readiness.icon}</span>

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;

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(
+			<PullRequestContext.Provider value={new PRContext(pr)}>
+				<Overview {...pr} />
+			</PullRequestContext.Provider>,
+		);
+		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(
+			<PullRequestContext.Provider value={new PRContext(pr)}>
+				<Overview {...pr} />
+			</PullRequestContext.Provider>,
+		);
+
+		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,