Merge pull request #11434 from desktop/add-branch-context-menu

Add branch context menu

app/src/lib/stores/app-store.ts

@@ -3346,21 +3346,29 @@ export class AppStore extends TypedBaseStore<IAppState> {
   public async _deleteBranch(
     repository: Repository,
     branch: Branch,
-    includeRemote?: boolean
+    includeUpstream?: boolean
   ): Promise<void> {
     return this.withAuthenticatingUser(repository, async (r, account) => {
       const gitStore = this.gitStoreCache.get(r)
 
       // If solely a remote branch, there is no need to checkout a branch.
       if (branch.type === BranchType.Remote) {
-        await gitStore.performFailableOperation(() => {
-          // Note: deleting a remote branch implementation not needed, yet.
-          return Promise.resolve()
-        })
+        const { remoteName, tip, nameWithoutRemote } = branch
+        if (remoteName === null) {
+          // This is based on the branches ref. It should not be null for a
+          // remote branch
+          throw new Error(
+            `Could not determine remote name from: ${branch.ref}.`
+          )
+        }
+
+        await gitStore.performFailableOperation(() =>
+          deleteRemoteBranch(r, account, remoteName, nameWithoutRemote)
+        )
 
         // We log the remote branch's sha so that the user can recover it.
         log.info(
-          `Deleted branch ${branch.upstreamWithoutRemote} (was ${branch.tip.sha})`
+          `Deleted branch ${branch.upstreamWithoutRemote} (was ${tip.sha})`
         )
 
         return this._refreshRepository(r)
@@ -3381,7 +3389,7 @@ export class AppStore extends TypedBaseStore<IAppState> {
           repository,
           branch,
           account,
-          includeRemote
+          includeUpstream
         )
       })
 
@@ -3390,7 +3398,7 @@ export class AppStore extends TypedBaseStore<IAppState> {
   }
 
   /**
-   * Deletes the local branch. If the parameter `includeRemote` is true, the
+   * Deletes the local branch. If the parameter `includeUpstream` is true, the
    * upstream branch will be deleted also.
    */
   private async deleteLocalBranchAndUpstreamBranch(

app/src/lib/stores/helpers/branch-pruner.ts

@@ -10,8 +10,8 @@ import {
   getBranchCheckouts,
   getSymbolicRef,
   formatAsLocalRef,
-  deleteLocalBranch,
   getBranches,
+  deleteLocalBranch,
 } from '../../git'
 import { fatalError } from '../../fatal-error'
 import { RepositoryStateCache } from '../repository-state-cache'

app/src/models/branch.ts

@@ -85,14 +85,13 @@ export class Branch {
     }
 
     const pieces = this.ref.match(/^refs\/remotes\/(.*?)\/.*/)
-    if (!pieces || pieces.length === 2) {
+    if (!pieces || pieces.length !== 2) {
       // This shouldn't happen, the remote ref should always be prefixed
       // with refs/remotes
       throw new Error(`Remote branch ref has unexpected format: ${this.ref}`)
     }
     return pieces[1]
   }
-
   /**
    * The name of the branch's upstream without the remote prefix.
    */

app/src/models/popup.ts

@@ -19,6 +19,7 @@ import { ITextDiff, DiffSelection } from './diff'
 export enum PopupType {
   RenameBranch = 1,
   DeleteBranch,
+  DeleteRemoteBranch,
   ConfirmDiscardChanges,
   Preferences,
   MergeBranch,
@@ -76,6 +77,11 @@ export type Popup =
       branch: Branch
       existsOnRemote: boolean
     }
+  | {
+      type: PopupType.DeleteRemoteBranch
+      repository: Repository
+      branch: Branch
+    }
   | {
       type: PopupType.ConfirmDiscardChanges
       repository: Repository

app/src/ui/app.tsx

@@ -42,7 +42,7 @@ import { TitleBar, ZoomInfo, FullScreenInfo } from './window'
 import { RepositoriesList } from './repositories-list'
 import { RepositoryView } from './repository'
 import { RenameBranch } from './rename-branch'
-import { DeleteBranch } from './delete-branch'
+import { DeleteBranch, DeleteRemoteBranch } from './delete-branch'
 import { CloningRepositoryView } from './cloning-repository'
 import {
   Toolbar,
@@ -1299,6 +1299,17 @@ export class App extends React.Component<IAppProps, IAppState> {
             onDeleted={this.onBranchDeleted}
           />
         )
+      case PopupType.DeleteRemoteBranch:
+        return (
+          <DeleteRemoteBranch
+            key="delete-remote-branch"
+            dispatcher={this.props.dispatcher}
+            repository={popup.repository}
+            branch={popup.branch}
+            onDismissed={onPopupDismissedFn}
+            onDeleted={this.onBranchDeleted}
+          />
+        )
       case PopupType.ConfirmDiscardChanges:
         const showSetting =
           popup.showDiscardChangesSetting === undefined

app/src/ui/branches/branch-list-item.tsx

@@ -5,6 +5,8 @@ import { IMatches } from '../../lib/fuzzy-find'
 
 import { Octicon, OcticonSymbol } from '../octicons'
 import { HighlightText } from '../lib/highlight-text'
+import { showContextualMenu } from '../main-process-proxy'
+import { IMenuItem } from '../../lib/menu-item'
 
 interface IBranchListItemProps {
   /** The name of the branch */
@@ -18,10 +20,51 @@ interface IBranchListItemProps {
 
   /** The characters in the branch name to highlight */
   readonly matches: IMatches
+
+  /** Specifies whether the branch is local */
+  readonly isLocal: boolean
+
+  readonly onRenameBranch?: (branchName: string) => void
+
+  readonly onDeleteBranch?: (branchName: string) => void
 }
 
 /** The branch component. */
 export class BranchListItem extends React.Component<IBranchListItemProps, {}> {
+  private onContextMenu = (event: React.MouseEvent<HTMLDivElement>) => {
+    event.preventDefault()
+
+    /*
+      There are multiple instances in the application where a branch list item
+      is rendered. We only want to be able to rename or delete them on the
+      branch dropdown menu. Thus, other places simply will not provide these
+      methods, such as the merge and rebase logic.
+    */
+    const { onRenameBranch, onDeleteBranch, name, isLocal } = this.props
+    if (onRenameBranch === undefined && onDeleteBranch === undefined) {
+      return
+    }
+
+    const items: Array<IMenuItem> = []
+
+    if (onRenameBranch !== undefined) {
+      items.push({
+        label: 'Rename…',
+        action: () => onRenameBranch(name),
+        enabled: isLocal,
+      })
+    }
+
+    if (onDeleteBranch !== undefined) {
+      items.push({
+        label: 'Delete…',
+        action: () => onDeleteBranch(name),
+      })
+    }
+
+    showContextualMenu(items)
+  }
+
   public render() {
     const lastCommitDate = this.props.lastCommitDate
     const isCurrentBranch = this.props.isCurrentBranch
@@ -35,7 +78,7 @@ export class BranchListItem extends React.Component<IBranchListItemProps, {}> {
       ? lastCommitDate.toString()
       : ''
     return (
-      <div className="branches-list-item">
+      <div onContextMenu={this.onContextMenu} className="branches-list-item">
         <Octicon className="icon" symbol={icon} />
         <div className="name" title={name}>
           <HighlightText text={name} highlight={this.props.matches.title} />

app/src/ui/branches/branch-renderer.tsx

@@ -1,6 +1,6 @@
 import * as React from 'react'
 
-import { Branch } from '../../models/branch'
+import { Branch, BranchType } from '../../models/branch'
 
 import { IBranchListItem } from './group-branches'
 import { BranchListItem } from './branch-list-item'
@@ -9,7 +9,9 @@ import { IMatches } from '../../lib/fuzzy-find'
 export function renderDefaultBranch(
   item: IBranchListItem,
   matches: IMatches,
-  currentBranch: Branch | null
+  currentBranch: Branch | null,
+  onRenameBranch?: (branchName: string) => void,
+  onDeleteBranch?: (branchName: string) => void
 ): JSX.Element {
   const branch = item.branch
   const commit = branch.tip
@@ -18,8 +20,11 @@ export function renderDefaultBranch(
     <BranchListItem
       name={branch.name}
       isCurrentBranch={branch.name === currentBranchName}
+      isLocal={branch.type === BranchType.Local}
       lastCommitDate={commit ? commit.author.date : null}
       matches={matches}
+      onRenameBranch={onRenameBranch}
+      onDeleteBranch={onDeleteBranch}
     />
   )
 }

app/src/ui/branches/branches-container.tsx

@@ -5,7 +5,7 @@ import {
   Repository,
   isRepositoryWithGitHubRepository,
 } from '../../models/repository'
-import { Branch } from '../../models/branch'
+import { Branch, BranchType } from '../../models/branch'
 import { BranchesTab } from '../../models/branches-tab'
 import { PopupType } from '../../models/popup'
 
@@ -146,7 +146,13 @@ export class BranchesContainer extends React.Component<
   }
 
   private renderBranch = (item: IBranchListItem, matches: IMatches) => {
-    return renderDefaultBranch(item, matches, this.props.currentBranch)
+    return renderDefaultBranch(
+      item,
+      matches,
+      this.props.currentBranch,
+      this.onRenameBranch,
+      this.onDeleteBranch
+    )
   }
 
   private renderSelectedTab() {
@@ -261,4 +267,46 @@ export class BranchesContainer extends React.Component<
   ) => {
     this.setState({ selectedPullRequest })
   }
+
+  private getBranchWithName(branchName: string): Branch | undefined {
+    return this.props.allBranches.find(branch => branch.name === branchName)
+  }
+
+  private onRenameBranch = (branchName: string) => {
+    const branch = this.getBranchWithName(branchName)
+
+    if (branch === undefined) {
+      return
+    }
+
+    this.props.dispatcher.showPopup({
+      type: PopupType.RenameBranch,
+      repository: this.props.repository,
+      branch: branch,
+    })
+  }
+
+  private onDeleteBranch = (branchName: string) => {
+    const branch = this.getBranchWithName(branchName)
+
+    if (branch === undefined) {
+      return
+    }
+
+    if (branch.type === BranchType.Remote) {
+      this.props.dispatcher.showPopup({
+        type: PopupType.DeleteRemoteBranch,
+        repository: this.props.repository,
+        branch,
+      })
+      return
+    }
+
+    this.props.dispatcher.showPopup({
+      type: PopupType.DeleteBranch,
+      repository: this.props.repository,
+      branch,
+      existsOnRemote: branch.upstreamRemoteName !== null,
+    })
+  }
 }

app/src/ui/delete-branch/delete-branch-dialog.tsx

@@ -100,7 +100,7 @@ export class DeleteBranch extends React.Component<
 
     this.setState({ isDeleting: true })
 
-    await dispatcher.deleteBranch(
+    await dispatcher.deleteLocalBranch(
       repository,
       branch,
       this.state.includeRemoteBranch

app/src/ui/delete-branch/delete-pull-request-dialog.tsx

@@ -51,10 +51,9 @@ export class DeletePullRequest extends React.Component<IDeleteBranchProps, {}> {
   }
 
   private deleteBranch = () => {
-    this.props.dispatcher.deleteBranch(
+    this.props.dispatcher.deleteLocalBranch(
       this.props.repository,
-      this.props.branch,
-      false
+      this.props.branch
     )
 
     return this.props.onDismissed()

app/src/ui/delete-branch/delete-remote-branch-dialog.tsx

@@ -0,0 +1,71 @@
+import * as React from 'react'
+
+import { Dispatcher } from '../dispatcher'
+import { Repository } from '../../models/repository'
+import { Branch } from '../../models/branch'
+import { Dialog, DialogContent, DialogFooter } from '../dialog'
+import { Ref } from '../lib/ref'
+import { OkCancelButtonGroup } from '../dialog/ok-cancel-button-group'
+
+interface IDeleteRemoteBranchProps {
+  readonly dispatcher: Dispatcher
+  readonly repository: Repository
+  readonly branch: Branch
+  readonly onDismissed: () => void
+  readonly onDeleted: (repository: Repository) => void
+}
+interface IDeleteRemoteBranchState {
+  readonly isDeleting: boolean
+}
+export class DeleteRemoteBranch extends React.Component<
+  IDeleteRemoteBranchProps,
+  IDeleteRemoteBranchState
+> {
+  public constructor(props: IDeleteRemoteBranchProps) {
+    super(props)
+
+    this.state = {
+      isDeleting: false,
+    }
+  }
+
+  public render() {
+    return (
+      <Dialog
+        id="delete-branch"
+        title={__DARWIN__ ? 'Delete Remote Branch' : 'Delete remote branch'}
+        type="warning"
+        onSubmit={this.deleteBranch}
+        onDismissed={this.props.onDismissed}
+        disabled={this.state.isDeleting}
+        loading={this.state.isDeleting}
+      >
+        <DialogContent>
+          <p>
+            Delete remote branch <Ref>{this.props.branch.name}</Ref>?<br />
+            This action cannot be undone.
+          </p>
+
+          <p>
+            This branch does not exist locally. Deleting it may impact others
+            collaborating on this branch.
+          </p>
+        </DialogContent>
+        <DialogFooter>
+          <OkCancelButtonGroup destructive={true} okButtonText="Delete" />
+        </DialogFooter>
+      </Dialog>
+    )
+  }
+
+  private deleteBranch = async () => {
+    const { dispatcher, repository, branch } = this.props
+
+    this.setState({ isDeleting: true })
+
+    await dispatcher.deleteRemoteBranch(repository, branch)
+    this.props.onDeleted(repository)
+
+    this.props.onDismissed()
+  }
+}

app/src/ui/delete-branch/index.ts

@@ -1 +1,2 @@
 export { DeleteBranch } from './delete-branch-dialog'
+export { DeleteRemoteBranch } from './delete-remote-branch-dialog'

app/src/ui/dispatcher/dispatcher.ts

@@ -686,14 +686,24 @@ export class Dispatcher {
 
   /**
    * Delete the branch. This will delete both the local branch and the remote
-   * branch, and then check out the default branch.
+   * branch if includeUpstream is true, and then check out the default branch.
    */
-  public deleteBranch(
+  public deleteLocalBranch(
     repository: Repository,
     branch: Branch,
-    includeRemote: boolean
+    includeUpstream?: boolean
   ): Promise<void> {
-    return this.appStore._deleteBranch(repository, branch, includeRemote)
+    return this.appStore._deleteBranch(repository, branch, includeUpstream)
+  }
+
+  /**
+   * Delete the remote branch.
+   */
+  public deleteRemoteBranch(
+    repository: Repository,
+    branch: Branch
+  ): Promise<void> {
+    return this.appStore._deleteBranch(repository, branch)
   }
 
   /** Discard the changes to the given files. */

app/test/unit/create-branch-test.ts

@@ -25,12 +25,12 @@ const defaultBranch: Branch = {
   upstream: null,
   tip: stubTip,
   type: BranchType.Local,
+  remoteName: null,
   upstreamRemoteName: null,
   upstreamWithoutRemote: null,
   nameWithoutRemote: 'my-default-branch',
   isDesktopForkRemoteBranch: false,
   ref: '',
-  remoteName: '',
 }
 
 const upstreamDefaultBranch = null
@@ -40,12 +40,12 @@ const someOtherBranch: Branch = {
   upstream: null,
   tip: stubTip,
   type: BranchType.Local,
+  remoteName: null,
   upstreamRemoteName: null,
   upstreamWithoutRemote: null,
   nameWithoutRemote: 'some-other-branch',
   isDesktopForkRemoteBranch: false,
   ref: '',
-  remoteName: '',
 }
 
 describe('create-branch/getStartPoint', () => {

app/test/unit/git/checkout-test.ts

@@ -42,10 +42,10 @@ describe('git/checkout', () => {
           tzOffset: 0,
         },
       },
+      remoteName: null,
       upstreamRemoteName: null,
       isDesktopForkRemoteBranch: false,
       ref: '',
-      remoteName: '',
     }
 
     let errorRaised = false