Polish timeline UI

webviews/common/common.css

@@ -163,7 +163,7 @@ body img.avatar {
 }
 
 .author-link {
-	font-weight: bolder;
+	font-weight: 600;
 	color: var(--vscode-editor-foreground);
 }
 
@@ -173,21 +173,21 @@ body img.avatar {
 
 .automerge-section {
 	display: flex;
+	padding: 16px;
+	background: var(--vscode-editorHoverWidget-background);
+	border-bottom-left-radius: 3px;
+	border-bottom-right-radius: 3px;
 }
 
-.automerge-section .merge-select-container{
-	padding-top: 4px;
-	padding-left: 4px;
+.automerge-section .merge-select-container {
+	margin-left: 8px;
 }
 
 .automerge-checkbox-wrapper,
 .automerge-checkbox-label {
 	display: flex;
 	align-items: center;
-}
-
-body button.comment-resolve {
-	margin: 10px;
+	margin-right: 4px;
 }
 
 /** Theming */

webviews/components/merge.tsx

@@ -3,7 +3,16 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import React, { ChangeEventHandler, Context, useCallback, useContext, useEffect, useReducer, useRef, useState } from 'react';
+import React, {
+	ChangeEventHandler,
+	Context,
+	useCallback,
+	useContext,
+	useEffect,
+	useReducer,
+	useRef,
+	useState,
+} from 'react';
 import { groupBy } from '../../src/common/utils';
 import { GithubItemStateEnum, MergeMethod, PullRequestMergeability } from '../../src/github/interface';
 import { PullRequest } from '../common/cache';
@@ -15,17 +24,19 @@ import { alertIcon, checkIcon, deleteIcon, mergeIcon, pendingIcon, skipIcon } fr
 import { nbsp } from './space';
 import { Avatar } from './user';
 
-const PRStatusMessage = ({ pr, isSimple }: { pr: PullRequest, isSimple: boolean }) => {
+const PRStatusMessage = ({ pr, isSimple }: { pr: PullRequest; isSimple: boolean }) => {
 	return pr.state === GithubItemStateEnum.Merged ? (
-		<div className="branch-status-message"><div className="branch-status-icon">{isSimple ? mergeIcon : null}</div> {'Pull request successfully merged.'}</div>
+		<div className="branch-status-message">
+			<div className="branch-status-icon">{isSimple ? mergeIcon : null}</div>{' '}
+			{'Pull request successfully merged.'}
+		</div>
 	) : pr.state === GithubItemStateEnum.Closed ? (
 		<div className="branch-status-message">{'This pull request is closed.'}</div>
 	) : null;
 };
 
 const DeleteOption = ({ pr }: { pr: PullRequest }) => {
-	return pr.state === GithubItemStateEnum.Open ? null
-		: (<DeleteBranch {...pr} />);
+	return pr.state === GithubItemStateEnum.Open ? null : <DeleteBranch {...pr} />;
 };
 
 const StatusChecks = ({ pr }: { pr: PullRequest }) => {
@@ -47,33 +58,35 @@ const StatusChecks = ({ pr }: { pr: PullRequest }) => {
 		}
 	}, status.statuses);
 
-	return ((state === GithubItemStateEnum.Open) && status.statuses.length) ? (
+	return state === GithubItemStateEnum.Open && status.statuses.length ? (
 		<>
 			<div className="status-section">
 				<div className="status-item">
 					<StateIcon state={status.state} />
-					<div>{getSummaryLabel(status.statuses)}</div>
-					<a href="javascript:void(0)" aria-role="button" onClick={toggleDetails}>
-						{showDetails ? 'Hide' : 'Show'}
-					</a>
+					<div className="status-item-detail-text">
+						<span>{getSummaryLabel(status.statuses)}</span>
+						<a href="javascript:void(0)" aria-role="button" onClick={toggleDetails}>
+							{showDetails ? 'Hide' : 'Show'}
+						</a>
+					</div>
 				</div>
 				{showDetails ? <StatusCheckDetails statuses={status.statuses} /> : null}
 			</div>
 		</>
 	) : null;
 };
 
-const InlineReviewers = ({ pr, isSimple }: { pr: PullRequest, isSimple: boolean }) => {
-	return (isSimple && (pr.state === GithubItemStateEnum.Open))
-		? pr.reviewers
-			?
-			<> {
-				pr.reviewers.map(state => (
+const InlineReviewers = ({ pr, isSimple }: { pr: PullRequest; isSimple: boolean }) => {
+	return isSimple && pr.state === GithubItemStateEnum.Open ? (
+		pr.reviewers ? (
+			<>
+				{' '}
+				{pr.reviewers.map(state => (
 					<Reviewer key={state.reviewer.login} {...state} canDelete={false} />
-				))
-			}</>
-			: null
-		: null;
+				))}
+			</>
+		) : null
+	) : null;
 };
 
 export const StatusChecksSection = ({ pr, isSimple }: { pr: PullRequest; isSimple: boolean }) => {
@@ -97,7 +110,7 @@ export const StatusChecksSection = ({ pr, isSimple }: { pr: PullRequest; isSimpl
 };
 
 export const MergeStatusAndActions = ({ pr, isSimple }: { pr: PullRequest; isSimple: boolean }) => {
-	if (isSimple && (pr.state !== GithubItemStateEnum.Open)) {
+	if (isSimple && pr.state !== GithubItemStateEnum.Open) {
 		const { create } = useContext(PullRequestContext);
 
 		const string = 'Create New Pull Request...';
@@ -147,18 +160,18 @@ export const MergeStatus = ({ mergeable, isSimple }: { mergeable: PullRequestMer
 			{isSimple
 				? null
 				: mergeable === PullRequestMergeability.Mergeable
-					? checkIcon
-					: (mergeable === PullRequestMergeability.NotMergeable || mergeable === PullRequestMergeability.Conflict)
-						? deleteIcon
-						: pendingIcon}
+				? checkIcon
+				: mergeable === PullRequestMergeability.NotMergeable || mergeable === PullRequestMergeability.Conflict
+				? deleteIcon
+				: pendingIcon}
 			<div>
-				{(mergeable) === PullRequestMergeability.Mergeable
+				{mergeable === PullRequestMergeability.Mergeable
 					? 'This branch has no conflicts with the base branch.'
 					: mergeable === PullRequestMergeability.Conflict
-						? 'This branch has conflicts that must be resolved.'
-						: mergeable === PullRequestMergeability.NotMergeable
-							? 'Branch protection policy must be fulfilled before merging.'
-							: 'Checking if this branch can be merged...'}
+					? 'This branch has conflicts that must be resolved.'
+					: mergeable === PullRequestMergeability.NotMergeable
+					? 'Branch protection policy must be fulfilled before merging.'
+					: 'Checking if this branch can be merged...'}
 			</div>
 		</div>
 	);
@@ -223,8 +236,15 @@ export const PrActions = ({ pr, isSimple }: { pr: PullRequest; isSimple: boolean
 		return isSimple ? <MergeSimple {...pr} /> : <Merge {...pr} />;
 	} else if (hasWritePermission) {
 		const ctx = useContext(PullRequestContext);
-		return <AutoMerge updateState={(params: Partial<{ autoMerge: boolean; autoMergeMethod: MergeMethod; }>) => { ctx.updateAutoMerge(params); }}
-			{...pr} defaultMergeMethod={pr.autoMergeMethod ?? pr.defaultMergeMethod} />;
+		return (
+			<AutoMerge
+				updateState={(params: Partial<{ autoMerge: boolean; autoMergeMethod: MergeMethod }>) => {
+					ctx.updateAutoMerge(params);
+				}}
+				{...pr}
+				defaultMergeMethod={pr.autoMergeMethod ?? pr.defaultMergeMethod}
+			/>
+		);
 	}
 
 	return null;
@@ -322,7 +342,7 @@ function ConfirmMerge({ pr, method, cancel }: { pr: PullRequest; method: MergeMe
 				<div className="form-actions">
 					<button className="secondary" onClick={cancel}>
 						Cancel
-				</button>
+					</button>
 					<input disabled={isBusy} type="submit" id="confirm-merge" value={MERGE_METHODS[method]} />
 				</div>
 			</form>
@@ -351,7 +371,8 @@ const MERGE_METHODS = {
 	rebase: 'Rebase and Merge',
 };
 
-type MergeSelectProps = Pick<PullRequest, 'mergeMethodsAvailability'> & Pick<PullRequest, 'defaultMergeMethod'> & { onChange?: ChangeEventHandler<HTMLSelectElement> };
+type MergeSelectProps = Pick<PullRequest, 'mergeMethodsAvailability'> &
+	Pick<PullRequest, 'defaultMergeMethod'> & { onChange?: ChangeEventHandler<HTMLSelectElement> };
 
 export const MergeSelect = React.forwardRef<HTMLSelectElement, MergeSelectProps>(
 	({ defaultMergeMethod, mergeMethodsAvailability: avail, onChange }: MergeSelectProps, ref) => (
@@ -370,15 +391,19 @@ const StatusCheckDetails = ({ statuses }: Partial<PullRequest['status']>) => (
 	<div>
 		{statuses.map(s => (
 			<div key={s.id} className="status-check">
-				<div>
+				<div className="status-check-details">
 					<StateIcon state={s.state} />
 					<Avatar for={{ avatarUrl: s.avatar_url, url: s.url }} />
 					<span className="status-check-detail-text">
 						{/* allow-any-unicode-next-line */}
 						{s.context} {s.description ? `— ${s.description}` : ''}
 					</span>
 				</div>
-				{!!s.target_url ? <a href={s.target_url} title={s.target_url}>Details</a> : null}
+				{!!s.target_url ? (
+					<a href={s.target_url} title={s.target_url}>
+						Details
+					</a>
+				) : null}
 			</div>
 		))}
 	</div>

webviews/components/timeline.tsx

@@ -69,11 +69,12 @@ const CommitEventView = (event: CommitEvent) => (
 				{event.message}
 			</a>
 		</div>
-		<a className="sha" href={event.htmlUrl} title={event.htmlUrl}>
-			{event.sha.slice(0, 7)}
-		</a>
-		{nbsp}
-		<Timestamp date={event.authoredDate} />
+		<div className="sha-with-timestamp">
+			<a className="sha" href={event.htmlUrl} title={event.htmlUrl}>
+				{event.sha.slice(0, 7)}
+			</a>
+			<Timestamp date={event.authoredDate} />
+		</div>
 	</div>
 );
 
@@ -86,7 +87,11 @@ const NewCommitsSinceReviewEventView = () => {
 				{nbsp}
 				<span style={{ fontWeight: 'bold' }}>New changes since your last Review</span>
 			</div>
-			<button aria-live="polite" title="View the changes since your last review" onClick={() => gotoChangesSinceReview()}>
+			<button
+				aria-live="polite"
+				title="View the changes since your last review"
+				onClick={() => gotoChangesSinceReview()}
+			>
 				View Changes
 			</button>
 		</div>
@@ -97,8 +102,8 @@ const association = ({ authorAssociation }: ReviewEvent, format = (assoc: string
 	authorAssociation.toLowerCase() === 'user'
 		? format('you')
 		: authorAssociation && authorAssociation !== 'NONE'
-			? format(authorAssociation)
-			: null;
+		? format(authorAssociation)
+		: null;
 
 const positionKey = (comment: IComment) =>
 	comment.position !== null ? `pos:${comment.position}` : `ori:${comment.originalPosition}`;
@@ -140,11 +145,16 @@ const ReviewEventView = (event: ReviewEvent) => {
 				{event.state !== 'PENDING' && event.body ? (
 					<CommentBody body={event.body} bodyHTML={event.bodyHTML} canApplyPatch={false} />
 				) : null}
-				<div className="comment-body review-comment-body">
-					{Object.entries(comments).map(([key, thread]) => {
-						return <CommentThread key={key} thread={thread} event={event} />;
-					})}
-				</div>
+
+				{/* Don't show the empty comment body unless a comment has been written. Shows diffs and suggested changes. */}
+				{event.comments.length ? (
+					<div className="comment-body review-comment-body">
+						{Object.entries(comments).map(([key, thread]) => {
+							return <CommentThread key={key} thread={thread} event={event} />;
+						})}
+					</div>
+				) : null}
+
 				{reviewIsPending ? <AddReviewSummaryComment /> : null}
 			</div>
 		</div>
@@ -156,7 +166,8 @@ function CommentThread({ thread, event }: { thread: IComment[]; event: ReviewEve
 	const [revealed, setRevealed] = useState(!comment.isResolved);
 	const [resolved, setResolved] = useState(!!comment.isResolved);
 	const { openDiff, toggleResolveComment } = useContext(PullRequestContext);
-	const resolvePermission = event.reviewThread &&
+	const resolvePermission =
+		event.reviewThread &&
 		((event.reviewThread.canResolve && !event.reviewThread.isResolved) ||
 			(event.reviewThread.canUnresolve && event.reviewThread.isResolved));
 
@@ -183,7 +194,7 @@ function CommentThread({ thread, event }: { thread: IComment[]; event: ReviewEve
 							{comment.path}
 						</a>
 					)}
-					{!resolved && !revealed ? <span className='unresolvedLabel'>Unresolved</span> : null}
+					{!resolved && !revealed ? <span className="unresolvedLabel">Unresolved</span> : null}
 				</div>
 				<button className="secondary" onClick={() => setRevealed(!revealed)}>
 					{revealed ? 'Hide' : 'Show'}
@@ -195,13 +206,13 @@ function CommentThread({ thread, event }: { thread: IComment[]; event: ReviewEve
 					{thread.map(c => (
 						<CommentView key={c.id} {...c} pullRequestReviewId={event.id} />
 					))}
-					{resolvePermission ?
-						<div className='comment-container comment review-comment'>
+					{resolvePermission ? (
+						<div className="resolve-comment-row">
 							<button className="secondary comment-resolve" onClick={() => toggleResolve()}>
 								{resolved ? 'Unresolve Conversation' : 'Resolve Conversation'}
 							</button>
 						</div>
-						: null}
+					) : null}
 				</div>
 			) : null}
 		</div>
@@ -217,7 +228,11 @@ function AddReviewSummaryComment() {
 			<textarea ref={comment} placeholder="Leave a review summary comment"></textarea>
 			<div className="form-actions">
 				{isAuthor ? null : (
-					<button id="request-changes" className="push-right" onClick={() => requestChanges(comment.current.value)}>
+					<button
+						id="request-changes"
+						className="push-right"
+						onClick={() => requestChanges(comment.current.value)}
+					>
 						Request Changes
 					</button>
 				)}
@@ -226,7 +241,11 @@ function AddReviewSummaryComment() {
 						Approve
 					</button>
 				)}
-				<button id="submit" className={isAuthor ? 'push-right' : ''} onClick={() => submit(comment.current.value)}>
+				<button
+					id="submit"
+					className={isAuthor ? 'push-right' : ''}
+					onClick={() => submit(comment.current.value)}
+				>
 					Submit Review
 				</button>
 			</div>

webviews/createPullRequestView/index.css

@@ -90,13 +90,13 @@ body {
 	display: flex;
 }
 
-.automerge-section .merge-select-container{
+.automerge-section .merge-select-container {
 	padding-top: 4px;
 	padding-left: 4px;
 }
 
 .automerge-checkbox-wrapper,
 .automerge-checkbox-label {
 	display: flex;
-    align-items: center;
+	align-items: center;
 }