Merge pull request #18084 from desktop/eol-changes-warning-banner

Use banner to announce line endings change warning in diffs

app/src/ui/diff/diff-contents-warning.tsx

@@ -0,0 +1,90 @@
+import React from 'react'
+import { Octicon } from '../octicons'
+import * as OcticonSymbol from '../octicons/octicons.generated'
+import { LinkButton } from '../lib/link-button'
+import { ITextDiff, LineEndingsChange } from '../../models/diff'
+
+enum DiffContentsWarningType {
+  UnicodeBidiCharacters,
+  LineEndingsChange,
+}
+
+type DiffContentsWarningItem =
+  | {
+      readonly type: DiffContentsWarningType.UnicodeBidiCharacters
+    }
+  | {
+      readonly type: DiffContentsWarningType.LineEndingsChange
+      readonly lineEndingsChange: LineEndingsChange
+    }
+
+interface IDiffContentsWarningProps {
+  readonly diff: ITextDiff
+}
+
+export class DiffContentsWarning extends React.Component<IDiffContentsWarningProps> {
+  public render() {
+    const items = this.getTextDiffWarningItems()
+
+    if (items.length === 0) {
+      return null
+    }
+
+    return (
+      <div className="diff-contents-warning-container">
+        {items.map((item, i) => (
+          <div className="diff-contents-warning" key={i}>
+            <Octicon symbol={OcticonSymbol.alert} />
+            {this.getWarningMessageForItem(item)}
+          </div>
+        ))}
+      </div>
+    )
+  }
+
+  private getTextDiffWarningItems(): ReadonlyArray<DiffContentsWarningItem> {
+    const items = new Array<DiffContentsWarningItem>()
+    const { diff } = this.props
+
+    if (diff.hasHiddenBidiChars) {
+      items.push({
+        type: DiffContentsWarningType.UnicodeBidiCharacters,
+      })
+    }
+
+    if (diff.lineEndingsChange) {
+      items.push({
+        type: DiffContentsWarningType.LineEndingsChange,
+        lineEndingsChange: diff.lineEndingsChange,
+      })
+    }
+
+    return items
+  }
+
+  private getWarningMessageForItem(item: DiffContentsWarningItem) {
+    switch (item.type) {
+      case DiffContentsWarningType.UnicodeBidiCharacters:
+        return (
+          <>
+            This diff contains bidirectional Unicode text that may be
+            interpreted or compiled differently than what appears below. To
+            review, open the file in an editor that reveals hidden Unicode
+            characters.{' '}
+            <LinkButton uri="https://github.co/hiddenchars">
+              Learn more about bidirectional Unicode characters
+            </LinkButton>
+          </>
+        )
+
+      case DiffContentsWarningType.LineEndingsChange:
+        const { lineEndingsChange } = item
+        return (
+          <>
+            This diff contains a change in line endings from '
+            {lineEndingsChange.from}' to '{lineEndingsChange.to}'.
+          </>
+        )
+    }
+  }
+}

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

@@ -3,7 +3,6 @@ import { PathLabel } from '../lib/path-label'
 import { AppFileStatus } from '../../models/status'
 import { IDiff, DiffType } from '../../models/diff'
 import { Octicon, iconForStatus } from '../octicons'
-import * as OcticonSymbol from '../octicons/octicons.generated'
 import { mapStatus } from '../../lib/status'
 import { DiffOptions } from './diff-options'
 
@@ -37,7 +36,6 @@ export class DiffHeader extends React.Component<IDiffHeaderProps, {}> {
     return (
       <div className="header">
         <PathLabel path={this.props.path} status={this.props.status} />
-        {this.renderDecorator()}
 
         {this.renderDiffOptions()}
 
@@ -68,25 +66,4 @@ export class DiffHeader extends React.Component<IDiffHeaderProps, {}> {
       />
     )
   }
-
-  private renderDecorator() {
-    const diff = this.props.diff
-
-    if (diff === null) {
-      return null
-    }
-
-    if (diff.kind === DiffType.Text && diff.lineEndingsChange) {
-      const message = `Warning: line endings will be changed from '${diff.lineEndingsChange.from}' to '${diff.lineEndingsChange.to}'.`
-      return (
-        <Octicon
-          symbol={OcticonSymbol.alert}
-          className={'line-endings'}
-          title={message}
-        />
-      )
-    } else {
-      return null
-    }
-  }
 }

