From f58c3ab3c22b0dc6ff6d2cc71b5a90458f95d670 Mon Sep 17 00:00:00 2001 From: Keith Daulton Date: Tue, 8 Nov 2022 16:21:35 -0500 Subject: [PATCH] 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 --- CHANGELOG.md | 1 + src/webviews/apps/rebase/rebase.scss | 136 ++++++++++----------- src/webviews/apps/rebase/rebase.ts | 68 +++++++---- .../apps/shared/components/avatars/avatar-item.ts | 54 ++++++++ .../apps/shared/components/avatars/avatar-stack.ts | 55 +++++++++ .../apps/shared/components/helpers/slots.ts | 7 ++ src/webviews/apps/shared/components/styles/a11y.ts | 19 +++ src/webviews/apps/shared/components/styles/base.ts | 16 +++ src/webviews/rebase/protocol.ts | 1 + src/webviews/rebase/rebaseEditor.ts | 10 ++ 10 files changed, 276 insertions(+), 91 deletions(-) create mode 100644 src/webviews/apps/shared/components/avatars/avatar-item.ts create mode 100644 src/webviews/apps/shared/components/avatars/avatar-stack.ts create mode 100644 src/webviews/apps/shared/components/helpers/slots.ts create mode 100644 src/webviews/apps/shared/components/styles/a11y.ts create mode 100644 src/webviews/apps/shared/components/styles/base.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index cb11d26..b2630c8 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/webviews/apps/rebase/rebase.scss b/src/webviews/apps/rebase/rebase.scss index 80c5206..89b44c1 100644 --- a/src/webviews/apps/rebase/rebase.scss +++ b/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; diff --git a/src/webviews/apps/rebase/rebase.ts b/src/webviews/apps/rebase/rebase.ts index 067d036..4790483 100644 --- a/src/webviews/apps/rebase/rebase.ts +++ b/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([ @@ -310,7 +313,9 @@ class RebaseEditor extends App { } private setSelectedEntry(sha: string, focusSelect: boolean = false) { - document.querySelectorAll(`${focusSelect ? 'select' : 'li'}[data-sha="${sha}"]`)[0]?.focus(); + window.requestAnimationFrame(() => { + document.querySelectorAll(`${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]; } diff --git a/src/webviews/apps/shared/components/avatars/avatar-item.ts b/src/webviews/apps/shared/components/avatars/avatar-item.ts new file mode 100644 index 0000000..c651804 --- /dev/null +++ b/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``; + +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})`; + } +} diff --git a/src/webviews/apps/shared/components/avatars/avatar-stack.ts b/src/webviews/apps/shared/components/avatars/avatar-stack.ts new file mode 100644 index 0000000..2111310 --- /dev/null +++ b/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``; + +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(); + }); + } +} diff --git a/src/webviews/apps/shared/components/helpers/slots.ts b/src/webviews/apps/shared/components/helpers/slots.ts new file mode 100644 index 0000000..5216c66 --- /dev/null +++ b/src/webviews/apps/shared/components/helpers/slots.ts @@ -0,0 +1,7 @@ +export function hasNodes(...nodes: Array) { + return nodes.some(nodes => (nodes?.length ?? 0) > 0); +} + +export function nodeTypeFilter(nodeType: Node['nodeType']) { + return (node: Node) => node.nodeType === nodeType; +} diff --git a/src/webviews/apps/shared/components/styles/a11y.ts b/src/webviews/apps/shared/components/styles/a11y.ts new file mode 100644 index 0000000..2542ed1 --- /dev/null +++ b/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; +`; diff --git a/src/webviews/apps/shared/components/styles/base.ts b/src/webviews/apps/shared/components/styles/base.ts new file mode 100644 index 0000000..a357741 --- /dev/null +++ b/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; + } +`; diff --git a/src/webviews/rebase/protocol.ts b/src/webviews/rebase/protocol.ts index 81d4b50..1bcb237 100644 --- a/src/webviews/rebase/protocol.ts +++ b/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; diff --git a/src/webviews/rebase/rebaseEditor.ts b/src/webviews/rebase/rebaseEditor.ts index 3f607cd..43a4787 100644 --- a/src/webviews/rebase/rebaseEditor.ts +++ b/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, + }); + } } } }