Tidying

app/src/models/cherry-pick.ts

@@ -20,20 +20,24 @@ export type CherryPickFlowStep =
 
 export const enum CherryPickStepKind {
   /**
-   * The initial state of a cherry pick.
+   * An initial state of a cherry pick.
    *
-   * This is where the user picks has started dragging a commit or set
-   * of commits but has not yet dropped them.
+   * This is where the user has started dragging a commit or set of commits but
+   * has not yet dropped them on a branch or an area to launch to choose branch
+   * dialog.
    */
   CommitsChosen = 'CommitsChosen',
+
   /**
-   * The initial state of a cherry pick.
+   * An initial state of a cherry pick.
    *
    * This is where the user picks which is the target of the cherry pick.
    * This step will be skipped when cherry pick is initiated through
-   * drag and drop onto a specific branch.
+   * drag and drop onto a specific branch. But, it will be the first step
+   * if the cherry pick is initiated through the context menu.
    */
   ChooseTargetBranch = 'ChooseTargetBranch',
+
   /**
    * After the user chooses the target branch of the cherry pick, the
    * progress view shows the cherry pick is progressing.
@@ -73,7 +77,7 @@ export type ShowConflictsStep = {
 }
 
 /**
- * Shape of data to when a user has chosen commits to cherry pick but not yet
+ * Shape of data for when a user has chosen commits to cherry pick but not yet
  * selected a target branch.
  */
 export type CommitsChosenStep = {

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

@@ -105,6 +105,10 @@ interface IBranchListProps {
    */
   readonly onFilterListResultsChanged?: (resultCount: number) => void
 
+  /**
+   * Callback to fire when something has been dragged and dropped onto a branch
+   * list item.
+   */
   readonly onBranchDrop?: (branch: Branch) => void
 }
 

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

@@ -312,9 +312,13 @@ export class BranchesContainer extends React.Component<
   }
 
   /**
-   * Currently this is being implemented with
-   * cherry picking. But, this could be expanded if we ever
-   * dropped something else on a branch.
+   * Method is to handle when something is dragged and dropped onto a branch
+   * list item.
+   *
+   * Currently this is being implemented with cherry picking. But, this could be
+   * expanded if we ever dropped something else on a branch; in which case,
+   * we would likely have to check the app state to see what action is being
+   * performed.
    */
   private onBranchDrop = (branch: Branch) => {
     this.props.dispatcher.startCherryPickWithBranch(

app/src/ui/dispatcher/dispatcher.ts

@@ -2631,7 +2631,7 @@ export class Dispatcher {
     return this.appStore._setCherryPickConflictsResolved(repository)
   }
 
-  /*
+  /**
    * Moves cherry pick flow step to progress and defers to allow user to
    * see the cherry picking progress dialog instead of suddenly appearing
    * and disappearing again.
@@ -2642,6 +2642,7 @@ export class Dispatcher {
     })
     await sleep(500)
   }
+
   /**
    * Continue with the cherryPick after the user has resolved all conflicts with
    * tracked files in the working directory.
@@ -2705,8 +2706,12 @@ export class Dispatcher {
   }
 
   /**
-   * When starting cherry pick from drag and drop onto a branch, we need to
-   * initialize the cherry pick state flow step with the ShowProgress step.
+   * This method starts a cherry pick after drag and dropping on a branch.
+   * It needs to:
+   *  - get the current branch,
+   *  - get the commits dragged from cherry picking state
+   *  - invoke popup
+   *  - invoke cherry pick
    */
   public async startCherryPickWithBranch(
     repository: Repository,
@@ -2720,20 +2725,19 @@ export class Dispatcher {
       cherryPickState.step == null ||
       cherryPickState.step.kind !== CherryPickStepKind.CommitsChosen
     ) {
-      log.warn('[cherryPick] - could not determine selected commits')
+      log.warn(
+        '[cherryPick] Invalid Cherry Picking State: Could not determine selected commits.'
+      )
       return
     }
 
     const { tip } = branchesState
-    let sourceBranch: Branch | null = null
-    if (tip.kind === TipState.Valid) {
-      sourceBranch = tip.branch
-    } else {
+    if (tip.kind !== TipState.Valid) {
       throw new Error(
         'Tip is not in a valid state, which is required to start the cherry pick flow.'
       )
     }
-
+    const sourceBranch = tip.branch
     const commits = cherryPickState.step.commits
 
     this.showPopup({

app/src/ui/history/commit-list-item.tsx

@@ -71,7 +71,8 @@ export class CommitListItem extends React.PureComponent<
       author: { date },
     } = commit
 
-    const isDraggable = this.onDragStart !== undefined && enableCherryPicking()
+    const isDraggable =
+      this.props.onDragStart !== undefined && enableCherryPicking()
 
     return (
       <div
@@ -266,18 +267,22 @@ export class CommitListItem extends React.PureComponent<
   }
 
   /**
-   * Note for typescript purposes event is required parameter, but at this point
-   * in time we don't need to bubble it up.
+   * Note: For typing, event is required parameter.
    **/
   private onDragStart = (event: React.DragEvent<HTMLDivElement>): void => {
     if (this.props.onDragStart !== undefined) {
       this.props.onDragStart([this.props.commit])
+
+      // This is done so that we can check if a dropzone should be enabled based
+      // on what is being dropped. The 'dataTransfer' will be readonly and we
+      // cannot access the hash map in the dropzone so we will just be checking
+      // that the 'commit' key exists and not the data.
+      event.dataTransfer.setData('commit', '')
     }
   }
 
   /**
-   * Note for typescript purposes event is required parameter, but at this point
-   * in time we don't need to bubble it up.
+   * Note: For typing, event is required parameter.
    **/
   private onDragEnd = (event: React.DragEvent<HTMLDivElement>): void => {
     if (this.props.onDragEnd !== undefined) {

app/src/ui/history/compare.tsx

@@ -518,6 +518,12 @@ export class CompareSidebar extends React.Component<
     this.props.onCherryPick(this.props.repository, commits)
   }
 
+  /**
+   * This method is a generic event handler for when a commit has started being
+   * dragged.
+   *
+   * Currently only used for cherry picking, but this could be more generic.
+   */
   private onDragCommitStart = (commits: ReadonlyArray<CommitOneLine>) => {
     this.props.dispatcher.setCherryPickFlowStep(this.props.repository, {
       kind: CherryPickStepKind.CommitsChosen,
@@ -525,6 +531,12 @@ export class CompareSidebar extends React.Component<
     })
   }
 
+  /**
+   * This method is a generic event handler for when a commit has ended being
+   * dragged.
+   *
+   * Currently only used for cherry picking, but this could be more generic.
+   */
   private onDragCommitEnd = () => {
     this.props.dispatcher.closeFoldout(FoldoutType.Branch)
   }

app/src/ui/lib/filter-list.tsx

@@ -502,6 +502,10 @@ export class FilterList<T extends IFilterListItem> extends React.Component<
     }
   }
 
+  /**
+   * Method to find the item that something was dragged and dropped onto
+   * and bubble up a more meaningful object.
+   */
   private onRowDrop = (index: number, e: React.DragEvent<HTMLDivElement>) => {
     if (this.props.onRowDrop === undefined) {
       return

app/src/ui/toolbar/branch-dropdown.tsx

@@ -179,9 +179,18 @@ export class BranchDropdown extends React.Component<IBranchDropdownProps> {
     )
   }
 
+  /**
+   * Method to capture when something is dragged over the branch dropdown. A
+   * data piece with type 'commit' was added to the `commit-list-item` drag
+   * event's dataTransfer. This was done preventively in case we ever drag
+   * anything else, the branch dropdown won't automatically open.
+   */
   private onDragOver = (event: React.DragEvent<HTMLDivElement>): void => {
     event.preventDefault()
-    this.props.dispatcher.showFoldout({ type: FoldoutType.Branch })
+    const commitType = event.dataTransfer.types.find(t => t === 'commit')
+    if (commitType !== undefined) {
+      this.props.dispatcher.showFoldout({ type: FoldoutType.Branch })
+    }
   }
 
   private renderPullRequestInfo() {