Merge pull request #17538 from desktop/commit-summary-expansion-move-expander-to-title

Commit Summary Expansion: Move expander to title

app/src/ui/history/expandable-commit-summary.tsx

@@ -18,6 +18,7 @@ import { LinkButton } from '../lib/link-button'
 import { UnreachableCommitsTab } from './unreachable-commits-dialog'
 import { TooltippedCommitSHA } from '../lib/tooltipped-commit-sha'
 import memoizeOne from 'memoize-one'
+import { Button } from '../lib/button'
 
 interface IExpandableCommitSummaryProps {
   readonly repository: Repository
@@ -221,22 +222,25 @@ export class ExpandableCommitSummary extends React.Component<
   }
 
   private renderExpander() {
-    if (
-      !this.state.body.length ||
-      (!this.props.isExpanded && !this.state.isOverflowed)
-    ) {
+    const { selectedCommits, isExpanded } = this.props
+    if (selectedCommits.length > 1) {
       return null
     }
 
-    const expanded = this.props.isExpanded
-    const onClick = expanded ? this.onCollapse : this.onExpand
-    const icon = expanded ? OcticonSymbol.fold : OcticonSymbol.unfold
-
     return (
-      <button onClick={onClick} className="expander">
-        <Octicon symbol={icon} />
-        {expanded ? 'Collapse' : 'Expand'}
-      </button>
+      <Button
+        onClick={isExpanded ? this.onCollapse : this.onExpand}
+        className="expander"
+        tooltip={isExpanded ? 'Collapse' : 'Expand'}
+        ariaExpanded={isExpanded}
+        ariaLabel={
+          isExpanded ? 'Collapse commit details' : 'Expand commit details'
+        }
+      >
+        <Octicon
+          symbol={isExpanded ? OcticonSymbol.fold : OcticonSymbol.unfold}
+        />
+      </Button>
     )
   }
 
@@ -323,8 +327,6 @@ export class ExpandableCommitSummary extends React.Component<
             text={this.state.body}
           />
         </div>
-
-        {this.renderExpander()}
       </div>
     )
   }
@@ -423,17 +425,13 @@ export class ExpandableCommitSummary extends React.Component<
     )
   }
 
-  private renderSummary = () => {
+  private renderSummaryText = () => {
     const { selectedCommits, shasInDiff } = this.props
-    const { summary, hasEmptySummary } = this.state
-    const summaryClassNames = classNames('commit-summary-title', {
-      'empty-summary': hasEmptySummary,
-    })
+    const { summary } = this.state
 
     if (selectedCommits.length === 1) {
       return (
         <RichText
-          className={summaryClassNames}
           emoji={this.props.emoji}
           repository={this.props.repository}
           text={summary}
@@ -447,11 +445,13 @@ export class ExpandableCommitSummary extends React.Component<
     )
     const numInDiff = selectedCommits.length - commitsNotInDiff
     const commitsPluralized = numInDiff > 1 ? 'commits' : 'commit'
+
     return (
-      <div className={summaryClassNames}>
+      <>
         Showing changes from{' '}
         {commitsNotInDiff > 0 ? (
           <LinkButton
+            className="commits-in-diff"
             onMouseOver={this.onHighlightShasInDiff}
             onMouseOut={this.onRemoveHighlightOfShas}
             onClick={this.showReachableCommits}
@@ -464,6 +464,20 @@ export class ExpandableCommitSummary extends React.Component<
             {numInDiff} {commitsPluralized}
           </>
         )}
+      </>
+    )
+  }
+
+  private renderSummary = () => {
+    const { hasEmptySummary } = this.state
+    const summaryClassNames = classNames('ecs-title', {
+      'empty-summary': hasEmptySummary,
+    })
+
+    return (
+      <div className={summaryClassNames}>
+        {this.renderSummaryText()}
+        {this.renderExpander()}
       </div>
     )
   }
@@ -471,7 +485,6 @@ export class ExpandableCommitSummary extends React.Component<
   public render() {
     const className = classNames('expandable-commit-summary', {
       expanded: this.props.isExpanded,
-      collapsed: !this.props.isExpanded,
       'has-expander': this.props.isExpanded || this.state.isOverflowed,
       'hide-description-border': this.props.hideDescriptionBorder,
     })

app/styles/ui/history/_expandable-commit-summary.scss

@@ -10,25 +10,6 @@
     height: 16px;
   }
 
-  .expander {
-    position: absolute;
-    width: 75px;
-    top: var(--spacing);
-    right: var(--spacing);
-    background: transparent;
-    padding: 0;
-    border: none;
-    color: var(--text-color);
-    font-size: inherit;
-    font-weight: normal;
-    font-family: inherit;
-
-    svg {
-      vertical-align: text-top;
-      margin-right: var(--spacing-half);
-    }
-  }
-
   &.expanded {
     .commit-summary-description-scroll-view {
       max-height: 400px;
@@ -83,7 +64,7 @@
     }
   }
 
-  .commit-summary-title,
+  .ecs-title,
   .commit-summary-meta {
     padding: var(--spacing);
 
@@ -96,16 +77,37 @@
     }
   }
 
-  .commit-summary-title {
+  .ecs-title {
     font-size: var(--font-size-md);
     font-weight: var(--font-weight-semibold);
     line-height: 16px;
     padding: var(--spacing);
     word-wrap: break-word;
+    display: flex;
+
+    .commits-in-diff {
+      margin-left: var(--spacing-half);
+    }
 
     &.empty-summary {
       color: var(--text-secondary-color);
     }
+
+    .expander {
+      background: transparent;
+      padding: 0;
+      border: none;
+      color: var(--text-color);
+      font-size: inherit;
+      font-weight: normal;
+      font-family: inherit;
+      margin-left: var(--spacing);
+      height: auto;
+
+      svg {
+        vertical-align: text-top;
+      }
+    }
   }
 
   .commit-summary-description-container {
@@ -125,7 +127,7 @@
     -webkit-line-clamp: 3;
   }
 
-  .commit-summary-title,
+  .ecs-title,
   .commit-summary-description {
     // Enable text selection inside the title and description elements.
     user-select: text;
@@ -187,4 +189,14 @@
   .commit-summary-header {
     border-bottom: var(--base-border);
   }
+
+  &.expanded {
+    .commit-summary-meta {
+      display: block;
+
+      .commit-summary-meta-item {
+        margin-bottom: var(--spacing-half);
+      }
+    }
+  }
 }