Improves rebase editor

- adds committer info
- ensures focus is restored on long-running updates
- prevents offscreen rows from being rendered
- appends entries to the DOM instead of a DocumentFragment

Co-authored-by: Eric Amodio <eamodio@gmail.com>

CHANGELOG.md

@@ -18,6 +18,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p
     - Adds a `gitlens.rebaseEditor.showDetailsView` setting to specify when to show the _Commit Details_ view for the selected row in the _Interactive Rebase Editor_
   - Adds full (multiline) commit message
   - Adds the `f` fixup shortcut key to UI
+  - Consolidates the UI for author and committer information into a stack of avatars
   - Adds emoji support for commit messages — closes [#1789](https://github.com/gitkraken/vscode-gitlens/issues/1789)
   - Ensures that large rebases show rich commit details
 - Changes the _Home_ view to always be available

src/webviews/apps/rebase/rebase.scss

@@ -3,9 +3,17 @@
 @import '../shared/utils';
 
 body {
+	--avatar-size: 2.2rem;
 	overflow: overlay;
 }
 
+.vscode-dark {
+	--avatar-bg: var(--color-background--lighten-30);
+}
+.vscode-light {
+	--avatar-bg: var(--color-background--darken-30);
+}
+
 .container {
 	display: grid;
 	font-size: 1.3em;
@@ -80,16 +88,7 @@ footer {
 }
 
 .entries {
-	border-left: 2px solid;
-	margin-left: 10px;
-	padding-left: 4px;
-
-	.vscode-dark & {
-		border-color: var(--color-background--lighten-15);
-	}
-	.vscode-light & {
-		border-color: var(--color-background--darken-15);
-	}
+	padding: 0;
 }
 
 .entries--empty {
@@ -136,17 +135,42 @@ footer {
 	}
 }
 
-$entry-padding: 5px;
+$entry-padding: 7px;
 
-.entry {
+.entry,
+.entry-blocked {
 	display: flex;
 	align-items: center;
 	justify-content: space-between;
-	margin: 0px 5px 0px 10px;
 	padding: $entry-padding 0;
-	border: 2px solid transparent;
 	border-radius: 3px;
-	position: relative;
+}
+
+.entry {
+	padding-left: 26px;
+	margin: 0 5px 0 0;
+	content-visibility: auto;
+	contain-intrinsic-size: auto 36px;
+
+	&::before {
+		display: inline-block;
+		content: ' ';
+		background-color: var(--color-background);
+		border-right: 2px solid;
+		height: 100%;
+		position: absolute;
+		z-index: 0;
+		left: 10px;
+		top: 0;
+		transform: translateX(-50%);
+
+		.vscode-dark & {
+			border-right-color: var(--color-background--lighten-15);
+		}
+		.vscode-light & {
+			border-right-color: var(--color-background--darken-15);
+		}
+	}
 
 	&::after {
 		display: inline-block;
@@ -156,15 +180,14 @@ $entry-padding: 5px;
 		border-radius: 50%;
 		height: 12px;
 		width: 12px;
-		margin-left: -25px;
+		left: 2px;
 		position: absolute;
 		z-index: 2;
 	}
 
 	&:focus-within {
-		outline: none;
-		border: 2px solid var(--color-highlight--50);
-		border-radius: 3px;
+		outline: 2px solid var(--color-highlight--50);
+		outline-offset: -2px;
 	}
 
 	&.entry--edit,
@@ -175,14 +198,7 @@ $entry-padding: 5px;
 		}
 
 		&::before {
-			display: inline-block;
-			content: ' ';
-			background-color: var(--color-background);
-			border-right: 2px solid rgba(0, 153, 0, 1);
-			height: #{28px + ($entry-padding * 2)};
-			margin-left: -18px;
-			position: absolute;
-			z-index: 1;
+			border-right-color: rgba(0, 153, 0, 1);
 		}
 	}
 
@@ -193,15 +209,7 @@ $entry-padding: 5px;
 		}
 
 		&::before {
-			display: inline-block;
-			content: ' ';
-			background-color: var(--color-background);
-			border-right: 2px solid rgba(212, 153, 0, 1);
-			height: #{31px + ($entry-padding * 2)};
-			margin-left: -18px;
-			margin-top: 5px;
-			position: absolute;
-			z-index: 1;
+			border-right-color: rgba(212, 153, 0, 1);
 		}
 	}
 
