Merge pull request #8813 from desktop/ku-preferences-redesign

Preferences redesign

app/src/lib/app-state.ts

@@ -39,6 +39,7 @@ import { GitRebaseProgress } from '../models/rebase'
 import { RebaseFlowStep } from '../models/rebase-flow-step'
 import { IStashEntry } from '../models/stash-entry'
 import { TutorialStep } from '../models/tutorial-step'
+import { UncommittedChangesStrategyKind } from '../models/uncommitted-changes-strategy'
 
 export enum SelectionType {
   Repository,
@@ -174,6 +175,9 @@ export interface IAppState {
   /** Should the app prompt the user to confirm a force push? */
   readonly askForConfirmationOnForcePush: boolean
 
+  /** How the app should handle uncommitted changes when switching branches */
+  readonly uncommittedChangesStrategyKind: UncommittedChangesStrategyKind
+
   /** The external editor to use when opening repositories */
   readonly selectedExternalEditor: ExternalEditor | null
 

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

@@ -234,7 +234,10 @@ import {
 import {
   UncommittedChangesStrategy,
   UncommittedChangesStrategyKind,
+  uncommittedChangesStrategyKindDefault,
+  getUncommittedChangesStrategy,
   askToStash,
+  parseStrategy,
 } from '../../models/uncommitted-changes-strategy'
 import { IStashEntry, StashedChangesLoadStates } from '../../models/stash-entry'
 import { RebaseFlowStep, RebaseStep } from '../../models/rebase-flow-step'
@@ -281,6 +284,9 @@ const confirmRepoRemovalKey: string = 'confirmRepoRemoval'
 const confirmDiscardChangesKey: string = 'confirmDiscardChanges'
 const confirmForcePushKey: string = 'confirmForcePush'
 
+const uncommittedChangesStrategyKindKey: string =
+  'uncommittedChangesStrategyKind'
+
 const externalEditorKey: string = 'externalEditor'
 
 const imageDiffTypeDefault = ImageDiffType.TwoUp
@@ -361,6 +367,8 @@ export class AppStore extends TypedBaseStore<IAppState> {
   private imageDiffType: ImageDiffType = imageDiffTypeDefault
   private hideWhitespaceInDiff: boolean = hideWhitespaceInDiffDefault
 
+  private uncommittedChangesStrategyKind: UncommittedChangesStrategyKind = uncommittedChangesStrategyKindDefault
+
   private selectedExternalEditor: ExternalEditor | null = null
 
   private resolvedExternalEditor: ExternalEditor | null = null
@@ -696,6 +704,7 @@ export class AppStore extends TypedBaseStore<IAppState> {
         .askForConfirmationOnRepositoryRemoval,
       askForConfirmationOnDiscardChanges: this.confirmDiscardChanges,
       askForConfirmationOnForcePush: this.askForConfirmationOnForcePush,
+      uncommittedChangesStrategyKind: this.uncommittedChangesStrategyKind,
       selectedExternalEditor: this.selectedExternalEditor,
       imageDiffType: this.imageDiffType,
       hideWhitespaceInDiff: this.hideWhitespaceInDiff,
@@ -1769,6 +1778,12 @@ export class AppStore extends TypedBaseStore<IAppState> {
       askForConfirmationOnForcePushDefault
     )
 
+    const strategy = parseStrategy(
+      localStorage.getItem(uncommittedChangesStrategyKindKey)
+    )
+    this.uncommittedChangesStrategyKind =
+      strategy || uncommittedChangesStrategyKindDefault
+
     this.updateSelectedExternalEditor(
       await this.lookupSelectedExternalEditor()
     ).catch(e => log.error('Failed resolving current editor at startup', e))
@@ -2944,7 +2959,9 @@ export class AppStore extends TypedBaseStore<IAppState> {
     repository: Repository,
     name: string,
     startPoint: string | null,
-    uncommittedChangesStrategy: UncommittedChangesStrategy = askToStash
+    uncommittedChangesStrategy: UncommittedChangesStrategy = getUncommittedChangesStrategy(
+      this.uncommittedChangesStrategyKind
+    )
   ): Promise<Repository> {
     const gitStore = this.gitStoreCache.get(repository)
     const branch = await gitStore.performFailableOperation(() =>
@@ -3014,7 +3031,9 @@ export class AppStore extends TypedBaseStore<IAppState> {
   public async _checkoutBranch(
     repository: Repository,
     branch: Branch | string,
-    uncommittedChangesStrategy: UncommittedChangesStrategy = askToStash
+    uncommittedChangesStrategy: UncommittedChangesStrategy = getUncommittedChangesStrategy(
+      this.uncommittedChangesStrategyKind
+    )
   ): Promise<Repository> {
     const gitStore = this.gitStoreCache.get(repository)
     const kind = 'checkout'
@@ -3034,20 +3053,22 @@ export class AppStore extends TypedBaseStore<IAppState> {
     let stashToPop: IStashEntry | null = null
     if (enableStashing()) {
       const hasChanges = changesState.workingDirectory.files.length > 0
-      if (hasChanges && uncommittedChangesStrategy.kind === askToStash.kind) {
-        this._showPopup({
-          type: PopupType.StashAndSwitchBranch,
-          branchToCheckout: foundBranch,
+      if (hasChanges) {
+        if (uncommittedChangesStrategy.kind === askToStash.kind) {
+          this._showPopup({
+            type: PopupType.StashAndSwitchBranch,
+            branchToCheckout: foundBranch,
+            repository,
+          })
+          return repository
+        }
+
+        stashToPop = await this.stashToPopAfterBranchCheckout(
           repository,
-        })
-        return repository
+          foundBranch,
+          uncommittedChangesStrategy
+        )
       }
-
-      stashToPop = await this.stashToPopAfterBranchCheckout(
-        repository,
-        foundBranch,
-        uncommittedChangesStrategy
-      )
     }
 
     const checkoutSucceeded =
@@ -3137,7 +3158,9 @@ export class AppStore extends TypedBaseStore<IAppState> {
   private async stashToPopAfterBranchCheckout(
     repository: Repository,
     branch: Branch,
-    uncommittedChangesStrategy: UncommittedChangesStrategy = askToStash
+    uncommittedChangesStrategy: UncommittedChangesStrategy = getUncommittedChangesStrategy(
+      this.uncommittedChangesStrategyKind
+    )
   ): Promise<IStashEntry | null> {
     const {
       changesState,
@@ -4422,6 +4445,17 @@ export class AppStore extends TypedBaseStore<IAppState> {
     return Promise.resolve()
   }
 
+  public _setUncommittedChangesStrategyKindSetting(
+    value: UncommittedChangesStrategyKind
+  ): Promise<void> {
+    this.uncommittedChangesStrategyKind = value
+
+    localStorage.setItem(uncommittedChangesStrategyKindKey, value)
+
+    this.emitUpdate()
+    return Promise.resolve()
+  }
+
   public _setExternalEditor(selectedEditor: ExternalEditor) {
     const promise = this.updateSelectedExternalEditor(selectedEditor)
     localStorage.setItem(externalEditorKey, selectedEditor)

app/src/models/preferences.ts

@@ -1,6 +1,7 @@
 export enum PreferencesTab {
   Accounts = 0,
-  Git = 1,
-  Appearance = 2,
-  Advanced = 3,
+  Integrations = 1,
+  Git = 2,
+  Appearance = 3,
+  Advanced = 4,
 }

app/src/models/uncommitted-changes-strategy.ts

@@ -1,11 +1,15 @@
 import { IStashEntry } from './stash-entry'
+import { assertNever } from '../lib/fatal-error'
 
 export enum UncommittedChangesStrategyKind {
   AskForConfirmation = 'AskForConfirmation',
   StashOnCurrentBranch = 'StashOnCurrentBranch',
   MoveToNewBranch = 'MoveToNewBranch',
 }
 
+export const uncommittedChangesStrategyKindDefault: UncommittedChangesStrategyKind =
+  UncommittedChangesStrategyKind.AskForConfirmation
+
 export type UncommittedChangesStrategy =
   | { kind: UncommittedChangesStrategyKind.AskForConfirmation }
   | { kind: UncommittedChangesStrategyKind.StashOnCurrentBranch }
@@ -20,3 +24,51 @@ export const askToStash: UncommittedChangesStrategy = {
 export const stashOnCurrentBranch: UncommittedChangesStrategy = {
   kind: UncommittedChangesStrategyKind.StashOnCurrentBranch,
 }
+export const moveToNewBranch: UncommittedChangesStrategy = {
+  kind: UncommittedChangesStrategyKind.MoveToNewBranch,
+  transientStashEntry: null,
+}
+
+/**
+ * Used to convert a `UncommittedChangesStrategyKind` into a
+ * `UncommittedChangesStrategy` object. For example, the
+ * user's preference is stored as a kind in state, which
+ * must be translated into a strategy before it can be
+ * used in stashing logic and methods.
+ */
+export function getUncommittedChangesStrategy(
+  kind: UncommittedChangesStrategyKind
+): UncommittedChangesStrategy {
+  switch (kind) {
+    case UncommittedChangesStrategyKind.AskForConfirmation:
+      return askToStash
+    case UncommittedChangesStrategyKind.MoveToNewBranch:
+      return moveToNewBranch
+    case UncommittedChangesStrategyKind.StashOnCurrentBranch:
+      return stashOnCurrentBranch
+    default:
+      return assertNever(
+        kind,
+        `Unknown UncommittedChangesStrategyKind: ${kind}`
+      )
+  }
+}
+
+/**
+ * Parse a string into a valid `UncommittedChangesStrategyKind`,
+ * if possible. Returns `null` if not.
+ */
+export function parseStrategy(
+  strategy: string | null
+): UncommittedChangesStrategyKind | null {
+  switch (strategy) {
+    case UncommittedChangesStrategyKind.AskForConfirmation:
+      return UncommittedChangesStrategyKind.AskForConfirmation
+    case UncommittedChangesStrategyKind.StashOnCurrentBranch:
+      return UncommittedChangesStrategyKind.StashOnCurrentBranch
+    case UncommittedChangesStrategyKind.MoveToNewBranch:
+      return UncommittedChangesStrategyKind.MoveToNewBranch
+    default:
+      return null
+  }
+}

app/src/ui/app.tsx

@@ -108,6 +108,7 @@ import { enableTutorial } from '../lib/feature-flag'
 import { ConfirmExitTutorial } from './tutorial'
 import { TutorialStep, isValidTutorialStep } from '../models/tutorial-step'
 import { WorkflowPushRejectedDialog } from './workflow-push-rejected/workflow-push-rejected'
+import { getUncommittedChangesStrategy } from '../models/uncommitted-changes-strategy'
 import { SAMLReauthRequiredDialog } from './saml-reauth-required/saml-reauth-required'
 
 const MinuteInMilliseconds = 1000 * 60
@@ -1329,6 +1330,9 @@ export class App extends React.Component<IAppProps, IAppState> {
               this.state.askForConfirmationOnDiscardChanges
             }
             confirmForcePush={this.state.askForConfirmationOnForcePush}
+            uncommittedChangesStrategyKind={
+              this.state.uncommittedChangesStrategyKind
+            }
             selectedExternalEditor={this.state.selectedExternalEditor}
             optOutOfUsageTracking={this.state.optOutOfUsageTracking}
             enterpriseAccount={this.getEnterpriseAccount()}
@@ -1444,6 +1448,9 @@ export class App extends React.Component<IAppProps, IAppState> {
             dispatcher={this.props.dispatcher}
             initialName={popup.initialName || ''}
             currentBranchProtected={currentBranchProtected}
+            selectedUncommittedChangesStrategy={getUncommittedChangesStrategy(
+              this.state.uncommittedChangesStrategyKind
+            )}
           />
         )
       }
@@ -2330,7 +2337,9 @@ export class App extends React.Component<IAppProps, IAppState> {
       currentFoldout !== null && currentFoldout.type === FoldoutType.Branch
 
     const repository = selection.repository
-    const branchesState = selection.state.branchesState
+    const { branchesState, changesState } = selection.state
+    const hasAssociatedStash = changesState.stashEntry !== null
+    const hasChanges = changesState.workingDirectory.files.length > 0
 
     return (
       <BranchDropdown
@@ -2346,6 +2355,10 @@ export class App extends React.Component<IAppProps, IAppState> {
         shouldNudge={
           this.state.currentOnboardingTutorialStep === TutorialStep.CreateBranch
         }
+        selectedUncommittedChangesStrategy={getUncommittedChangesStrategy(
+          this.state.uncommittedChangesStrategyKind
+        )}
+        couldOverwriteStash={hasChanges && hasAssociatedStash}
       />
     )
   }

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

@@ -25,7 +25,7 @@ import { startTimer } from '../lib/timing'
 import {
   UncommittedChangesStrategyKind,
   UncommittedChangesStrategy,
-  askToStash,
+  stashOnCurrentBranch,
 } from '../../models/uncommitted-changes-strategy'
 import memoizeOne from 'memoize-one'
 
@@ -46,6 +46,10 @@ interface IBranchesContainerProps {
   readonly isLoadingPullRequests: boolean
 
   readonly currentBranchProtected: boolean
+
+  readonly selectedUncommittedChangesStrategy: UncommittedChangesStrategy
+
+  readonly couldOverwriteStash: boolean
 }
 
 interface IBranchesContainerState {
@@ -249,9 +253,29 @@ export class BranchesContainer extends React.Component<
   private onBranchItemClick = (branch: Branch) => {
     this.props.dispatcher.closeFoldout(FoldoutType.Branch)
 
-    const { currentBranch, repository, currentBranchProtected } = this.props
+    const {
+      currentBranch,
+      repository,
+      currentBranchProtected,
+      dispatcher,
+      couldOverwriteStash,
+    } = this.props
 
     if (currentBranch == null || currentBranch.name !== branch.name) {
+      if (
+        !currentBranchProtected &&
+        this.props.selectedUncommittedChangesStrategy.kind ===
+          stashOnCurrentBranch.kind &&
+        couldOverwriteStash
+      ) {
+        dispatcher.showPopup({
+          type: PopupType.ConfirmOverwriteStash,
+          repository,
+          branchToCheckout: branch,
+        })
+        return
+      }
+
       const timer = startTimer('checkout branch from list', repository)
 
       // Never prompt to stash changes if someone is switching away from a protected branch
@@ -260,7 +284,7 @@ export class BranchesContainer extends React.Component<
             kind: UncommittedChangesStrategyKind.MoveToNewBranch,
             transientStashEntry: null,
           }
-        : askToStash
+        : this.props.selectedUncommittedChangesStrategy
 
       this.props.dispatcher
         .checkoutBranch(repository, branch, strategy)

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

@@ -27,7 +27,6 @@ import { startTimer } from '../lib/timing'
 import {
   UncommittedChangesStrategy,
   UncommittedChangesStrategyKind,
-  askToStash,
 } from '../../models/uncommitted-changes-strategy'
 
 interface ICreateBranchProps {
@@ -39,6 +38,7 @@ interface ICreateBranchProps {
   readonly allBranches: ReadonlyArray<Branch>
   readonly initialName: string
   readonly currentBranchProtected: boolean
+  readonly selectedUncommittedChangesStrategy: UncommittedChangesStrategy
 }
 
 interface ICreateBranchState {
@@ -303,7 +303,7 @@ export class CreateBranch extends React.Component<
             kind: UncommittedChangesStrategyKind.MoveToNewBranch,
             transientStashEntry: null,
           }
-        : askToStash
+        : this.props.selectedUncommittedChangesStrategy
 
       this.setState({ isCreatingBranch: true })
       const timer = startTimer('create branch', repository)

app/src/ui/dispatcher/dispatcher.ts

@@ -82,7 +82,10 @@ import {
   StatusCallBack,
 } from '../../lib/stores/commit-status-store'
 import { MergeResult } from '../../models/merge'
-import { UncommittedChangesStrategy } from '../../models/uncommitted-changes-strategy'
+import {
+  UncommittedChangesStrategy,
+  UncommittedChangesStrategyKind,
+} from '../../models/uncommitted-changes-strategy'
 import { RebaseFlowStep, RebaseStep } from '../../models/rebase-flow-step'
 import { IStashEntry } from '../../models/stash-entry'
 
@@ -1477,6 +1480,15 @@ export class Dispatcher {
     return this.appStore._setConfirmDiscardChangesSetting(value)
   }
 
+  /**
+   * Sets the user's preference for handling uncommitted changes when switching branches
+   */
+  public setUncommittedChangesStrategyKindSetting(
+    value: UncommittedChangesStrategyKind
+  ): Promise<void> {
+    return this.appStore._setUncommittedChangesStrategyKindSetting(value)
+  }
+
   /**
    * Sets the user's preference for an external program to open repositories in.
    */