From 31f1cb8607ab58ba39573b82df9719d4379f66e9 Mon Sep 17 00:00:00 2001 From: Mattias Granlund Date: Thu, 16 May 2024 00:59:33 +0200 Subject: [PATCH] Refactor commits section of branch lanes Fix remote lane line issue Drop unused file Fix criteria for showing push button Fix alignment of avatar for local commit Fix criteria for showing pus/rebase button Show tooltip on shadow commit marker hover Add missing type param to CommitCard Fix lint Remove extraneous parameter Drop a couple of console.log() calls --- app/src/lib/components/Avatar.svelte | 37 ++++ app/src/lib/components/BaseBranch.svelte | 9 +- app/src/lib/components/BranchCard.svelte | 7 +- app/src/lib/components/BranchCommits.svelte | 23 --- app/src/lib/components/BranchFooter.svelte | 65 +++++++ app/src/lib/components/BranchHeader.svelte | 47 ++++- app/src/lib/components/BranchLane.svelte | 38 ++-- .../lib/components/BranchLanePopupMenu.svelte | 3 +- app/src/lib/components/CommitCard.svelte | 116 ++++++++----- app/src/lib/components/CommitDialog.svelte | 2 +- app/src/lib/components/CommitLines.svelte | 55 ++++++ app/src/lib/components/CommitList.svelte | 164 ++++++++++++------ .../lib/components/CommitListFooter.svelte | 149 ---------------- .../lib/components/CommitListHeader.svelte | 64 ------- app/src/lib/components/CommitListItem.svelte | 50 +----- app/src/lib/components/LocalLine.svelte | 90 ++++++++++ .../lib/components/PullRequestButton.svelte | 68 ++++++++ app/src/lib/components/PullRequestCard.svelte | 3 +- app/src/lib/components/PushButton.svelte | 142 +++++++-------- .../lib/components/RemoteBranchPreview.svelte | 2 +- app/src/lib/components/RemoteLine.svelte | 123 +++++++++++++ app/src/lib/components/ShadowLine.svelte | 102 +++++++++++ app/src/lib/github/service.ts | 6 +- app/src/lib/vbranches/contexts.ts | 2 - app/src/lib/vbranches/types.ts | 38 ++-- app/src/lib/vbranches/virtualBranch.ts | 8 +- app/src/styles/tokens.css | 1 + 27 files changed, 903 insertions(+), 511 deletions(-) create mode 100644 app/src/lib/components/Avatar.svelte delete mode 100644 app/src/lib/components/BranchCommits.svelte create mode 100644 app/src/lib/components/BranchFooter.svelte create mode 100644 app/src/lib/components/CommitLines.svelte delete mode 100644 app/src/lib/components/CommitListFooter.svelte delete mode 100644 app/src/lib/components/CommitListHeader.svelte create mode 100644 app/src/lib/components/LocalLine.svelte create mode 100644 app/src/lib/components/PullRequestButton.svelte create mode 100644 app/src/lib/components/RemoteLine.svelte create mode 100644 app/src/lib/components/ShadowLine.svelte diff --git a/app/src/lib/components/Avatar.svelte b/app/src/lib/components/Avatar.svelte new file mode 100644 index 000000000..9943e469b --- /dev/null +++ b/app/src/lib/components/Avatar.svelte @@ -0,0 +1,37 @@ + + +Gravatar for {author.email} + + diff --git a/app/src/lib/components/BaseBranch.svelte b/app/src/lib/components/BaseBranch.svelte index eb6e3fb70..21cdfb883 100644 --- a/app/src/lib/components/BaseBranch.svelte +++ b/app/src/lib/components/BaseBranch.svelte @@ -48,7 +48,12 @@
{#each base.upstreamCommits as commit} - + {/each}
@@ -62,7 +67,7 @@ Local {#each base.recentCommits as commit} - + {/each} diff --git a/app/src/lib/components/BranchCard.svelte b/app/src/lib/components/BranchCard.svelte index d041f9b79..df9b456e7 100644 --- a/app/src/lib/components/BranchCard.svelte +++ b/app/src/lib/components/BranchCard.svelte @@ -1,8 +1,9 @@ - -{#if $unknownCommits && $unknownCommits.length > 0} - -{/if} - - - diff --git a/app/src/lib/components/BranchFooter.svelte b/app/src/lib/components/BranchFooter.svelte new file mode 100644 index 000000000..a1cabae7e --- /dev/null +++ b/app/src/lib/components/BranchFooter.svelte @@ -0,0 +1,65 @@ + + +{#if !isUnapplied} +
+ {#if !isPushed} + {#if $prompt} + + {/if} + { + try { + if (e.detail.action == BranchAction.Push) { + isLoading = true; + await branchController.pushBranch($branch.id, $branch.requiresForce); + isLoading = false; + } else if (e.detail.action == BranchAction.Rebase) { + isLoading = true; + await branchController.mergeUpstream($branch.id); + isLoading = false; + } + } catch (e) { + console.error(e); + } + }} + /> + {:else} + Branch origin/second-branch is up to date with the remote. + {/if} +
+{/if} + + diff --git a/app/src/lib/components/BranchHeader.svelte b/app/src/lib/components/BranchHeader.svelte index c01a74dcb..e07603c72 100644 --- a/app/src/lib/components/BranchHeader.svelte +++ b/app/src/lib/components/BranchHeader.svelte @@ -2,16 +2,21 @@ import ActiveBranchStatus from './ActiveBranchStatus.svelte'; import BranchLabel from './BranchLabel.svelte'; import BranchLanePopupMenu from './BranchLanePopupMenu.svelte'; + import PullRequestButton from './PullRequestButton.svelte'; import Tag from './Tag.svelte'; import { Project } from '$lib/backend/projects'; + import { BranchService } from '$lib/branches/service'; import { clickOutside } from '$lib/clickOutside'; import Button from '$lib/components/Button.svelte'; import Icon from '$lib/components/Icon.svelte'; + import { GitHubService } from '$lib/github/service'; import { showError } from '$lib/notifications/toasts'; import { normalizeBranchName } from '$lib/utils/branch'; import { getContext, getContextStore } from '$lib/utils/context'; import { BranchController } from '$lib/vbranches/branchController'; - import { Branch } from '$lib/vbranches/types'; + import { BaseBranch, Branch } from '$lib/vbranches/types'; + import toast from 'svelte-french-toast'; + import type { PullRequest } from '$lib/github/types'; import type { Persisted } from '$lib/persisted/persisted'; import { goto } from '$app/navigation'; @@ -20,16 +25,21 @@ export let isLaneCollapsed: Persisted; const branchController = getContext(BranchController); + const githubService = getContext(GitHubService); const branchStore = getContextStore(Branch); const project = getContext(Project); + const branchService = getContext(BranchService); + const baseBranch = getContextStore(BaseBranch); $: branch = $branchStore; + $: hasPullRequest = branch.upstreamName && githubService.hasPr(branch.upstreamName); let meatballButton: HTMLDivElement; let visible = false; let isApplying = false; let isDeleting = false; let branchName = branch?.upstreamName || normalizeBranchName($branchStore.name); + let isLoading: boolean; function handleBranchNameChange(title: string) { if (title == '') return; @@ -49,6 +59,34 @@ $: hasIntegratedCommits = branch.commits?.some((b) => b.isIntegrated); let headerInfoHeight = 0; + + interface CreatePrOpts { + draft: boolean; + } + + const defaultPrOpts: CreatePrOpts = { + draft: true + }; + + async function createPr(createPrOpts: CreatePrOpts): Promise { + const opts = { ...defaultPrOpts, ...createPrOpts }; + if (!githubService.isEnabled) { + toast.error('Cannot create PR without GitHub credentials'); + return; + } + + if (!$baseBranch?.shortName) { + toast.error('Cannot create PR without base branch'); + return; + } + + isLoading = true; + try { + return await branchService.createPr(branch, $baseBranch.shortName, opts.draft); + } finally { + isLoading = false; + } + } {#if $isLaneCollapsed} @@ -215,6 +253,12 @@ {:else}
+ {#if !hasPullRequest} + await createPr({ draft: e.detail.action == 'draft' })} + loading={isLoading} + /> + {/if} - - diff --git a/app/src/lib/components/CommitListItem.svelte b/app/src/lib/components/CommitListItem.svelte index 164222759..1a7b69b58 100644 --- a/app/src/lib/components/CommitListItem.svelte +++ b/app/src/lib/components/CommitListItem.svelte @@ -1,5 +1,4 @@
- {#if isChained} -
- {/if} -
- +
diff --git a/app/src/lib/components/PullRequestButton.svelte b/app/src/lib/components/PullRequestButton.svelte new file mode 100644 index 000000000..e1f70c97a --- /dev/null +++ b/app/src/lib/components/PullRequestButton.svelte @@ -0,0 +1,68 @@ + + + { + dispatch('click', { action: $action }); + }} +> + {$selection$?.label} + { + // TODO: Refactor to use generics if/when that works with Svelte + switch (e.detail?.id) { + case Action.Create: + $action = Action.Create; + break; + case Action.Draft: + $action = Action.Draft; + break; + default: + toasts.error('Unknown merge method'); + } + dropDown.close(); + }} + > + + + + + + diff --git a/app/src/lib/components/PullRequestCard.svelte b/app/src/lib/components/PullRequestCard.svelte index 3576d4692..af6050cd9 100644 --- a/app/src/lib/components/PullRequestCard.svelte +++ b/app/src/lib/components/PullRequestCard.svelte @@ -255,7 +255,7 @@ }} />
-
+
PR #{pr.number}: {pr.title}
@@ -350,6 +350,7 @@ .pr-card { position: relative; padding: var(--size-14); + margin-bottom: var(--size-8); } .pr-title { diff --git a/app/src/lib/components/PushButton.svelte b/app/src/lib/components/PushButton.svelte index 7cff398e9..e6ab10a46 100644 --- a/app/src/lib/components/PushButton.svelte +++ b/app/src/lib/components/PushButton.svelte @@ -1,14 +1,12 @@ -{#if (isPr || commits.length === 0) && !isPushed} - -{:else if !isPr} - { - dispatch('trigger', { action }); + { + dispatch('trigger', { action }); + }} +> + {$selection$?.label} + { + // TODO: Refactor to use generics if/when that works with Svelte + switch (e.detail?.id) { + case BranchAction.Push: + $preferredAction = BranchAction.Push; + break; + case BranchAction.Rebase: + $preferredAction = BranchAction.Rebase; + break; + default: + toasts.error('Uknown branch action'); + } + dropDown.close(); }} > - {$selection$?.label} - { - // TODO: Refactor to use generics if/when that works with Svelte - switch (e.detail?.id) { - case BranchAction.Push: - $preferredAction = BranchAction.Push; - break; - case BranchAction.Pr: - $preferredAction = BranchAction.Pr; - break; - case BranchAction.DraftPr: - $preferredAction = BranchAction.DraftPr; - break; - default: - toasts.error('Uknown branch action'); - } - dropDown.close(); - }} - > - + + {#if !isPushed} + {/if} + {#if !branch.requiresForce} + {/if} + {#if canBeRebased} - - - -{/if} + {/if} + + + diff --git a/app/src/lib/components/RemoteBranchPreview.svelte b/app/src/lib/components/RemoteBranchPreview.svelte index 6cb929adc..0faf8a8dd 100644 --- a/app/src/lib/components/RemoteBranchPreview.svelte +++ b/app/src/lib/components/RemoteBranchPreview.svelte @@ -68,7 +68,7 @@ {#if branchData.commits && branchData.commits.length > 0}
{#each branchData.commits as commit (commit.id)} - + {/each}
{/if} diff --git a/app/src/lib/components/RemoteLine.svelte b/app/src/lib/components/RemoteLine.svelte new file mode 100644 index 000000000..528c78e10 --- /dev/null +++ b/app/src/lib/components/RemoteLine.svelte @@ -0,0 +1,123 @@ + + +
+ {#if base} +
+ {#if root} +
+ {/if} +
+ + + +
+ {:else} + {#if line} +
+ {/if} + {#if hasRoot} +
+ {/if} + {#if commit} + {@const author = commit.author} +
+ +
+ {/if} + {/if} +
+ + diff --git a/app/src/lib/components/ShadowLine.svelte b/app/src/lib/components/ShadowLine.svelte new file mode 100644 index 000000000..df14049e4 --- /dev/null +++ b/app/src/lib/components/ShadowLine.svelte @@ -0,0 +1,102 @@ + + +
+ {#if line} + {#if upstreamLine} +
+ {/if} +
+ {:else if upstreamLine} +
+ {/if} + {#if localCommit} +
+ {/if} + {#if remoteCommit} + {@const author = remoteCommit.author} + Gravatar for {author.email} + {/if} +
+ + diff --git a/app/src/lib/github/service.ts b/app/src/lib/github/service.ts index 4cb93cea1..1fec38708 100644 --- a/app/src/lib/github/service.ts +++ b/app/src/lib/github/service.ts @@ -151,7 +151,7 @@ export class GitHubService { if (!skipCache) { const cachedRsp = lscache.get(key); - if (cachedRsp) subscriber.next(cachedRsp.data.map(ghResponseToInstance)); + if (cachedRsp?.data) subscriber.next(cachedRsp.data.map(ghResponseToInstance)); } try { @@ -239,6 +239,10 @@ export class GitHubService { return this.prs$.pipe(map((prs) => prs.find((pr) => pr.targetBranch == branch))); } + hasPr(branch: string): boolean { + return !!this.prs$.value.find((pr) => pr.targetBranch == branch); + } + /* TODO: Figure out a way to cleanup old behavior subjects */ getState(branchId: string) { let state$ = this.stateMap.get(branchId); diff --git a/app/src/lib/vbranches/contexts.ts b/app/src/lib/vbranches/contexts.ts index c395241bb..01e8d836b 100644 --- a/app/src/lib/vbranches/contexts.ts +++ b/app/src/lib/vbranches/contexts.ts @@ -8,8 +8,6 @@ export const [getLocalCommits, createLocalContextStore] = buildContextStore('localCommits'); export const [getRemoteCommits, createRemoteContextStore] = buildContextStore('remoteCommits'); -export const [getIntegratedCommits, createIntegratedContextStore] = - buildContextStore('integratedCommits'); export const [getUnknownCommits, createUnknownContextStore] = buildContextStore('unknownCommits'); export const [getSelectedFiles, createSelectedFiles] = buildContextStore< diff --git a/app/src/lib/vbranches/types.ts b/app/src/lib/vbranches/types.ts index 2804368bc..8f7bed67c 100644 --- a/app/src/lib/vbranches/types.ts +++ b/app/src/lib/vbranches/types.ts @@ -131,10 +131,6 @@ export class Branch { get remoteCommits() { return this.commits.filter((c) => c.status == 'remote'); } - - get integratedCommits() { - return this.commits.filter((c) => c.status == 'integrated'); - } } // Used for dependency injection @@ -148,7 +144,7 @@ export type ComponentColor = | 'error' | 'warning' | 'purple'; -export type CommitStatus = 'local' | 'remote' | 'integrated' | 'upstream'; +export type CommitStatus = 'local' | 'remote' | 'upstream'; export class Commit { id!: string; @@ -164,6 +160,7 @@ export class Commit { branchId!: string; changeId!: string; isSigned!: boolean; + relatedTo?: RemoteCommit; parent?: Commit; children?: Commit[]; @@ -172,14 +169,9 @@ export class Commit { return !this.isRemote && !this.isIntegrated; } - get status() { - if (!this.isIntegrated && !this.isRemote) { - return 'local'; - } else if (!this.isIntegrated && this.isRemote) { - return 'remote'; - } else if (this.isIntegrated) { - return 'integrated'; - } + get status(): CommitStatus { + if (this.isRemote) return 'remote'; + return 'local'; } get descriptionTitle(): string | undefined { @@ -195,6 +187,10 @@ export class Commit { } } +export function isLocalCommit(obj: any): obj is Commit { + return obj instanceof Commit; +} + export class RemoteCommit { id!: string; author!: Author; @@ -218,6 +214,14 @@ export class RemoteCommit { get descriptionBody(): string | undefined { return splitMessage(this.description).description || undefined; } + + get status(): CommitStatus { + return 'upstream'; + } +} + +export function isRemoteCommit(obj: any): obj is RemoteCommit { + return obj instanceof RemoteCommit; } export type AnyCommit = Commit | RemoteCommit; @@ -227,6 +231,14 @@ export const REMOTE_COMMITS = Symbol('RemoteCommits'); export const INTEGRATED_COMMITS = Symbol('IntegratedCommits'); export const UNKNOWN_COMMITS = Symbol('UnknownCommits'); +export function commitCompare(left: AnyCommit, right: AnyCommit): boolean { + if (left.id == right.id) return true; + if (left.description != right.description) return false; + if (left.author.name != right.author.name) return false; + if (left.author.email != right.author.email) return false; + return true; +} + export class RemoteHunk { diff!: string; hash?: string; diff --git a/app/src/lib/vbranches/virtualBranch.ts b/app/src/lib/vbranches/virtualBranch.ts index 4f7048b1c..09766cb3e 100644 --- a/app/src/lib/vbranches/virtualBranch.ts +++ b/app/src/lib/vbranches/virtualBranch.ts @@ -58,11 +58,13 @@ export class VirtualBranchService { const commits = branch.commits; for (let j = 0; j < commits.length; j++) { const commit = commits[j]; - if (j != 0) { - commit.parent = commits[j - 1]; + if (j == 0) { + commit.children = []; + } else { + commit.children = [commits[j - 1]]; } if (j != commits.length - 1) { - commit.children = [commits[j + 1]]; + commit.parent = commits[j + 1]; } } } diff --git a/app/src/styles/tokens.css b/app/src/styles/tokens.css index 286b1354f..e026cae94 100644 --- a/app/src/styles/tokens.css +++ b/app/src/styles/tokens.css @@ -189,6 +189,7 @@ --radius-l: 0.75rem; --radius-m: 0.375rem; --radius-s: 0.25rem; + --size-1: 0.06125rem; --size-2: 0.125rem; --size-4: 0.25rem; --size-6: 0.375rem;