Merge pull request desktop#19107 from desktop/always-announce-titles

Provide a TitleID for our dialogs with custom headers

app/src/ui/about/about.tsx

@@ -322,10 +322,12 @@ export class About extends React.Component<IAboutProps, IAboutState> {
     )
 
     const versionText = __DEV__ ? `Build ${version}` : `Version ${version}`
+    const titleId = 'Dialog_about'
 
     return (
       <Dialog
         id="about"
+        titleId={titleId}
         onSubmit={this.props.onDismissed}
         onDismissed={this.props.onDismissed}
       >
@@ -339,7 +341,7 @@ export class About extends React.Component<IAboutProps, IAboutState> {
               height="64"
             />
           </Row>
-          <h2>{name}</h2>
+          <h1 id={titleId}>About {name}</h1>
           <p className="no-padding">
             <span className="selectable-text">
               {versionText} ({this.props.applicationArchitecture})

app/src/ui/dialog/dialog.tsx

@@ -68,6 +68,16 @@ interface IDialogProps {
    */
   readonly title?: string | JSX.Element
 
+  /**
+   * Typically, a titleId is automatically generated based on the title
+   * attribute if it is a string. If it is not provided, we must assume the
+   * responsibility of providing a titleID that is used as the id of the h1 in
+   * the custom header and used in the aria attributes in this dialog component.
+   * By providing this titleID, the state.titleID will be set to this value and
+   * used in the aria attributes.
+   * */
+  readonly titleId?: string
+
   /**
    * An optional element to render to the right of the dialog title.
    * This can be used to render additional controls that don't belong to the
@@ -268,7 +278,7 @@ export class Dialog extends React.Component<DialogProps, IDialogState> {
 
   public constructor(props: DialogProps) {
     super(props)
-    this.state = { isAppearing: true }
+    this.state = { isAppearing: true, titleId: this.props.titleId }
 
     // Observe size changes and let codemirror know
     // when it needs to refresh.
@@ -346,6 +356,11 @@ export class Dialog extends React.Component<DialogProps, IDialogState> {
   }
 
   private updateTitleId() {
+    if (this.props.titleId) {
+      // Using the one provided that is used in a custom header
+      return
+    }
+
     if (this.state.titleId) {
       releaseUniqueId(this.state.titleId)
       this.setState({ titleId: undefined })

app/src/ui/open-pull-request/open-pull-request-dialog.tsx

@@ -9,7 +9,10 @@ import { Dispatcher } from '../dispatcher'
 import { Ref } from '../lib/ref'
 import { Octicon } from '../octicons'
 import * as octicons from '../octicons/octicons.generated'
-import { OpenPullRequestDialogHeader } from './open-pull-request-header'
+import {
+  OpenPullRequestDialogHeader,
+  OpenPullRequestDialogId,
+} from './open-pull-request-header'
 import { PullRequestFilesChanged } from './pull-request-files-changed'
 import { PullRequestMergeStatus } from './pull-request-merge-status'
 import { ComputedAction } from '../../models/computed-action'
@@ -274,6 +277,7 @@ export class OpenPullRequestDialog extends React.Component<IOpenPullRequestDialo
   public render() {
     return (
       <Dialog
+        titleId={OpenPullRequestDialogId}
         className="open-pull-request"
         onSubmit={this.onCreatePullRequest}
         onDismissed={this.props.onDismissed}

app/src/ui/open-pull-request/open-pull-request-header.tsx

@@ -2,9 +2,10 @@ import * as React from 'react'
 import { Branch } from '../../models/branch'
 import { BranchSelect } from '../branches/branch-select'
 import { DialogHeader } from '../dialog/header'
-import { createUniqueId, releaseUniqueId } from '../lib/id-pool'
 import { Ref } from '../lib/ref'
 
+export const OpenPullRequestDialogId = 'Dialog_Open_Pull_Request'
+
 interface IOpenPullRequestDialogHeaderProps {
   /** The base branch of the pull request */
   readonly baseBranch: Branch | null
@@ -46,31 +47,13 @@ interface IOpenPullRequestDialogHeaderProps {
   readonly onDismissed?: () => void
 }
 
-interface IOpenPullRequestDialogHeaderState {
-  /**
-   * An id for the h1 element that contains the title of this dialog. Used to
-   * aid in accessibility by allowing the h1 to be referenced in an
-   * aria-labeledby/aria-describedby attributed. Undefined if the dialog does
-   * not have a title or the component has not yet been mounted.
-   */
-  readonly titleId: string
-}
-
 /**
  * A header component for the open pull request dialog. Made to house the
  * base branch dropdown and merge details common to all pull request views.
  */
-export class OpenPullRequestDialogHeader extends React.Component<
-  IOpenPullRequestDialogHeaderProps,
-  IOpenPullRequestDialogHeaderState
-> {
+export class OpenPullRequestDialogHeader extends React.Component<IOpenPullRequestDialogHeaderProps> {
   public constructor(props: IOpenPullRequestDialogHeaderProps) {
     super(props)
-    this.state = { titleId: createUniqueId(`Dialog_Open_Pull_Request`) }
-  }
-
-  public componentWillUnmount() {
-    releaseUniqueId(this.state.titleId)
   }
 
   public render() {
@@ -90,7 +73,7 @@ export class OpenPullRequestDialogHeader extends React.Component<
     return (
       <DialogHeader
         title={title}
-        titleId={this.state.titleId}
+        titleId={OpenPullRequestDialogId}
         onCloseButtonClick={onDismissed}
       >
         <div className="break"></div>

app/styles/ui/_about.scss

@@ -5,10 +5,14 @@ dialog#about {
     }
 
     p,
-    h2 {
+    h1 {
       text-align: center;
     }
 
+    h1 {
+      font-size: var(--font-size-md);
+    }
+
     // The version, EULA link, and acknowledgements are conceptually two lines
     // in a paragraph but we don't have a great way of modelling that now
     // (and don't you say <br/>) so we'll just kill off the bottom margin.