@@ -211,14 +219,7 @@ $entry-padding: 5px;
 		}
 
 		&::before {
-			display: inline-block;
-			content: ' ';
-			background-color: var(--color-background);
-			border-right: 2px solid rgba(153, 0, 0, 1);
-			height: #{28px + ($entry-padding * 2)};
-			margin-left: -18px;
-			position: absolute;
-			z-index: 1;
+			border-right-color: rgba(153, 0, 0, 1);
 		}
 	}
 
@@ -229,18 +230,11 @@ $entry-padding: 5px;
 		}
 
 		&::before {
-			display: inline-block;
-			content: ' ';
-			background-color: var(--color-background);
-			border-right: 2px solid rgba(212, 153, 0, 1);
-			height: 40px;
-			margin-left: -18px;
-			margin-top: -5px;
-			position: absolute;
-			z-index: 1;
+			border-right-color: rgba(212, 153, 0, 1);
 		}
 	}
 
+	.entry-blocked,
 	&.entry--base,
 	&.entry--done {
 		& > .entry-action {
@@ -252,7 +246,6 @@ $entry-padding: 5px;
 			opacity: 0.4;
 		}
 
-		& > .entry-author,
 		& > .entry-date {
 			opacity: 0.3;
 		}
@@ -288,20 +281,12 @@ $entry-padding: 5px;
 
 	&.entry--base {
 		.entries--ascending & {
-			margin-bottom: 10px;
+			padding-top: 1px;
+			padding-bottom: 9px;
 		}
 		.entries:not(.entries--ascending) & {
-			margin-top: 10px;
-		}
-
-		.vscode-dark & {
-			background: rgba(255, 255, 255, 0.1);
-			box-shadow: 0px -1px 0px 0px rgba(255, 255, 255, 0.2);
-		}
-
-		.vscode-light & {
-			background: rgba(0, 0, 0, 0.1);
-			box-shadow: 0px -1px 0px 0px rgba(0, 0, 0, 0.2);
+			padding-top: 9px;
+			padding-bottom: 1px;
 		}
 	}
 
@@ -326,6 +311,20 @@ $entry-padding: 5px;
 	}
 }
 
+.entry-blocked {
+	width: 100%;
+
+	.vscode-dark & {
+		background: rgba(255, 255, 255, 0.1);
+		box-shadow: 0px -1px 0px 0px rgba(255, 255, 255, 0.2);
+	}
+
+	.vscode-light & {
+		background: rgba(0, 0, 0, 0.1);
+		box-shadow: 0px -1px 0px 0px rgba(0, 0, 0, 0.2);
+	}
+}
+
 .entry--drag {
 	opacity: 0 !important;
 }
@@ -398,9 +397,7 @@ $entry-padding: 5px;
 
 .entry-avatar {
 	flex: auto 0 0;
-	margin: 0 -5px 0 0;
-	max-height: 16px;
-	max-width: 16px;
+	margin: 0;
 
 	.entry--squash &,
 	.entry--fixup &,
@@ -409,7 +406,6 @@ $entry-padding: 5px;
 	}
 }
 
