Merge pull request #12129 from desktop/hide-whitespace-fixes

app/src/ui/diff/diff-options.tsx

@@ -6,6 +6,7 @@ import { getBoolean, setBoolean } from '../../lib/local-storage'
 import { Popover, PopoverCaretPosition } from '../lib/popover'
 import { enableHideWhitespaceInDiffOption } from '../../lib/feature-flag'
 import { RepositorySectionTab } from '../../lib/app-state'
+import { HideWhitespaceWarning } from './hide-whitespace-warning'
 
 interface IDiffOptionsProps {
   readonly sourceTab: RepositorySectionTab
@@ -163,6 +164,9 @@ export class DiffOptions extends React.Component<
             __DARWIN__ ? 'Hide Whitespace Changes' : 'Hide whitespace changes'
           }
         />
+        {this.props.sourceTab === RepositorySectionTab.Changes && (
+          <p className="secondary-text">{HideWhitespaceWarning}</p>
+        )}
       </section>
     )
   }

app/src/ui/diff/hide-whitespace-warning.ts

@@ -0,0 +1,2 @@
+export const HideWhitespaceWarning =
+  'Interacting with individual lines or hunks will be disabled while hiding whitespace.'

app/src/ui/diff/index.tsx

@@ -246,12 +246,16 @@ export class Diff extends React.Component<IDiffProps, IDiffState> {
   }
 
   private renderTextDiff(diff: ITextDiff) {
+    const hideWhitespaceInDiff =
+      enableHideWhitespaceInDiffOption() && this.props.hideWhitespaceInDiff
+
     if (enableExperimentalDiffViewer() || this.props.showSideBySideDiff) {
       return (
         <SideBySideDiff
           repository={this.props.repository}
           file={this.props.file}
           diff={diff}
+          hideWhitespaceInDiff={hideWhitespaceInDiff}
           showSideBySideDiff={this.props.showSideBySideDiff}
           onIncludeChanged={this.props.onIncludeChanged}
           onDiscardChanges={this.props.onDiscardChanges}
@@ -262,9 +266,6 @@ export class Diff extends React.Component<IDiffProps, IDiffState> {
       )
     }
 
-    const hideWhitespaceInDiff =
-      enableHideWhitespaceInDiffOption() && this.props.hideWhitespaceInDiff
-
     return (
       <TextDiff
         repository={this.props.repository}

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

@@ -14,6 +14,7 @@ import { narrowNoNewlineSymbol } from './text-diff'
 import { shallowEquals, structuralEquals } from '../../lib/equality'
 import { DiffHunkExpansionType } from '../../models/diff'
 import { DiffExpansionKind } from './text-diff-expansion'
+import { HideWhitespaceWarning } from './hide-whitespace-warning'
 
 interface ISideBySideDiffRowProps {
   /**
@@ -36,6 +37,9 @@ interface ISideBySideDiffRowProps {
    */
   readonly showSideBySideDiff: boolean
 
+  /** Whether or not whitespace changes are hidden. */
+  readonly hideWhitespaceInDiff: boolean
+
   /**
    * The index of the row in the displayed diff.
    */
@@ -362,9 +366,13 @@ export class SideBySideDiffRow extends React.Component<
       return null
     }
 
+    const classes = classNames('hunk-handle', {
+      hoverable: !this.props.hideWhitespaceInDiff,
+    })
+
     return (
       <div
-        className="hunk-handle"
+        className={classes}
         onMouseEnter={this.onMouseEnterHunk}
         onMouseLeave={this.onMouseLeaveHunk}
         onClick={this.onClickHunk}
@@ -399,8 +407,12 @@ export class SideBySideDiffRow extends React.Component<
       <div
         className={classNames('line-number', 'selectable', {
           'line-selected': isSelected,
+          hoverable: !this.props.hideWhitespaceInDiff,
           hover: this.props.isHunkHovered,
         })}
+        title={
+          this.props.hideWhitespaceInDiff ? HideWhitespaceWarning : undefined
+        }
         onMouseDown={this.onMouseDownLineNumber}
         onContextMenu={this.onContextMenuLineNumber}
       >
@@ -473,7 +485,7 @@ export class SideBySideDiffRow extends React.Component<
   }
 
   private onMouseDownLineNumber = (evt: React.MouseEvent) => {
-    if (evt.buttons === 2) {
+    if (evt.buttons === 2 || this.props.hideWhitespaceInDiff) {
       return
     }
 
@@ -486,6 +498,10 @@ export class SideBySideDiffRow extends React.Component<
   }
 
   private onMouseEnterLineNumber = (evt: React.MouseEvent) => {
+    if (this.props.hideWhitespaceInDiff) {
+      return
+    }
+
     const data = this.getDiffData(evt.currentTarget)
     const column = this.getDiffColumn(evt.currentTarget)
 
@@ -495,12 +511,20 @@ export class SideBySideDiffRow extends React.Component<
   }
 
   private onMouseEnterHunk = () => {
+    if (this.props.hideWhitespaceInDiff) {
+      return
+    }
+
     if ('hunkStartLine' in this.props.row) {
       this.props.onMouseEnterHunk(this.props.row.hunkStartLine)
     }
   }
 
   private onMouseLeaveHunk = () => {
+    if (this.props.hideWhitespaceInDiff) {
+      return
+    }
+
     if ('hunkStartLine' in this.props.row) {
       this.props.onMouseLeaveHunk(this.props.row.hunkStartLine)
     }
@@ -511,6 +535,10 @@ export class SideBySideDiffRow extends React.Component<
   }
 
   private onClickHunk = () => {
+    if (this.props.hideWhitespaceInDiff) {
+      return
+    }
+
     // Since the hunk handler lies between the previous and the next columns,
     // when clicking on it on modified lines we cannot know if we should
     // use the state of the previous or the next line to know whether we should
@@ -525,6 +553,10 @@ export class SideBySideDiffRow extends React.Component<
   }
 
   private onContextMenuLineNumber = (evt: React.MouseEvent) => {
+    if (this.props.hideWhitespaceInDiff) {
+      return
+    }
+
     const data = this.getDiffData(evt.currentTarget)
     if (data !== null && data.diffLineNumber !== null) {
       this.props.onContextMenuLine(data.diffLineNumber)
@@ -532,6 +564,10 @@ export class SideBySideDiffRow extends React.Component<
   }
 
   private onContextMenuHunk = () => {
+    if (this.props.hideWhitespaceInDiff) {
+      return
+    }
+
     if ('hunkStartLine' in this.props.row) {
       this.props.onContextMenuHunk(this.props.row.hunkStartLine)
     }

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

@@ -97,6 +97,9 @@ interface ISideBySideDiffProps {
     diffSelection: DiffSelection
   ) => void
 
+  /** Whether or not whitespace changes are hidden. */
+  readonly hideWhitespaceInDiff: boolean
+
   /**
    * Whether we'll show a confirmation dialog when the user
    * discards changes.
@@ -317,6 +320,7 @@ export class SideBySideDiff extends React.Component<
             isDiffSelectable={canSelect(this.props.file)}
             isHunkHovered={isHunkHovered}
             showSideBySideDiff={this.props.showSideBySideDiff}
+            hideWhitespaceInDiff={this.props.hideWhitespaceInDiff}
             onStartSelection={this.onStartSelection}
             onUpdateSelection={this.onUpdateSelection}
             onMouseEnterHunk={this.onMouseEnterHunk}
@@ -720,13 +724,17 @@ export class SideBySideDiff extends React.Component<
    * @param diffLineNumber the line number the diff where the user clicked
    */
   private onContextMenuLine = (diffLineNumber: number) => {
-    const { file } = this.props
+    const { file, hideWhitespaceInDiff } = this.props
     const { diff } = this.state
 
     if (!canSelect(file)) {
       return
     }
 
+    if (hideWhitespaceInDiff) {
+      return
+    }
+
     if (this.props.onDiscardChanges === undefined) {
       return
     }

app/src/ui/diff/text-diff.tsx

@@ -52,6 +52,7 @@ import {
   expandWholeTextDiff,
 } from './text-diff-expansion'
 import { createOcticonElement } from '../octicons/octicon'
+import { HideWhitespaceWarning } from './hide-whitespace-warning'
 
 /** The longest line for which we'd try to calculate a line diff. */
 const MaxIntraLineDiffStringLength = 4096
@@ -1246,8 +1247,7 @@ export class TextDiff extends React.Component<ITextDiffProps, ITextDiffState> {
     }
 
     if (this.props.hideWhitespaceInDiff) {
-      marker.title =
-        'Partial committing is not available while hiding whitespace changes'
+      marker.title = HideWhitespaceWarning
     }
 
     const hunkExpandWholeHandle = marker.getElementsByClassName(

app/styles/ui/_diff-options.scss

@@ -42,6 +42,10 @@
     align-items: center;
   }
 
+  .secondary-text {
+    color: var(--text-secondary-color);
+  }
+
   h3 {
     display: flex;
     flex-direction: row;

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

@@ -74,7 +74,7 @@
     }
 
     &.selectable {
-      span {
+      &.hoverable span {
         cursor: pointer;
       }
 
@@ -83,7 +83,7 @@
         color: var(--diff-selected-text-color);
         border-color: var(--diff-selected-border-color);
 
-        &:hover,
+        &.hoverable:hover,
         &.hover {
           background: var(--diff-hover-background-color);
           color: var(--diff-hover-text-color);
@@ -111,7 +111,7 @@
       padding: 4px 0;
     }
 
-    &.selectable {
+    &.selectable.hoverable {
       * {
         cursor: pointer;
       }
@@ -130,7 +130,10 @@
     width: 20px;
     left: calc(50% - 10px);
     top: 0;
-    cursor: pointer;
+
+    &.hoverable {
+      cursor: pointer;
+    }
   }
 
   .content {
@@ -177,7 +180,7 @@
       border-color: var(--diff-delete-border-color);
     }
 
-    .selectable:hover,
+    .selectable.hoverable:hover,
     .selectable.hover {
       background: var(--diff-delete-hover-background-color);
       color: var(--diff-delete-hover-text-color);
@@ -194,7 +197,7 @@
       border-color: var(--diff-add-border-color);
     }
 
-    .selectable:hover,
+    .selectable.hoverable:hover,
     .selectable.hover {
       background: var(--diff-add-hover-background-color);
       color: var(--diff-add-hover-text-color);