Merge branch 'development' into Add-errors-to-popups

.github/workflows/release-pr.yml

@@ -37,7 +37,7 @@ jobs:
           private_key: ${{ secrets.DESKTOP_RELEASES_APP_PRIVATE_KEY }}
 
       - name: Create Release Pull Request
-        uses: peter-evans/create-pull-request@v4.1.4
+        uses: peter-evans/create-pull-request@v4.2.0
         if: |
           startsWith(github.ref, 'refs/heads/releases/') && !contains(github.ref, 'test')
         with:

app/package.json

@@ -3,7 +3,7 @@
   "productName": "GitHub Desktop",
   "bundleID": "com.github.GitHubClient",
   "companyName": "GitHub, Inc.",
-  "version": "3.1.3-beta1",
+  "version": "3.1.3-beta2",
   "main": "./main.js",
   "repository": {
     "type": "git",

app/src/lib/feature-flag.ts

@@ -46,7 +46,7 @@ export function enableWSLDetection(): boolean {
  * Should we use the new diff viewer for unified diffs?
  */
 export function enableExperimentalDiffViewer(): boolean {
-  return false
+  return enableBetaFeatures()
 }
 
 /**
@@ -117,3 +117,8 @@ export function enableStartingPullRequests(): boolean {
 export function enableStackedPopups(): boolean {
   return enableDevelopmentFeatures()
 }
+
+/** Should we enable mechanism to prevent closing while the app is updating? */
+export function enablePreventClosingWhileUpdating(): boolean {
+  return enableBetaFeatures()
+}

app/src/lib/ipc-shared.ts

@@ -46,6 +46,8 @@ export type RequestChannels = {
   'menu-event': (name: MenuEvent) => void
   log: (level: LogLevel, message: string) => void
   'will-quit': () => void
+  'will-quit-even-if-updating': () => void
+  'cancel-quitting': () => void
   'crash-ready': () => void
   'crash-quit': () => void
   'window-state-changed': (windowState: WindowState) => void
@@ -63,6 +65,7 @@ export type RequestChannels = {
   blur: () => void
   'update-accounts': (accounts: ReadonlyArray<EndpointToken>) => void
   'quit-and-install-updates': () => void
+  'quit-app': () => void
   'minimize-window': () => void
   'maximize-window': () => void
   'unmaximize-window': () => void
@@ -77,6 +80,7 @@ export type RequestChannels = {
   'focus-window': () => void
   'notification-event': NotificationCallback<DesktopAliveEvent>
   'set-window-zoom-factor': (zoomFactor: number) => void
+  'show-installing-update': () => void
 }
 
 /**

app/src/lib/rebase.ts

@@ -3,6 +3,26 @@ import { IAheadBehind } from '../models/branch'
 import { TipState } from '../models/tip'
 import { clamp } from './clamp'
 
+/** Represents the force-push availability state of a branch. */
+export enum ForcePushBranchState {
+  /** The branch cannot be force-pushed (it hasn't diverged from its upstream) */
+  NotAvailable,
+
+  /**
+   * The branch can be force-pushed, but the user didn't do any operation that
+   * we consider should be followed by a force-push, like rebasing or amending a
+   * pushed commit.
+   */
+  Available,
+
+  /**
+   * The branch can be force-pushed, and the user did some operation that we
+   * consider should be followed by a force-push, like rebasing or amending a
+   * pushed commit.
+   */
+  Recommended,
+}
+
 /**
  * Format rebase percentage to ensure it's a value between 0 and 1, but to also
  * constrain it to two significant figures, avoiding the remainder that comes
@@ -16,17 +36,23 @@ export function formatRebaseValue(value: number) {
  * Check application state to see whether the action applied to the current
  * branch should be a force push
  */
-export function isCurrentBranchForcePush(
+export function getCurrentBranchForcePushState(
   branchesState: IBranchesState,
   aheadBehind: IAheadBehind | null
-) {
+): ForcePushBranchState {
   if (aheadBehind === null) {
     // no tracking branch found
-    return false
+    return ForcePushBranchState.NotAvailable
+  }
+
+  const { ahead, behind } = aheadBehind
+
+  if (behind === 0 || ahead === 0) {
+    // no a diverged branch to force push
+    return ForcePushBranchState.NotAvailable
   }
 
   const { tip, forcePushBranches } = branchesState
-  const { ahead, behind } = aheadBehind
 
   let canForcePushBranch = false
   if (tip.kind === TipState.Valid) {
@@ -36,5 +62,7 @@ export function isCurrentBranchForcePush(
     canForcePushBranch = foundEntry === sha
   }
 
-  return canForcePushBranch && behind > 0 && ahead > 0
+  return canForcePushBranch
+    ? ForcePushBranchState.Recommended
+    : ForcePushBranchState.Available
 }

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

@@ -77,6 +77,10 @@ import {
   updatePreferredAppMenuItemLabels,
   updateAccounts,
   setWindowZoomFactor,
+  onShowInstallingUpdate,
+  sendWillQuitEvenIfUpdatingSync,
+  quitApp,
+  sendCancelQuittingSync,
 } from '../../ui/main-process-proxy'
 import {
   API,
@@ -180,7 +184,7 @@ import {
   matchExistingRepository,
   urlMatchesRemote,
 } from '../repository-matching'
-import { isCurrentBranchForcePush } from '../rebase'
+import { ForcePushBranchState, getCurrentBranchForcePushState } from '../rebase'
 import { RetryAction, RetryActionType } from '../../models/retry-actions'
 import {
   Default as DefaultShell,
@@ -584,6 +588,8 @@ export class AppStore extends TypedBaseStore<IAppState> {
     this.notificationsStore.onPullRequestReviewSubmitNotification(
       this.onPullRequestReviewSubmitNotification
     )
+
+    onShowInstallingUpdate(this.onShowInstallingUpdate)
   }
 
   private initializeWindowState = async () => {
@@ -654,6 +660,12 @@ export class AppStore extends TypedBaseStore<IAppState> {
     })
   }
 
+  private onShowInstallingUpdate = () => {
+    this._showPopup({
+      type: PopupType.InstallingUpdate,
+    })
+  }
+
   /** Figure out what step of the tutorial the user needs to do next */
   private async updateCurrentTutorialStep(
     repository: Repository
@@ -2233,10 +2245,12 @@ export class AppStore extends TypedBaseStore<IAppState> {
           ?.name ?? undefined
     }
 
-    const isForcePushForCurrentRepository = isCurrentBranchForcePush(
-      branchesState,
-      aheadBehind
-    )
+    // From the menu, we'll offer to force-push whenever it's possible, regardless
+    // of whether or not the user performed any action we know would be followed
+    // by a force-push.
+    const isForcePushForCurrentRepository =
+      getCurrentBranchForcePushState(branchesState, aheadBehind) !==
+      ForcePushBranchState.NotAvailable
 
     const isStashedChangesVisible =
       changesState.selection.kind === ChangesSelectionKind.Stash
@@ -6575,6 +6589,25 @@ export class AppStore extends TypedBaseStore<IAppState> {
     }
   }
 
+  /**
+   * Multi selection on the commit list can give an order of 1, 5, 3 if that is
+   * how the user selected them. However, we want to main chronological ordering
+   * of the commits to reduce the chance of conflicts during interact rebasing.
+   * Thus, assuming 1 is the first commit made by the user and 5 is the last. We
+   * want the order to be, 1, 3, 5.
+   */
+  private orderCommitsByHistory(
+    repository: Repository,
+    commits: ReadonlyArray<CommitOneLine>
+  ) {
+    const { compareState } = this.repositoryStateCache.get(repository)
+    const { commitSHAs } = compareState
+
+    return [...commits].sort(
+      (a, b) => commitSHAs.indexOf(b.sha) - commitSHAs.indexOf(a.sha)
+    )
+  }
+
   /** This shouldn't be called directly. See `Dispatcher`. */
   public async _cherryPick(
     repository: Repository,
@@ -6585,13 +6618,15 @@ export class AppStore extends TypedBaseStore<IAppState> {
       return CherryPickResult.UnableToStart
     }
 
+    const orderedCommits = this.orderCommitsByHistory(repository, commits)
+
     await this._refreshRepository(repository)
 
     const progressCallback =
       this.getMultiCommitOperationProgressCallBack(repository)
     const gitStore = this.gitStoreCache.get(repository)
     const result = await gitStore.performFailableOperation(() =>
-      cherryPick(repository, commits, progressCallback)
+      cherryPick(repository, orderedCommits, progressCallback)
     )
 
     return result || CherryPickResult.Error
@@ -7470,6 +7505,18 @@ export class AppStore extends TypedBaseStore<IAppState> {
       }
     )
   }
+
+  public _quitApp(evenIfUpdating: boolean) {
+    if (evenIfUpdating) {
+      sendWillQuitEvenIfUpdatingSync()
+    }
+
+    quitApp()
+  }
+
+  public _cancelQuittingApp() {
+    sendCancelQuittingSync()
+  }
 }
 
 /**

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

@@ -2,7 +2,9 @@ import {
   Repository,
   isRepositoryWithGitHubRepository,
   RepositoryWithGitHubRepository,
+  isRepositoryWithForkedGitHubRepository,
 } from '../../models/repository'
+import { ForkContributionTarget } from '../../models/workflow-preferences'
 import { PullRequest } from '../../models/pull-request'
 import { API, APICheckConclusion } from '../api'
 import {
@@ -121,6 +123,10 @@ export class NotificationsStore {
       return
     }
 
+    if (!this.isValidRepositoryForEvent(repository, event)) {
+      return
+    }
+
     const pullRequests = await this.pullRequestCoordinator.getAllPullRequests(
       repository
     )
@@ -192,6 +198,10 @@ export class NotificationsStore {
       return
     }
 
+    if (!this.isValidRepositoryForEvent(repository, event)) {
+      return
+    }
+
     const pullRequests = await this.pullRequestCoordinator.getAllPullRequests(
       repository
     )
@@ -283,6 +293,31 @@ export class NotificationsStore {
     this.statsStore.recordChecksFailedNotificationShown()
   }
 
+  private isValidRepositoryForEvent(
+    repository: RepositoryWithGitHubRepository,
+    event: DesktopAliveEvent
+  ) {
+    // If it's a fork and set to contribute to the parent repository, try to
+    // match the parent repository.
+    if (
+      isRepositoryWithForkedGitHubRepository(repository) &&
+      repository.workflowPreferences.forkContributionTarget ===
+        ForkContributionTarget.Parent
+    ) {
+      const parentRepository = repository.gitHubRepository.parent
+      return (
+        parentRepository.owner.login === event.owner &&
+        parentRepository.name === event.repo
+      )
+    }
+
+    const ghRepository = repository.gitHubRepository
+    return (
+      ghRepository.owner.login === event.owner &&
+      ghRepository.name === event.repo
+    )
+  }
+
   /**
    * Makes the store to keep track of the currently selected repository. Only
    * notifications for the currently selected repository will be shown.

app/src/main-process/app-window.ts

@@ -26,6 +26,7 @@ import {
   terminateDesktopNotifications,
 } from './notifications'
 import { addTrustedIPCSender } from './trusted-ipc-sender'
+import { enablePreventClosingWhileUpdating } from '../lib/feature-flag'
 
 export class AppWindow {
   private window: Electron.BrowserWindow
@@ -33,6 +34,7 @@ export class AppWindow {
 
   private _loadTime: number | null = null
   private _rendererReadyTime: number | null = null
+  private isDownloadingUpdate: boolean = false
 
   private minWidth = 960
   private minHeight = 660
@@ -86,6 +88,7 @@ export class AppWindow {
     this.shouldMaximizeOnShow = savedWindowState.isMaximized
 
     let quitting = false
+    let quittingEvenIfUpdating = false
     app.on('before-quit', () => {
       quitting = true
     })
@@ -95,7 +98,40 @@ export class AppWindow {
       event.returnValue = true
     })
 
+    ipcMain.on('will-quit-even-if-updating', event => {
+      quitting = true
+      quittingEvenIfUpdating = true
+      event.returnValue = true
+    })
+
+    ipcMain.on('cancel-quitting', event => {
+      quitting = false
+      quittingEvenIfUpdating = false
+      event.returnValue = true
+    })
+
     this.window.on('close', e => {
+      // On macOS, closing the window doesn't mean the app is quitting. If the
+      // app is updating, we will prevent the window from closing only when the
+      // app is also quitting.
+      if (
+        enablePreventClosingWhileUpdating() &&
+        (!__DARWIN__ || quitting) &&
+        !quittingEvenIfUpdating &&
+        this.isDownloadingUpdate
+      ) {
+        e.preventDefault()
+        ipcWebContents.send(this.window.webContents, 'show-installing-update')
+
+        // Make sure the window is visible, so the user can see why we're
+        // preventing the app from quitting. This is important on macOS, where
+        // the window could be hidden/closed when the user tries to quit.
+        // It could also happen on Windows if the user quits the app from the
+        // task bar while it's in the background.
+        this.show()
+        return
+      }
+
       // on macOS, when the user closes the window we really just hide it. This
       // lets us activate quickly and keep all our interesting logic in the
       // renderer.
@@ -213,7 +249,7 @@ export class AppWindow {
     return !!this.loadTime && !!this.rendererReadyTime
   }
 
-  public onClose(fn: () => void) {
+  public onClosed(fn: () => void) {
     this.window.on('closed', fn)
   }
 
@@ -344,10 +380,12 @@ export class AppWindow {
 
   public setupAutoUpdater() {
     autoUpdater.on('error', (error: Error) => {
+      this.isDownloadingUpdate = false
       ipcWebContents.send(this.window.webContents, 'auto-updater-error', error)
     })
 
     autoUpdater.on('checking-for-update', () => {
+      this.isDownloadingUpdate = false
       ipcWebContents.send(
         this.window.webContents,
         'auto-updater-checking-for-update'
@@ -355,6 +393,7 @@ export class AppWindow {
     })
 
     autoUpdater.on('update-available', () => {
+      this.isDownloadingUpdate = true
       ipcWebContents.send(
         this.window.webContents,
         'auto-updater-update-available'
@@ -362,6 +401,7 @@ export class AppWindow {
     })
 
     autoUpdater.on('update-not-available', () => {
+      this.isDownloadingUpdate = false
       ipcWebContents.send(
         this.window.webContents,
         'auto-updater-update-not-available'
@@ -369,6 +409,7 @@ export class AppWindow {
     })
 
     autoUpdater.on('update-downloaded', () => {
+      this.isDownloadingUpdate = false
       ipcWebContents.send(
         this.window.webContents,
         'auto-updater-update-downloaded'

app/src/main-process/main.ts

@@ -490,6 +490,8 @@ app.on('ready', () => {
     mainWindow?.quitAndInstallUpdate()
   )
 
+  ipcMain.on('quit-app', () => app.quit())
+
   ipcMain.on('minimize-window', () => mainWindow?.minimizeWindow())
 
   ipcMain.on('maximize-window', () => mainWindow?.maximizeWindow())
@@ -738,7 +740,7 @@ function createWindow() {
     }
   }
 
-  window.onClose(() => {
+  window.onClosed(() => {
     mainWindow = null
     if (!__DARWIN__ && !preventQuit) {
       app.quit()

app/src/main-process/menu/build-default-menu.ts

@@ -285,6 +285,12 @@ export function buildDefaultMenu({
         accelerator: 'CmdOrCtrl+Shift+P',
         click: emit('pull'),
       },
+      {
+        id: 'fetch',
+        label: __DARWIN__ ? 'Fetch' : '&Fetch',
+        accelerator: 'CmdOrCtrl+Shift+T',
+        click: emit('fetch'),
+      },
       {
         label: removeRepoLabel,
         id: 'remove-repository',

app/src/main-process/menu/menu-event.ts

@@ -2,6 +2,7 @@ export type MenuEvent =
   | 'push'
   | 'force-push'
   | 'pull'
+  | 'fetch'
   | 'show-changes'
   | 'show-history'
   | 'add-local-repository'

app/src/models/popup.ts

@@ -88,6 +88,7 @@ export enum PopupType {
   UnreachableCommits = 'UnreachableCommits',
   StartPullRequest = 'StartPullRequest',
   Error = 'Error',
+  InstallingUpdate = 'InstallingUpdate',
 }
 
 interface IBasePopup {
@@ -384,5 +385,8 @@ export type PopupDetail =
       type: PopupType.Error
       error: Error
     }
+  | {
+      type: PopupType.InstallingUpdate
+    }
 
 export type Popup = IBasePopup & PopupDetail

app/src/ui/add-repository/add-existing-repository.tsx

@@ -11,6 +11,7 @@ import * as OcticonSymbol from '../octicons/octicons.generated'
 import { LinkButton } from '../lib/link-button'
 import { PopupType } from '../../models/popup'
 import { OkCancelButtonGroup } from '../dialog/ok-cancel-button-group'
+import { FoldoutType } from '../../lib/app-state'
 
 import untildify from 'untildify'
 import { showOpenDialog } from '../main-process-proxy'
@@ -265,6 +266,7 @@ export class AddExistingRepository extends React.Component<
     const repositories = await dispatcher.addRepositories([resolvedPath])
 
     if (repositories.length > 0) {
+      dispatcher.closeFoldout(FoldoutType.Repository)
       dispatcher.selectRepository(repositories[0])
       dispatcher.recordAddExistingRepository()
     }

app/src/ui/add-repository/create-repository.tsx

@@ -34,6 +34,7 @@ import { showOpenDialog } from '../main-process-proxy'
 import { pathExists } from '../lib/path-exists'
 import { mkdir } from 'fs/promises'
 import { directoryExists } from '../../lib/directory-exists'
+import { FoldoutType } from '../../lib/app-state'
 import { join } from 'path'
 
 /** The sentinel value used to indicate no gitignore should be used. */
@@ -391,6 +392,7 @@ export class CreateRepository extends React.Component<
 
     this.updateDefaultDirectory()
 
+    this.props.dispatcher.closeFoldout(FoldoutType.Repository)
     this.props.dispatcher.selectRepository(repository)
     this.props.dispatcher.recordCreateRepository()
     this.props.onDismissed()

app/src/ui/app.tsx

@@ -13,6 +13,7 @@ import { assertNever } from '../lib/fatal-error'
 import { shell } from '../lib/app-shell'
 import { updateStore, UpdateStatus } from './lib/update-store'
 import { RetryAction } from '../models/retry-actions'
+import { FetchType } from '../models/fetch'
 import { shouldRenderApplicationMenu } from './lib/features'
 import { matchExistingRepository } from '../lib/repository-matching'
 import { getDotComAPIEndpoint } from '../lib/api'
@@ -92,7 +93,10 @@ import { RepositoryStateCache } from '../lib/stores/repository-state-cache'
 import { PopupType, Popup } from '../models/popup'
 import { OversizedFiles } from './changes/oversized-files-warning'
 import { PushNeedsPullWarning } from './push-needs-pull'
-import { isCurrentBranchForcePush } from '../lib/rebase'
+import {
+  ForcePushBranchState,
+  getCurrentBranchForcePushState,
+} from '../lib/rebase'
 import { Banner, BannerType } from '../models/banner'
 import { StashAndSwitchBranch } from './stash-changes/stash-and-switch-branch-dialog'
 import { OverwriteStash } from './stash-changes/overwrite-stashed-changes-dialog'
@@ -160,6 +164,7 @@ import { OpenPullRequestDialog } from './open-pull-request/open-pull-request-dia
 import { sendNonFatalException } from '../lib/helpers/non-fatal-exception'
 import { createCommitURL } from '../lib/commit-url'
 import { uuid } from '../lib/uuid'
+import { InstallingUpdate } from './installing-update/installing-update'
 
 const MinuteInMilliseconds = 1000 * 60
 const HourInMilliseconds = MinuteInMilliseconds * 60
@@ -280,7 +285,14 @@ export class App extends React.Component<IAppProps, IAppState> {
     updateStore.onError(error => {
       log.error(`Error checking for updates`, error)
 
-      this.props.dispatcher.postError(error)
+      // It is possible to obtain an error with no message. This was found to be
+      // the case on a windows instance where there was not space on the hard
+      // drive to download the installer. In this case, we want to override the
+      // error message so the user is not given a blank dialog.
+      const hasErrorMsg = error.message.trim().length > 0
+      this.props.dispatcher.postError(
+        hasErrorMsg ? error : new Error('Checking for updates failed.')
+      )
     })
 
     ipcRenderer.on('launch-timing-stats', (_, stats) => {
@@ -359,6 +371,8 @@ export class App extends React.Component<IAppProps, IAppState> {
         return this.push({ forceWithLease: true })
       case 'pull':
         return this.pull()
+      case 'fetch':
+        return this.fetch()
       case 'show-changes':
         return this.showChanges()
       case 'show-history':
@@ -954,6 +968,15 @@ export class App extends React.Component<IAppProps, IAppState> {
     this.props.dispatcher.pull(state.repository)
   }
 
+  private async fetch() {
+    const state = this.state.selectedState
+    if (state == null || state.type !== SelectionType.Repository) {
+      return
+    }
+
+    this.props.dispatcher.fetch(state.repository, FetchType.UserInitiatedTask)
+  }
+
   private showStashedChanges() {
     const state = this.state.selectedState
     if (state == null || state.type !== SelectionType.Repository) {
@@ -2319,6 +2342,15 @@ export class App extends React.Component<IAppProps, IAppState> {
           />
         )
       }
+      case PopupType.InstallingUpdate: {
+        return (
+          <InstallingUpdate
+            key="installing-update"
+            dispatcher={this.props.dispatcher}
+            onDismissed={onPopupDismissedFn}
+          />
+        )
+      }
       default:
         return assertNever(popup, `Unknown popup type: ${popup}`)
     }
@@ -2742,7 +2774,9 @@ export class App extends React.Component<IAppProps, IAppState> {
       remoteName = tip.branch.upstreamRemoteName
     }
 
-    const isForcePush = isCurrentBranchForcePush(branchesState, aheadBehind)
+    const isForcePush =
+      getCurrentBranchForcePushState(branchesState, aheadBehind) ===
+      ForcePushBranchState.Recommended
 
     return (
       <PushPullButton

app/src/ui/changes/no-changes.tsx

@@ -13,7 +13,10 @@ import { TipState, IValidBranch } from '../../models/tip'
 import { Ref } from '../lib/ref'
 import { IAheadBehind } from '../../models/branch'
 import { IRemote } from '../../models/remote'
-import { isCurrentBranchForcePush } from '../../lib/rebase'
+import {
+  ForcePushBranchState,
+  getCurrentBranchForcePushState,
+} from '../../lib/rebase'
 import { StashedChangesLoadStates } from '../../models/stash-entry'
 import { Dispatcher } from '../dispatcher'
 import { SuggestedActionGroup } from '../suggested-actions'
@@ -341,7 +344,9 @@ export class NoChanges extends React.Component<
       return this.renderPublishBranchAction(tip)
     }
 
-    const isForcePush = isCurrentBranchForcePush(branchesState, aheadBehind)
+    const isForcePush =
+      getCurrentBranchForcePushState(branchesState, aheadBehind) ===
+      ForcePushBranchState.Recommended
     if (isForcePush) {
       // do not render an action currently after the rebase has completed, as
       // the default behaviour is currently to pull in changes from the tracking

app/src/ui/dialog/dialog.tsx

@@ -214,6 +214,13 @@ export class Dialog extends React.Component<IDialogProps, IDialogState> {
 
   private onDismissGraceTimer = () => {
     this.setState({ isAppearing: false })
+
+    this.dialogElement?.dispatchEvent(
+      new CustomEvent('dialog-appeared', {
+        bubbles: true,
+        cancelable: false,
+      })
+    )
   }
 
   private isDismissable() {

app/src/ui/diff/code-mirror-host.tsx

@@ -164,6 +164,14 @@ export class CodeMirrorHost extends React.Component<ICodeMirrorHostProps, {}> {
 
     CodeMirrorHost.updateDoc(this.codeMirror, this.props.value)
     this.resizeObserver.observe(this.codeMirror.getWrapperElement())
+
+    if (this.wrapper !== null && this.wrapper.closest('dialog') !== null) {
+      document.addEventListener('dialog-appeared', this.onDialogAppeared)
+    }
+  }
+
+  private onDialogAppeared = () => {
+    requestAnimationFrame(this.onResized)
   }
 
   private onSwapDoc = (cm: Editor, oldDoc: Doc) => {
@@ -199,6 +207,7 @@ export class CodeMirrorHost extends React.Component<ICodeMirrorHostProps, {}> {
     }
 
     this.resizeObserver.disconnect()
+    document.removeEventListener('dialog-show', this.onDialogAppeared)
   }
 
   public componentDidUpdate(prevProps: ICodeMirrorHostProps) {

app/src/ui/diff/side-by-side-diff-row.tsx

@@ -363,6 +363,16 @@ export class SideBySideDiffRow extends React.Component<
     throw new Error(`Unexpected expansion type ${expansionType}`)
   }
 
+  /**
+   * This method returns the width of a line gutter in pixels. For unified diffs
+   * the gutter contains the line number of both before and after sides, whereas
+   * for side-by-side diffs the gutter contains the line number of only one side.
+   */
+  private get lineGutterWidth() {
+    const { showSideBySideDiff, lineNumberWidth } = this.props
+    return showSideBySideDiff ? lineNumberWidth : lineNumberWidth * 2
+  }
+
   private renderHunkExpansionHandle(
     hunkIndex: number,
     expansionType: DiffHunkExpansionType
@@ -372,7 +382,7 @@ export class SideBySideDiffRow extends React.Component<
         <div
           className="hunk-expansion-handle"
           onContextMenu={this.props.onContextMenuExpandHunk}
-          style={{ width: this.props.lineNumberWidth }}
+          style={{ width: this.lineGutterWidth }}
         >
           <span></span>
         </div>
@@ -389,7 +399,7 @@ export class SideBySideDiffRow extends React.Component<
       <div
         className="hunk-expansion-handle selectable hoverable"
         onClick={elementInfo.handler}
-        style={{ width: this.props.lineNumberWidth }}
+        style={{ width: this.lineGutterWidth }}
         onContextMenu={this.props.onContextMenuExpandHunk}
       >
         <TooltippedContent
@@ -452,10 +462,7 @@ export class SideBySideDiffRow extends React.Component<
   ) {
     if (!this.props.isDiffSelectable || isSelected === undefined) {
       return (
-        <div
-          className="line-number"
-          style={{ width: this.props.lineNumberWidth }}
-        >
+        <div className="line-number" style={{ width: this.lineGutterWidth }}>
           {lineNumbers.map((lineNumber, index) => (
             <span key={index}>{lineNumber}</span>
           ))}
@@ -470,7 +477,7 @@ export class SideBySideDiffRow extends React.Component<
           'line-selected': isSelected,
           hover: this.props.isHunkHovered,
         })}
-        style={{ width: this.props.lineNumberWidth }}
+        style={{ width: this.lineGutterWidth }}
         onMouseDown={this.onMouseDownLineNumber}
         onContextMenu={this.onContextMenuLineNumber}
       >
@@ -493,7 +500,7 @@ export class SideBySideDiffRow extends React.Component<
 
     const style: React.CSSProperties = {
       [column === DiffColumn.Before ? 'marginRight' : 'marginLeft']:
-        this.props.lineNumberWidth + 10,
+        this.lineGutterWidth + 10,
       marginTop: -10,
     }
 

app/src/ui/diff/side-by-side-diff.tsx

@@ -256,7 +256,7 @@ export class SideBySideDiff extends React.Component<
         : [DiffLineType.Add, DiffLineType.Context]
       : [DiffLineType.Add, DiffLineType.Delete, DiffLineType.Context]
 
-    const contents = this.props.diff.hunks
+    const contents = this.state.diff.hunks
       .flatMap(h =>
         h.lines
           .filter(line => lineTypes.includes(line.type))

app/src/ui/dispatcher/dispatcher.ts

@@ -4011,4 +4011,24 @@ export class Dispatcher {
   public updatePullRequestBaseBranch(repository: Repository, branch: Branch) {
     this.appStore._updatePullRequestBaseBranch(repository, branch)
   }
+
+  /**
+   * Attempts to quit the app if it's not updating, unless requested to quit
+   * even if it is updating.
+   *
+   * @param evenIfUpdating Whether to quit even if the app is updating.
+   */
+  public quitApp(evenIfUpdating: boolean) {
+    this.appStore._quitApp(evenIfUpdating)
+  }
+
+  /**
+   * Cancels quitting the app. This could be needed if, on macOS, the user tries
+   * to quit the app while an update is in progress, but then after being
+   * informed about the issues that could cause they decided to not close the
+   * app yet.
+   */
+  public cancelQuittingApp() {
+    this.appStore._cancelQuittingApp()
+  }
 }

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

@@ -275,15 +275,9 @@ export class CommitList extends React.Component<ICommitListProps, {}> {
   }
 
   private onSelectionChanged = (rows: ReadonlyArray<number>) => {
-    // Multi select can give something like 1, 5, 3 depending on order that user
-    // selects. We want to ensure they are in chronological order for best
-    // cherry-picking results. If user wants to use cherry-picking for
-    // reordering, they will need to do multiple cherry-picks.
-    // Goal: first commit in history -> first on array
-    const sorted = [...rows].sort((a, b) => b - a)
-    const selectedShas = sorted.map(r => this.props.commitSHAs[r])
+    const selectedShas = rows.map(r => this.props.commitSHAs[r])
     const selectedCommits = this.lookupCommits(selectedShas)
-    this.props.onCommitsSelected?.(selectedCommits, this.isContiguous(sorted))
+    this.props.onCommitsSelected?.(selectedCommits, this.isContiguous(rows))
   }
 
   /**
@@ -297,13 +291,15 @@ export class CommitList extends React.Component<ICommitListProps, {}> {
       return true
     }
 
-    for (let i = 0; i < indexes.length; i++) {
-      const current = indexes[i]
-      if (i + 1 === indexes.length) {
+    const sorted = [...indexes].sort((a, b) => b - a)
+
+    for (let i = 0; i < sorted.length; i++) {
+      const current = sorted[i]
+      if (i + 1 === sorted.length) {
         continue
       }
 
-      if (current - 1 !== indexes[i + 1]) {
+      if (current - 1 !== sorted[i + 1]) {
         return false
       }
     }

app/src/ui/installing-update/installing-update.tsx

@@ -0,0 +1,96 @@
+import * as React from 'react'
+
+import { Row } from '../lib/row'
+import {
+  Dialog,
+  DialogContent,
+  OkCancelButtonGroup,
+  DialogFooter,
+} from '../dialog'
+import { updateStore, IUpdateState, UpdateStatus } from '../lib/update-store'
+import { Disposable } from 'event-kit'
+import { DialogHeader } from '../dialog/header'
+import { Dispatcher } from '../dispatcher'
+
+interface IInstallingUpdateProps {
+  /**
+   * Event triggered when the dialog is dismissed by the user in the
+   * ways described in the Dialog component's dismissable prop.
+   */
+  readonly onDismissed: () => void
+
+  readonly dispatcher: Dispatcher
+}
+
+/**
+ * A dialog that presents information about the
+ * running application such as name and version.
+ */
+export class InstallingUpdate extends React.Component<IInstallingUpdateProps> {
+  private updateStoreEventHandle: Disposable | null = null
+
+  private onUpdateStateChanged = (updateState: IUpdateState) => {
+    // If the update is not being downloaded (`UpdateStatus.UpdateAvailable`),
+    // i.e. if it's already downloaded or not available, close the window.
+    if (updateState.status !== UpdateStatus.UpdateAvailable) {
+      this.props.dispatcher.quitApp(false)
+    }
+  }
+
+  public componentDidMount() {
+    this.updateStoreEventHandle = updateStore.onDidChange(
+      this.onUpdateStateChanged
+    )
+
+    // Manually update the state to ensure we're in sync with the store
+    this.onUpdateStateChanged(updateStore.state)
+  }
+
+  public componentWillUnmount() {
+    if (this.updateStoreEventHandle) {
+      this.updateStoreEventHandle.dispose()
+      this.updateStoreEventHandle = null
+    }
+
+    // This will ensure the app doesn't try to quit after the update is
+    // installed once the dialog is closed (explicitly or implicitly, by
+    // opening another dialog on top of this one).
+    this.props.dispatcher.cancelQuittingApp()
+  }
+
+  private onQuitAnywayButtonClicked = () => {
+    this.props.dispatcher.quitApp(true)
+  }
+
+  public render() {
+    return (
+      <Dialog
+        id="installing-update"
+        onSubmit={this.props.onDismissed}
+        dismissable={false}
+        type="warning"
+      >
+        <DialogHeader
+          title={__DARWIN__ ? 'Installing Update…' : 'Installing update…'}
+          loading={true}
+          dismissable={true}
+          onDismissed={this.props.onDismissed}
+        />
+        <DialogContent>
+          <Row className="updating-message">
+            Do not close GitHub Desktop while the update is in progress. Closing
+            now may break your installation.
+          </Row>
+        </DialogContent>
+        <DialogFooter>
+          <OkCancelButtonGroup
+            okButtonText={__DARWIN__ ? 'Quit Anyway' : 'Quit anyway'}
+            onOkButtonClick={this.onQuitAnywayButtonClicked}
+            onCancelButtonClick={this.props.onDismissed}
+            destructive={true}
+          />
+        </DialogFooter>
+      </Dialog>
+    )
+  }
+}

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

@@ -74,6 +74,14 @@ interface IListProps {
    * The currently selected rows indexes. Used to attach a special
    * selection class on those row's containers as well as being used
    * for keyboard selection.
+   *
+   * It is expected that the use case for this is setting of the initially
+   * selected rows or clearing a list selection.
+   *
+   * N.B. Since it is used for keyboard selection, changing the ordering of
+   * elements in this array in a parent component may result in unexpected
+   * behaviors when a user modifies their selection via key commands.
+   * See #15536 lessons learned.
    */
   readonly selectedRows: ReadonlyArray<number>
 

app/src/ui/main-process-proxy.ts

@@ -164,6 +164,9 @@ export const checkForUpdates = invokeProxy('check-for-updates', 1)
 /** Tell the main process to quit the app and install updates */
 export const quitAndInstallUpdate = sendProxy('quit-and-install-updates', 0)
 
+/** Tell the main process to quit the app */
+export const quitApp = sendProxy('quit-app', 0)
+
 /** Subscribes to auto updater error events originating from the main process */
 export function onAutoUpdaterError(
   errorHandler: (evt: Electron.IpcRendererEvent, error: Error) => void
@@ -200,6 +203,12 @@ export function onNativeThemeUpdated(eventHandler: () => void) {
   ipcRenderer.on('native-theme-updated', eventHandler)
 }
 
+/** Subscribes to the "show installing update dialog" event originating from the
+ * main process */
+export function onShowInstallingUpdate(eventHandler: () => void) {
+  ipcRenderer.on('show-installing-update', eventHandler)
+}
+
 /** Tell the main process to set the native theme source */
 export const setNativeThemeSource = sendProxy('set-native-theme-source', 1)
 
@@ -273,6 +282,29 @@ export function sendWillQuitSync() {
   ipcRenderer.sendSync('will-quit')
 }
 
+/**
+ * Tell the main process that we're going to quit, even if the app is installing
+ * an update. This means it should allow the window to close.
+ *
+ * This event is sent synchronously to avoid any races with subsequent calls
+ * that would tell the app to quit.
+ */
+export function sendWillQuitEvenIfUpdatingSync() {
+  // eslint-disable-next-line no-sync
+  ipcRenderer.sendSync('will-quit-even-if-updating')
+}
+
+/**
+ * Tell the main process that the user cancelled quitting.
+ *
+ * This event is sent synchronously to avoid any races with subsequent calls
+ * that would tell the app to quit.
+ */
+export function sendCancelQuittingSync() {
+  // eslint-disable-next-line no-sync
+  ipcRenderer.sendSync('cancel-quitting')
+}
+
 /**
  * Tell the main process to move the application to the application folder
  */

app/styles/ui/_dialog.scss

@@ -21,12 +21,16 @@
 @import 'dialogs/ci-check-run-rerun';
 @import 'dialogs/unreachable-commits';
 @import 'dialogs/open-pull-request';
+@import 'dialogs/installing-update';
 
 // The styles herein attempt to follow a flow where margins are only applied
 // to the bottom of elements (with the exception of the last child). This to
 // allow easy layout using generalized components and elements such as <Row>
 // and <p>.
 dialog {
+  display: flex;
+  flex-direction: column;
+
   overflow: unset;
   // These are the 24px versions of the alert and stop octicons
   // from oction v10.0.0
@@ -128,11 +132,21 @@ dialog {
   // The dialog embeds a fieldset as the first child of the form element
   // in order to be able to disable all form elements and buttons in one
   // swoop. This resets all styles for that fieldset.
-  & > form > fieldset {
-    border: 0;
-    margin: 0;
-    padding: 0;
-    min-width: 0;
+  & > form {
+    min-height: 0;
+    height: 100%;
+    & > fieldset {
+      border: 0;
+      margin: 0;
+      padding: 0;
+      min-width: 0;
+      min-height: 0;
+      max-height: 100%;
+      height: 100%;
+
+      display: flex;
+      flex-direction: column;
+    }
   }
 
   .dialog-header {

app/styles/ui/_pull-request-files-changed.scss

@@ -1,4 +1,9 @@
 .pull-request-files-changed {
+  display: flex;
+  flex-direction: column;
+  min-height: 0;
+  flex-grow: 1;
+
   border: var(--base-border);
   border-radius: var(--border-radius);
 
@@ -13,8 +18,9 @@
   }
 
   .files-diff-viewer {
-    height: 500px;
     display: flex;
+    min-height: 0;
+    flex-grow: 1;
   }
 
   .file-list {

app/styles/ui/_side-by-side-diff.scss

@@ -326,9 +326,23 @@
         left: 100px;
       }
 
-      &.hunk-info .line-number {
-        background: var(--diff-hunk-gutter-background-color);
-        border-color: var(--diff-hunk-border-color);
+      &.hunk-info {
+        .line-number {
+          background: var(--diff-hunk-gutter-background-color);
+          border-color: var(--diff-hunk-border-color);
+        }
+        .hunk-expansion-handle {
+          background: var(--diff-hunk-gutter-background-color);
+          border-right-width: 1px;
+          border-right-style: solid;
+          border-color: var(--diff-hunk-gutter-background-color);
+          align-self: stretch;
+          align-items: center;
+
+          &.selectable:hover {
+            border-color: var(--diff-hover-background-color);
+          }
+        }
       }
 
       .line-number {
@@ -349,8 +363,13 @@
       }
     }
 
-    &.editable .row .line-number {
-      border-right-width: 4px;
+    &.editable .row {
+      .line-number {
+        border-right-width: 4px;
+      }
+      .hunk-expansion-handle {
+        border-right-width: 4px;
+      }
     }
   }
 

app/styles/ui/dialogs/_installing-update.scss

@@ -0,0 +1,7 @@
+#installing-update {
+  max-width: 400px;
+
+  .updating-message {
+    align-items: center;
+  }
+}

app/styles/ui/dialogs/_open-pull-request.scss

@@ -1,6 +1,8 @@
 .open-pull-request {
-  width: 850px;
-  max-width: none;
+  width: 100%;
+  height: 100%;
+  max-width: calc(100% - var(--spacing-double) * 4);
+  max-height: calc(100% - var(--spacing-double) * 4);
 
   header.dialog-header {
     padding-bottom: var(--spacing);
@@ -21,6 +23,10 @@
 
   .open-pull-request-content {
     padding: var(--spacing);
+    display: flex;
+    flex-direction: column;
+    min-height: 0;
+    flex-grow: 1;
   }
 
   .open-pull-request-no-changes {

changelog.json

@@ -1,5 +1,20 @@
 {
   "releases": {
+    "3.1.3-beta2": [
+      "[Added] Enable menu option to Force-push branches that have diverged - #15211",
+      "[Added] Add menu option to Fetch the current repository at any time - #7805",
+      "[Added] Support VSCodium as an external editor - #15348. Thanks @daniel-ciaglia!",
+      "[Fixed] Prevent closing the GitHub Desktop while it's being updated - #7055, #5197",
+      "[Fixed] Notifications are shown only when they are relevant to the current repository - #15487",
+      "[Fixed] Disable reorder, squashing, cherry-picking while an action of this type is in progress. - #15468",
+      "[Fixed] Fix repository change indicator not visible if selected and in focus - #7651. Thanks @angusdev!",
+      "[Fixed] Close 'Resolve conflicts before Rebase' dialog will not disable menu items - #13081. Thanks @angusdev!",
+      "[Fixed] Tooltips are positioned properly if mouse is not moved - #13636. Thanks @angusdev!",
+      "[Fixed] Fix tooltips of long commit author emails not breaking properly - #15424. Thanks @angusdev!",
+      "[Fixed] Clone repository progress bar no longer hidden by repository list - #11953. Thanks @angusdev!",
+      "[Fixed] Fix commit shortcut (Ctrl/Cmd + Enter) while amending a commit - #15445",
+      "[Improved] Pull request preview dialog width and height is responsive - #15500"
+    ],
     "3.1.3-beta1": ["[Improved] Upgrade embedded Git to 2.35.5"],
     "3.1.2": ["[Improved] Upgrade embedded Git to 2.35.5"],
     "3.1.2-beta1": [