Merge pull request desktop#19748 from desktop/confirm-users-knows-abo…

…ut-all-files-to-be-committed

Filtered Changes: Confirm users knows about all files to be committed

app/src/lib/app-state.ts

@@ -232,6 +232,9 @@ export interface IAppState {
   /** Should the app prompt the user to confirm an undo commit? */
   readonly askForConfirmationOnUndoCommit: boolean
 
+  /** Should the app prompt the user to confirm they want to commit with changes are hidden by filter? */
+  readonly askForConfirmationOnCommitFilteredChanges: boolean
+
   /** How the app should handle uncommitted changes when switching branches */
   readonly uncommittedChangesStrategy: UncommittedChangesStrategy
 

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

@@ -379,6 +379,7 @@ const confirmDiscardStashDefault: boolean = true
 const confirmCheckoutCommitDefault: boolean = true
 const askForConfirmationOnForcePushDefault = true
 const confirmUndoCommitDefault: boolean = true
+const confirmCommitFilteredChangesDefault: boolean = true
 const askToMoveToApplicationsFolderKey: string = 'askToMoveToApplicationsFolder'
 const confirmRepoRemovalKey: string = 'confirmRepoRemoval'
 const showCommitLengthWarningKey: string = 'showCommitLengthWarning'
@@ -389,6 +390,8 @@ const confirmDiscardChangesPermanentlyKey: string =
   'confirmDiscardChangesPermanentlyKey'
 const confirmForcePushKey: string = 'confirmForcePush'
 const confirmUndoCommitKey: string = 'confirmUndoCommit'
+const confirmCommitFilteredChangesKey: string =
+  'confirmCommitFilteredChangesKey'
 
 const uncommittedChangesStrategyKey = 'uncommittedChangesStrategyKind'
 
@@ -518,6 +521,8 @@ export class AppStore extends TypedBaseStore<IAppState> {
   private confirmCheckoutCommit: boolean = confirmCheckoutCommitDefault
   private askForConfirmationOnForcePush = askForConfirmationOnForcePushDefault
   private confirmUndoCommit: boolean = confirmUndoCommitDefault
+  private confirmCommitFilteredChanges: boolean =
+    confirmCommitFilteredChangesDefault
   private imageDiffType: ImageDiffType = imageDiffTypeDefault
   private hideWhitespaceInChangesDiff: boolean =
     hideWhitespaceInChangesDiffDefault
@@ -1041,6 +1046,8 @@ export class AppStore extends TypedBaseStore<IAppState> {
       askForConfirmationOnCheckoutCommit: this.confirmCheckoutCommit,
       askForConfirmationOnForcePush: this.askForConfirmationOnForcePush,
       askForConfirmationOnUndoCommit: this.confirmUndoCommit,
+      askForConfirmationOnCommitFilteredChanges:
+        this.confirmCommitFilteredChanges,
       uncommittedChangesStrategy: this.uncommittedChangesStrategy,
       selectedExternalEditor: this.selectedExternalEditor,
       imageDiffType: this.imageDiffType,
@@ -2211,6 +2218,11 @@ export class AppStore extends TypedBaseStore<IAppState> {
       confirmUndoCommitDefault
     )
 
+    this.confirmCommitFilteredChanges = getBoolean(
+      confirmCommitFilteredChangesKey,
+      confirmCommitFilteredChangesDefault
+    )
+
     this.uncommittedChangesStrategy =
       getEnum(uncommittedChangesStrategyKey, UncommittedChangesStrategy) ??
       defaultUncommittedChangesStrategy
@@ -5771,6 +5783,15 @@ export class AppStore extends TypedBaseStore<IAppState> {
     return Promise.resolve()
   }
 
+  public _setConfirmCommitFilteredChanges(value: boolean): Promise<void> {
+    this.confirmCommitFilteredChanges = value
+    setBoolean(confirmCommitFilteredChangesKey, value)
+
+    this.emitUpdate()
+
+    return Promise.resolve()
+  }
+
   public _setUncommittedChangesStrategySetting(
     value: UncommittedChangesStrategy
   ): Promise<void> {

app/src/models/popup.ts

@@ -95,6 +95,7 @@ export enum PopupType {
   PullRequestComment = 'PullRequestComment',
   UnknownAuthors = 'UnknownAuthors',
   TestIcons = 'TestIcons',
+  ConfirmCommitFilteredChanges = 'ConfirmCommitFilteredChanges',
 }
 
 interface IBasePopup {
@@ -423,5 +424,10 @@ export type PopupDetail =
   | {
       type: PopupType.TestIcons
     }
+  | {
+      type: PopupType.ConfirmCommitFilteredChanges
+      onCommitAnyway: () => void
+      onClearFilter: () => void
+    }
 
 export type Popup = IBasePopup & PopupDetail

app/src/ui/app.tsx

@@ -181,6 +181,7 @@ import { accessibilityBannerDismissed } from './banners/accessibilty-settings-ba
 import { isCertificateErrorSuppressedFor } from '../lib/suppress-certificate-error'
 import { webUtils } from 'electron'
 import { showTestUI } from './lib/test-ui-components/test-ui-components'
+import { ConfirmCommitFilteredChanges } from './changes/confirm-commit-filtered-changes-dialog'
 
 const MinuteInMilliseconds = 1000 * 60
 const HourInMilliseconds = MinuteInMilliseconds * 60
@@ -1557,6 +1558,9 @@ export class App extends React.Component<IAppProps, IAppState> {
             }
             confirmForcePush={this.state.askForConfirmationOnForcePush}
             confirmUndoCommit={this.state.askForConfirmationOnUndoCommit}
+            askForConfirmationOnCommitFilteredChanges={
+              this.state.askForConfirmationOnCommitFilteredChanges
+            }
             uncommittedChangesStrategy={this.state.uncommittedChangesStrategy}
             selectedExternalEditor={this.state.selectedExternalEditor}
             useWindowsOpenSSH={this.state.useWindowsOpenSSH}
@@ -2482,11 +2486,27 @@ export class App extends React.Component<IAppProps, IAppState> {
           />
         )
       }
+      case PopupType.ConfirmCommitFilteredChanges: {
+        return (
+          <ConfirmCommitFilteredChanges
+            onCommitAnyway={popup.onCommitAnyway}
+            onDismissed={onPopupDismissedFn}
+            onClearFilter={popup.onClearFilter}
+            setConfirmCommitFilteredChanges={
+              this.setConfirmCommitFilteredChanges
+            }
+          />
+        )
+      }
       default:
         return assertNever(popup, `Unknown popup type: ${popup}`)
     }
   }
 
+  private setConfirmCommitFilteredChanges = (value: boolean) => {
+    this.props.dispatcher.setConfirmCommitFilteredChanges(value)
+  }
+
   private getPullRequestState() {
     const { selectedState } = this.state
     if (
@@ -3225,6 +3245,9 @@ export class App extends React.Component<IAppProps, IAppState> {
           askForConfirmationOnCheckoutCommit={
             state.askForConfirmationOnCheckoutCommit
           }
+          askForConfirmationOnCommitFilteredChanges={
+            state.askForConfirmationOnCommitFilteredChanges
+          }
           accounts={state.accounts}
           isExternalEditorAvailable={
             state.useCustomEditor || state.selectedExternalEditor !== null

app/src/ui/changes/commit-message.tsx

@@ -69,11 +69,19 @@ const addAuthorIcon: OcticonSymbolVariant = {
   ],
 }
 
+interface ICreateCommitOptions {
+  warnUnknownAuthors: boolean
+  warnFilesNotVisible: boolean
+}
+
 interface ICommitMessageProps {
   readonly onCreateCommit: (context: ICommitContext) => Promise<boolean>
   readonly branch: string | null
   readonly commitAuthor: CommitIdentity | null
   readonly anyFilesSelected: boolean
+  /** Whether the user can see all the files to commit in the changes list. They
+   * may not be able to if the list is filtered */
+  readonly showPromptForCommittingFileHiddenByFilter?: boolean
   readonly isShowingModal: boolean
   readonly isShowingFoldout: boolean
 
@@ -161,7 +169,8 @@ interface ICommitMessageProps {
   readonly onCommitSpellcheckEnabledChanged: (enabled: boolean) => void
   readonly onStopAmending: () => void
   readonly onShowCreateForkDialog: () => void
-
+  readonly onFilesToCommitNotVisible?: (onCommitAnyway: () => {}) => void
+  readonly onSuccessfulCommitCreated?: () => void
   readonly accounts: ReadonlyArray<Account>
 }
 
@@ -498,26 +507,24 @@ export class CommitMessage extends React.Component<
       : this.state.summary
   }
 
-  private forceCreateCommit = async () => {
-    return this.createCommit(false)
-  }
-
-  private async createCommit(warnUnknownAuthors: boolean = true) {
+  private async createCommit(options?: ICreateCommitOptions) {
     const { description } = this.state
 
     if (!this.canCommit() && !this.canAmend()) {
       return
     }
 
-    if (warnUnknownAuthors) {
+    if (options?.warnUnknownAuthors !== false) {
       const unknownAuthors = this.props.coAuthors.filter(
         (author): author is UnknownAuthor => !isKnownAuthor(author)
       )
 
       if (unknownAuthors.length > 0) {
-        this.props.onConfirmCommitWithUnknownCoAuthors(
-          unknownAuthors,
-          this.forceCreateCommit
+        this.props.onConfirmCommitWithUnknownCoAuthors(unknownAuthors, () =>
+          this.createCommit({
+            warnUnknownAuthors: false,
+            warnFilesNotVisible: options?.warnFilesNotVisible === true,
+          })
         )
         return
       }
@@ -532,11 +539,26 @@ export class CommitMessage extends React.Component<
       amend: this.props.commitToAmend !== null,
     }
 
+    if (
+      options?.warnFilesNotVisible !== false &&
+      this.props.showPromptForCommittingFileHiddenByFilter === true &&
+      this.props.onFilesToCommitNotVisible
+    ) {
+      this.props.onFilesToCommitNotVisible(() =>
+        this.createCommit({
+          warnUnknownAuthors: options?.warnUnknownAuthors === true,
+          warnFilesNotVisible: false,
+        })
+      )
+      return
+    }
+
     const timer = startTimer('create commit', this.props.repository)
     const commitCreated = await this.props.onCreateCommit(commitContext)
     timer.done()
 
     if (commitCreated) {
+      this.props.onSuccessfulCommitCreated?.()
       this.clearCommitMessage()
     }
   }

app/src/ui/changes/confirm-commit-filtered-changes-dialog.tsx

@@ -0,0 +1,92 @@
+import * as React from 'react'
+import { Dialog, DialogContent, DialogFooter } from '../dialog'
+import { Row } from '../lib/row'
+import { OkCancelButtonGroup } from '../dialog/ok-cancel-button-group'
+import { Checkbox, CheckboxValue } from '../lib/checkbox'
+
+interface IConfirmCommitFilteredChangesProps {
+  readonly onCommitAnyway: () => void
+  readonly onDismissed: () => void
+  readonly onClearFilter: () => void
+  readonly setConfirmCommitFilteredChanges: (value: boolean) => void
+}
+
+interface IConfirmCommitFilteredChangesState {
+  readonly askForConfirmationOnCommitFilteredChanges: boolean
+}
+
+export class ConfirmCommitFilteredChanges extends React.Component<
+  IConfirmCommitFilteredChangesProps,
+  IConfirmCommitFilteredChangesState
+> {
+  public constructor(props: IConfirmCommitFilteredChangesProps) {
+    super(props)
+
+    this.state = {
+      askForConfirmationOnCommitFilteredChanges: true,
+    }
+  }
+
+  public render() {
+    return (
+      <Dialog
+        id="hidden-changes"
+        type="warning"
+        title={
+          __DARWIN__ ? 'Commit Filtered Changes?' : 'Commit filtered changes?'
+        }
+        onSubmit={this.onSubmit}
+        onDismissed={this.props.onDismissed}
+        role="alertdialog"
+        ariaDescribedBy="confirm-commit-filtered-changes-message"
+      >
+        <DialogContent>
+          <p id="confirm-commit-filtered-changes-message">
+            You have a filter applied. There are changes that will be committed
+            hidden from view. Are you sure you want to commit these changes?
+          </p>
+          <Row>
+            <Checkbox
+              label="Do not show this message again"
+              value={
+                this.state.askForConfirmationOnCommitFilteredChanges
+                  ? CheckboxValue.Off
+                  : CheckboxValue.On
+              }
+              onChange={this.onShowMessageChange}
+            />
+          </Row>
+        </DialogContent>
+        <DialogFooter>
+          <OkCancelButtonGroup
+            destructive={true}
+            okButtonText={__DARWIN__ ? 'Commit Anyway' : 'Commit anyway'}
+            cancelButtonText={
+              __DARWIN__ ? 'Cancel and Clear Filter' : 'Cancel and clear filter'
+            }
+            onCancelButtonClick={this.onClearFilter}
+          />
+        </DialogFooter>
+      </Dialog>
+    )
+  }
+
+  private onShowMessageChange = (event: React.FormEvent<HTMLInputElement>) => {
+    const value = !event.currentTarget.checked
+
+    this.setState({ askForConfirmationOnCommitFilteredChanges: value })
+  }
+
+  private onClearFilter = () => {
+    this.props.onClearFilter()
+    this.props.onDismissed()
+  }
+
+  private onSubmit = () => {
+    this.props.setConfirmCommitFilteredChanges(
+      this.state.askForConfirmationOnCommitFilteredChanges
+    )
+    this.props.onCommitAnyway()
+    this.props.onDismissed()
+  }
+}