app/src/ui/diff/hidden-bidi-chars-warning.tsx

@@ -1,20 +0,0 @@
-import React from 'react'
-import { Octicon } from '../octicons'
-import * as OcticonSymbol from '../octicons/octicons.generated'
-import { LinkButton } from '../lib/link-button'
-
-export class HiddenBidiCharsWarning extends React.Component {
-  public render() {
-    return (
-      <div className="hidden-bidi-chars-warning">
-        <Octicon symbol={OcticonSymbol.alert} />
-        This diff contains bidirectional Unicode text that may be interpreted or
-        compiled differently than what appears below. To review, open the file
-        in an editor that reveals hidden Unicode characters.{' '}
-        <LinkButton uri="https://github.co/hiddenchars">
-          Learn more about bidirectional Unicode characters
-        </LinkButton>
-      </div>
-    )
-  }
-}

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

@@ -60,7 +60,7 @@ import {
   expandWholeTextDiff,
 } from './text-diff-expansion'
 import { IMenuItem } from '../../lib/menu-item'
-import { HiddenBidiCharsWarning } from './hidden-bidi-chars-warning'
+import { DiffContentsWarning } from './diff-contents-warning'
 import { findDOMNode } from 'react-dom'
 import escapeRegExp from 'lodash/escapeRegExp'
 
@@ -537,7 +537,7 @@ export class SideBySideDiff extends React.Component<
         onMouseDown={this.onMouseDown}
         onKeyDown={this.onKeyDown}
       >
-        {diff.hasHiddenBidiChars && <HiddenBidiCharsWarning />}
+        <DiffContentsWarning diff={diff} />
         {this.state.isSearching && (
           <DiffSearchInput
             onSearch={this.onSearch}

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

@@ -56,7 +56,7 @@ import { createOcticonElement } from '../octicons/octicon'
 import * as OcticonSymbol from '../octicons/octicons.generated'
 import { WhitespaceHintPopover } from './whitespace-hint-popover'
 import { PopoverAnchorPosition } from '../lib/popover'
-import { HiddenBidiCharsWarning } from './hidden-bidi-chars-warning'
+import { DiffContentsWarning } from './diff-contents-warning'
 
 // This is a custom version of the no-newline octicon that's exactly as
 // tall as it needs to be (8px) which helps with aligning it on the line.
@@ -1556,7 +1556,7 @@ export class TextDiff extends React.Component<ITextDiffProps, ITextDiffState> {
 
     return (
       <>
-        {diff.hasHiddenBidiChars && <HiddenBidiCharsWarning />}
+        <DiffContentsWarning diff={diff} />
         <CodeMirrorHost
           className="diff-code-mirror"
           value={doc}

app/styles/ui/_diff.scss

@@ -802,7 +802,7 @@
     pointer-events: none;
   }
 
-  .hidden-bidi-chars-warning {
+  .diff-contents-warning-container {
     background-color: var(--file-warning-background-color);
     padding: var(--spacing) var(--spacing-double);
     border-bottom: var(--file-warning-border-color) solid 1px;
@@ -816,6 +816,13 @@
     a.link-button-component {
       display: unset;
     }
+
+    // Separate warning items
+    .diff-contents-warning:not(:last-child) {
+      margin-bottom: var(--spacing);
+      border-bottom: var(--file-warning-border-color) solid 1px;
+      padding-bottom: var(--spacing);
+    }
   }
 
   // When loading a diff