-.entry-author,
 .entry-date,
 .entry-sha {
 	flex: auto 0 0;

src/webviews/apps/rebase/rebase.ts

@@ -16,7 +16,10 @@ import {
 	UpdateSelectionCommandType,
 } from '../../rebase/protocol';
 import { App } from '../shared/appBase';
+import type { AvatarItem } from '../shared/components/avatars/avatar-item';
 import { DOM } from '../shared/dom';
+import '../shared/components/avatars/avatar-item';
+import '../shared/components/avatars/avatar-stack';
 
 const rebaseActions = ['pick', 'reword', 'edit', 'squash', 'fixup', 'drop'];
 const rebaseActionsMap = new Map<string, RebaseEntryAction>([
@@ -310,7 +313,9 @@ class RebaseEditor extends App {
 	}
 
 	private setSelectedEntry(sha: string, focusSelect: boolean = false) {
-		document.querySelectorAll<HTMLLIElement>(`${focusSelect ? 'select' : 'li'}[data-sha="${sha}"]`)[0]?.focus();
+		window.requestAnimationFrame(() => {
+			document.querySelectorAll<HTMLLIElement>(`${focusSelect ? 'select' : 'li'}[data-sha="${sha}"]`)[0]?.focus();
+		});
 	}
 
 	protected override onMessageReceived(e: MessageEvent) {
@@ -386,7 +391,6 @@ class RebaseEditor extends App {
 		let squashToHere = false;
 		let tabIndex = 0;
 
-		const $entries = document.createDocumentFragment();
 		for (const entry of state.entries) {
 			squashToHere = false;
 			if (entry.action === 'squash' || entry.action === 'fixup') {
@@ -401,9 +405,9 @@ class RebaseEditor extends App {
 			[$el, tabIndex] = this.createEntry(entry, state, ++tabIndex, squashToHere);
 
 			if (state.ascending) {
-				$entries.prepend($el);
+				$container.prepend($el);
 			} else {
-				$entries.append($el);
+				$container.append($el);
 			}
 		}
 
@@ -422,9 +426,9 @@ class RebaseEditor extends App {
 					false,
 				);
 				if (state.ascending) {
-					$entries.prepend($el);
+					$container.prepend($el);
 				} else {
-					$entries.appendChild($el);
+					$container.appendChild($el);
 				}
 				$container.classList.add('entries--base');
 			}
@@ -436,8 +440,6 @@ class RebaseEditor extends App {
 			($checkbox as HTMLInputElement).checked = state.ascending;
 		}
 
-		$container.appendChild($entries);
-
 		this.setSelectedEntry(focusRef ?? state.entries[0].sha, focusSelect);
 	}
 
@@ -448,10 +450,18 @@ class RebaseEditor extends App {
 		squashToHere: boolean,
 	): [HTMLLIElement, number] {
 		const $entry = document.createElement('li');
-		$entry.classList.add('entry', `entry--${entry.action ?? 'base'}`);
+		const action: string = entry.action ?? 'base';
+		$entry.classList.add('entry', `entry--${action}`);
 		$entry.classList.toggle('entry--squash-to', squashToHere);
 		$entry.dataset.sha = entry.sha;
 
+		let $content: HTMLElement = $entry;
+		if (action === 'base') {
+			$content = document.createElement('div');
+			$content.classList.add('entry-blocked');
+			$entry.appendChild($content);
+		}
+
 		if (entry.action != null) {
 			$entry.tabIndex = 0;
 
@@ -490,22 +500,38 @@ class RebaseEditor extends App {
 		const message = commit?.message.trim() ?? entry.message.trim();
 		$message.textContent = message.replace(/\n+(?:\s+\n+)?/g, ' | ');
 		$message.title = message;
-		$entry.appendChild($message);
+		$content.appendChild($message);
 
 		if (commit != null) {
 			if (commit.author) {
 				const author = state.authors[commit.author];
-				if (author?.avatarUrl.length) {
-					const $avatar = document.createElement('img');
-					$avatar.classList.add('entry-avatar');
-					$avatar.src = author.avatarUrl;
-					$entry.appendChild($avatar);
-				}
+				const committer = state.authors[commit.committer];
+				if (author?.avatarUrl != null || committer?.avatarUrl != null) {
+					const $avatarStack = document.createElement('avatar-stack');
+					$avatarStack.classList.add('entry-avatar');
+
+					const hasAuthor = author?.avatarUrl.length;
+					const hasCommitter = author !== committer && committer?.avatarUrl.length;
+					if (hasAuthor) {
+						const $avatar = document.createElement('avatar-item') as AvatarItem;
+						$avatar.media = author.avatarUrl;
+						$avatar.ariaLabel = $avatar.title = hasCommitter
+							? `Authored by: ${author.author}`
+							: author.author;
+						$avatarStack.appendChild($avatar);
+					}
+
+					if (hasCommitter) {
+						const $avatar = document.createElement('avatar-item') as AvatarItem;
+						$avatar.media = committer.avatarUrl;
+						$avatar.ariaLabel = $avatar.title = hasAuthor
+							? `Committed by: ${committer.author}`
+							: committer.author;
+						$avatarStack.appendChild($avatar);
+					}
 
-				const $author = document.createElement('span');
-				$author.classList.add('entry-author');
-				$author.textContent = commit.author;
-				$entry.appendChild($author);
+					$entry.appendChild($avatarStack);
+				}
 			}
 
 			if (commit.dateFromNow) {
@@ -521,7 +547,7 @@ class RebaseEditor extends App {
 		$sha.classList.add('entry-sha', 'icon--commit');
 		$sha.href = state.commands.commit.replace(this.commitTokenRegex, commit?.sha ?? entry.sha);
 		$sha.textContent = entry.sha.substr(0, 7);
-		$entry.appendChild($sha);
+		$content.appendChild($sha);
 
 		return [$entry, tabIndex];
 	}

src/webviews/apps/shared/components/avatars/avatar-item.ts

@@ -0,0 +1,54 @@
+import { attr, css, customElement, FASTElement, html } from '@microsoft/fast-element';
+import { focusOutline } from '../../../shared/components/styles/a11y';
+import { elementBase } from '../../../shared/components/styles/base';
+
+const template = html<AvatarItem>`<template role="img" tabindex="${x => x.tabIndex ?? '0'}">
+	<slot></slot>
+</template>`;
+
+const styles = css`
+	${elementBase}
+
+	:host {
+		display: inline-flex;
+		justify-content: center;
+		align-items: center;
+		width: var(--avatar-size, 2.4rem);
+		aspect-ratio: 1 / 1;
+		border-radius: 50%;
+		border: 1px solid var(--color-background);
+		background-color: var(--avatar-bg);
+		background-position: center;
+		background-repeat: no-repeat;
+		background-size: cover;
+		transition: all ease 200ms;
+		font-size: calc(var(--avatar-size) * 0.42);
+	}
+
+	:host(:hover) {
+		transform: translateY(-0.2em);
+	}
+
+	:host(:focus-visible) {
+		${focusOutline}
+	}
+`;
+
+@customElement({ name: 'avatar-item', template: template, styles: styles })
+export class AvatarItem extends FASTElement {
+	@attr
+	media = '';
+
+	@attr({ mode: 'boolean' })
+	static = false;
+
+	override attributeChangedCallback(name: string, oldValue: string, newValue: string): void {
+		super.attributeChangedCallback(name, oldValue, newValue);
+
+		if (name !== 'media' || oldValue === newValue) {
+			return;
+		}
+
+		this.style.backgroundImage = `url(${this.media})`;
+	}
+}

src/webviews/apps/shared/components/avatars/avatar-stack.ts

@@ -0,0 +1,55 @@
+import { css, customElement, FASTElement, html, observable, slotted } from '@microsoft/fast-element';
+import { elementBase } from '../../../shared/components/styles/base';
+import { nodeTypeFilter } from '../helpers/slots';
+
+const template = html<AvatarStack>`<template>
+	<slot ${slotted({ property: 'avatarNodes', filter: nodeTypeFilter(Node.ELEMENT_NODE) })}></slot>
+</template>`;
+
+const styles = css`
+	${elementBase}
+
+	:host {
+		display: inline-flex;
+		flex-direction: row;
+		justify-content: center;
+		align-items: center;
+	}
+
+	slot::slotted(*:not(:first-child)) {
+		margin-left: calc(var(--avatar-size, 2.4rem) * -0.2);
+	}
+
+	:host(:focus-within) slot::slotted(*),
+	:host(:hover) slot::slotted(*) {
+		opacity: 0.5;
+	}
+
+	:host(:focus-within) slot::slotted(*:focus),
+	:host(:hover) slot::slotted(*:hover) {
+		opacity: 1;
+		z-index: var(--avatar-selected-zindex, 1) !important;
+	}
+`;
+
+@customElement({ name: 'avatar-stack', template: template, styles: styles })
+export class AvatarStack extends FASTElement {
+	zindex = 1;
+
+	@observable
+	avatarNodes?: HTMLElement[];
+
+	avatarNodesChanged() {
+		if (this.avatarNodes == null) return;
+
+		const length = this.avatarNodes.length;
+		if (length !== this.zindex - 1) {
+			this.zindex = length + 1;
+			this.style.setProperty('--avatar-selected-zindex', this.zindex.toString());
+		}
+
+		this.avatarNodes.forEach((el, i) => {
+			el.style.zIndex = (length - i).toString();
+		});
+	}
+}

src/webviews/apps/shared/components/helpers/slots.ts

@@ -0,0 +1,7 @@
+export function hasNodes(...nodes: Array<Node[] | undefined>) {
+	return nodes.some(nodes => (nodes?.length ?? 0) > 0);
+}
+
+export function nodeTypeFilter(nodeType: Node['nodeType']) {
+	return (node: Node) => node.nodeType === nodeType;
+}

src/webviews/apps/shared/components/styles/a11y.ts

@@ -0,0 +1,19 @@
+import { css, cssPartial } from '@microsoft/fast-element';
+
+export const srOnly = css`
+	.sr-only,
+	.sr-only-focusable:not(:active):not(:focus) {
+		clip: rect(0 0 0 0);
+		clip-path: inset(50%);
+		width: 1px;
+		height: 1px;
+		overflow: hidden;
+		position: absolute;
+		white-space: nowrap;
+	}
+`;
+
+export const focusOutline = cssPartial`
+	outline: 1px solid var(--focus-color);
+	outline-offset: -1px;
+`;

src/webviews/apps/shared/components/styles/base.ts

@@ -0,0 +1,16 @@
+import { css } from '@microsoft/fast-element';
+
+export const elementBase = css`
+	:host {
+		box-sizing: border-box;
+	}
+	:host *,
+	:host *::before,
+	:host *::after {
+		box-sizing: inherit;
+	}
+
+	[hidden] {
+		display: none !important;
+	}
+`;

src/webviews/rebase/protocol.ts

@@ -33,6 +33,7 @@ export interface Author {
 export interface Commit {
 	readonly sha: string;
 	readonly author: string;
+	readonly committer: string;
 	// readonly avatarUrl: string;
 	readonly date: string;
 	readonly dateFromNow: string;

src/webviews/rebase/rebaseEditor.ts

@@ -602,6 +602,7 @@ export class RebaseEditorProvider implements CustomTextEditorProvider, Disposabl
 			entry.commit = {
 				sha: commit.sha,
 				author: commit.author.name,
+				committer: commit.committer.name,
 				date: commit.formatDate(defaultDateFormat),
 				dateFromNow: commit.formatDateFromNow(),
 				message: emojify(commit.message ?? commit.summary),
@@ -618,6 +619,7 @@ export class RebaseEditorProvider implements CustomTextEditorProvider, Disposabl
 								? {
 										sha: ontoCommit.sha,
 										author: ontoCommit.author.name,
+										committer: ontoCommit.committer.name,
 										date: ontoCommit.formatDate(defaultDateFormat),
 										dateFromNow: ontoCommit.formatDateFromNow(),
 										message: emojify(ontoCommit.message || 'root'),
@@ -659,6 +661,14 @@ export class RebaseEditorProvider implements CustomTextEditorProvider, Disposabl
 						email: c.author.email,
 					});
 				}
+				if (!context.authors.has(c.committer.name)) {
+					const avatarUri = await c.committer.getAvatarUri(c);
+					context.authors.set(c.committer.name, {
+						author: c.committer.name,
+						avatarUrl: avatarUri.toString(true),
+						email: c.committer.email,
+					});
+				}
 			}
 		}
 	}