Merge pull request #18204 from desktop/visual-improvements-on-checkall

Visual improvements on check all

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

@@ -18,7 +18,7 @@ import { PopoverAnchorPosition } from '../lib/popover'
 import { WhitespaceHintPopover } from './whitespace-hint-popover'
 import { TooltipDirection } from '../lib/tooltip'
 import { Button } from '../lib/button'
-import { diffCheck } from '../octicons/diff-check'
+import { diffCheck, diffDash } from '../octicons/diff'
 import {
   enableDiffCheckMarks,
   enableGroupDiffCheckmarks,
@@ -52,11 +52,6 @@ export interface IRowSelectableGroup {
    */
   isHovered: boolean
 
-  /**
-   * Whether or not the group is focused by the keyboard
-   */
-  isFocused: boolean
-
   /**
    * The selection state of the group - 'All', 'Partial', or 'None'
    */
@@ -637,6 +632,8 @@ export class SideBySideDiffRow extends React.Component<
             : false
         }
         onChange={this.onClickHunk}
+        onFocus={this.onHunkFocus}
+        onBlur={this.onHunkBlur}
       />
     )
 
@@ -658,7 +655,7 @@ export class SideBySideDiffRow extends React.Component<
       return <Octicon symbol={diffCheck} />
     }
     if (selectionState === DiffSelectionType.Partial) {
-      return <Octicon symbol={octicons.dash} />
+      return <Octicon symbol={diffDash} />
     }
 
     return null
@@ -906,6 +903,18 @@ export class SideBySideDiffRow extends React.Component<
     }
   }
 
