Merge pull request desktop#17062 from desktop/diff-settings-a11y

Improve accessibility of diff options button
shiftkey · Jul 21, 2023 · afb5c16 · afb5c16
2 parents cb740de + 44f627f
commit afb5c16
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 14 deletions.
diff --git a/app/src/ui/diff/diff-options.tsx b/app/src/ui/diff/diff-options.tsx
@@ -8,6 +8,8 @@ import {
   PopoverAnchorPosition,
   PopoverDecoration,
 } from '../lib/popover'
+import { Tooltip, TooltipDirection } from '../lib/tooltip'
+import { createObservableRef } from '../lib/observable-ref'
 
 interface IDiffOptionsProps {
   readonly isInteractiveDiff: boolean
@@ -31,6 +33,7 @@ export class DiffOptions extends React.Component<
   IDiffOptionsProps,
   IDiffOptionsState
 > {
+  private innerButtonRef = createObservableRef<HTMLButtonElement>()
   private diffOptionsRef = React.createRef<HTMLDivElement>()
   private gearIconRef = React.createRef<HTMLSpanElement>()
 
@@ -79,9 +82,21 @@ export class DiffOptions extends React.Component<
   }
 
   public render() {
+    const buttonLabel = `Diff ${__DARWIN__ ? 'Settings' : 'Options'}`
     return (
       <div className="diff-options-component" ref={this.diffOptionsRef}>
-        <button onClick={this.onButtonClick}>
+        <button
+          aria-label={buttonLabel}
+          onClick={this.onButtonClick}
+          aria-expanded={this.state.isPopoverOpen}
+          ref={this.innerButtonRef}
+        >
+          <Tooltip
+            target={this.innerButtonRef}
+            direction={TooltipDirection.NORTH}
+          >
+            {buttonLabel}
+          </Tooltip>
           <span ref={this.gearIconRef}>
             <Octicon symbol={OcticonSymbol.gear} />
           </span>
@@ -119,8 +134,8 @@ export class DiffOptions extends React.Component<
 
   private renderShowSideBySide() {
     return (
-      <section>
-        <h4>Diff display</h4>
+      <fieldset role="radiogroup">
+        <legend>Diff display</legend>
         <RadioButton
           value="Unified"
           checked={!this.props.showSideBySideDiff}
@@ -137,14 +152,14 @@ export class DiffOptions extends React.Component<
           }
           onSelected={this.onSideBySideSelected}
         />
-      </section>
+      </fieldset>
     )
   }
 
   private renderHideWhitespaceChanges() {
     return (
-      <section>
-        <h4>Whitespace</h4>
+      <fieldset>
+        <legend>Whitespace</legend>
         <Checkbox
           value={
             this.props.hideWhitespaceChanges
@@ -162,7 +177,7 @@ export class DiffOptions extends React.Component<
             hiding whitespace.
           </p>
         )}
-      </section>
+      </fieldset>
     )
   }
 }
diff --git a/app/src/ui/history/commit-summary.tsx b/app/src/ui/history/commit-summary.tsx
@@ -504,10 +504,7 @@ export class CommitSummary extends React.Component<
             {this.renderLinesChanged()}
             {this.renderTags()}
 
-            <li
-              className="commit-summary-meta-item without-truncation"
-              title="Diff Options"
-            >
+            <li className="commit-summary-meta-item without-truncation">
               <DiffOptions
                 isInteractiveDiff={false}
                 hideWhitespaceChanges={this.props.hideWhitespaceInDiff}

diff --git a/app/styles/ui/_diff-options.scss b/app/styles/ui/_diff-options.scss
@@ -56,11 +56,20 @@
     align-items: center;
   }
 
-  section + section {
-    margin-top: var(--spacing);
+  legend {
+    margin-top: 0px;
+    margin-bottom: 0.5rem;
+    font-weight: bold;
+    padding: 0px;
   }
 
-  section.button-group {
+  fieldset {
+    border: none;
+    margin: 0px;
+    padding: 0px;
+  }
+
+  fieldset.button-group {
     display: flex;
     flex-direction: row;
   }