+  private onHunkFocus = () => {
+    if ('hunkStartLine' in this.props.row) {
+      this.props.onMouseEnterHunk(this.props.row.hunkStartLine)
+    }
+  }
+
+  private onHunkBlur = () => {
+    if ('hunkStartLine' in this.props.row) {
+      this.props.onMouseLeaveHunk(this.props.row.hunkStartLine)
+    }
+  }
+
   private onExpandHunk =
     (hunkIndex: number, kind: DiffHunkExpansionType) => () => {
       this.props.onExpandHunk(hunkIndex, kind)

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

@@ -663,7 +663,6 @@ export class SideBySideDiff extends React.Component<
       isFirst: prev === undefined || !selectableType.includes(prev.type),
       isLast: next === undefined || !selectableType.includes(next.type),
       isHovered: hoveredHunk === row.hunkStartLine,
-      isFocused: true, // focusedHunk === row.hunkStartLine, - To be added in later PR
       selectionState: selection.isRangeSelected(from, to - from + 1),
       height: this.getRowSelectableGroupHeight(row.hunkStartLine),
       lineNumbers: Array.from(lineNumbers),

app/src/ui/octicons/diff.ts

@@ -0,0 +1,26 @@
+import { OcticonSymbolVariant } from '.'
+
+/**
+ * An check mark produced by Gavin that is scaled for 12x12 as opposed to the
+ * regular 16x16 and is thicker for better visibility.
+ */
+export const diffCheck: OcticonSymbolVariant = {
+  w: 12,
+  h: 12,
+  p: [
+    'M10.5303 2.96967C10.8232 3.26256 10.8232 3.73744 10.5303 4.03033L5.03033 9.53033C4.73744 9.82322 4.26256 9.82322 3.96967 9.53033L1.46967 7.03033C1.17678 6.73744 1.17678 6.26256 1.46967 5.96967C1.76256 5.67678 2.23744 5.67678 2.53033 5.96967L4.5 7.93934L9.46967 2.96967C9.76256 2.67678 10.2374 2.67678 10.5303 2.96967Z',
+  ],
+}
+
+/**
+ * An dash for the "mixed" state of the diff hunk handle check all produced by
+ * Gavin that is scaled for 12x12 as opposed to the regular 16x16 and is thicker
+ * for better visibility.
+ */
+export const diffDash: OcticonSymbolVariant = {
+  w: 12,
+  h: 12,
+  p: [
+    'm1.3125 6c0-.41421.33579-.75.75-.75h7.875c.4142 0 .75.33579.75.75s-.3358.75-.75.75h-7.875c-.41421 0-.75-.33579-.75-.75z',
+  ],
+}

app/styles/_variables.scss

@@ -363,6 +363,7 @@ $overlay-background-color: rgba(0, 0, 0, 0.4);
 
   /** The highlight color used for focus rings and focus box shadows */
   --focus-color: #{$blue};
+  --diff-linenumber-focus-color: #{$blue-600};
 
   /**
    * Variables for form elements

app/styles/themes/_dark.scss

@@ -269,6 +269,7 @@ body.theme-dark {
 
   /** The highlight color used for focus rings and focus box shadows */
   --focus-color: #{$blue};
+  --diff-linenumber-focus-color: #{$blue-200};
 
   /**
    * Variables for form elements

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

@@ -33,18 +33,19 @@
       }
 
       &.context .before {
-        margin-right: calc(var(--hunk-handle-width) / 2);
+        border-right: calc(var(--hunk-handle-width) / 2) solid var(--diff-border-color);
       }
+
       &.context .after {
-        margin-right: calc(var(--hunk-handle-width) / 2);
+        border-right: calc(var(--hunk-handle-width) / 2) solid var(--diff-border-color);
       }
 
       &.has-check-all-control.context .before {
-        margin-right: calc(var(--hunk-handle-width-with-check-all) / 2);
+        border-right: calc(var(--hunk-handle-width-with-check-all) / 2) solid var(--diff-border-color);
       }
 
       &.has-check-all-control.context .after {
-        margin-left: calc(var(--hunk-handle-width-with-check-all) / 2);
+        border-left: calc(var(--hunk-handle-width-with-check-all) / 2) solid var(--diff-border-color);
       }
 
       &.has-check-all-control {
@@ -58,14 +59,6 @@
         padding-left: calc(var(--hunk-handle-width) / 2);
         padding-right: calc(var(--hunk-handle-width) / 2);
       }
-
-      &.context {
-        margin-left: var(--hunk-handle-width);
-      }
-
-      &.has-check-all-control.context {
-        margin-left: var(--hunk-handle-width-with-check-all);
-      }
     }
   }
 
@@ -188,9 +181,13 @@
       }
     }
 
+    input:focus-visible + label.hunk-handle.selected {
+      background: var(--diff-hover-background-color);
+    }
+
     input:focus-visible + label.hunk-handle {
       .focus-handle {
-        outline: 2px solid #0659d6;
+        outline: 2px solid var(--diff-linenumber-focus-color);
         outline-offset: 3px;
       }
     }
@@ -216,6 +213,7 @@
         width: 100%;
         margin-top: -4px;
         padding-top: 3px;
+        cursor: pointer;
       }
 
       .octicon {
@@ -239,7 +237,6 @@
 
       input:focus-visible + label.hunk-handle {
         .focus-handle {
-          outline: 2px solid white;
           outline-offset: -3px;
         }
       }
@@ -351,7 +348,7 @@
 
       &.selectable {
         input:focus-visible + label {
-          outline: 2px solid #0659d6;
+          outline: 2px solid var(--diff-linenumber-focus-color);
           outline-offset: -3px;
         }
 
@@ -470,6 +467,23 @@
         }
       }
     }
+
+    &.editable {
+      .row {
+        &.context .before,
+        &.context .after {
+          border-right: none;
+        }
+
+        &.context {
+          border-left: var(--hunk-handle-width) solid var(--diff-border-color);
+        }
+
+        &.has-check-all-control.context {
+          border-left: var(--hunk-handle-width-with-check-all) solid var(--diff-border-color);
+        }
+      }
+    }
   }
 
   .diff-search {