Merge pull request desktop#17787 from desktop/settings-radio-button-announcments

tidy-dev · web-flow · commit 35005966469c · 2023-12-06T12:00:49.000Z
Grouping our settings radio and checkbox groups.
diff --git a/app/src/ui/lib/radio-group.tsx b/app/src/ui/lib/radio-group.tsx
@@ -28,7 +28,7 @@ interface IRadioGroupProps<T> {
   readonly onSelectionChanged: (key: T) => void
 
   /** Render radio button label contents */
-  readonly renderRadioButtonLabelContents: (key: T) => JSX.Element
+  readonly renderRadioButtonLabelContents: (key: T) => JSX.Element | string
 }
 
 /**
diff --git a/app/src/ui/lib/ref-name-text-box.tsx b/app/src/ui/lib/ref-name-text-box.tsx
@@ -19,6 +19,11 @@ interface IRefNameProps {
    */
   readonly label?: string | JSX.Element
 
+  /**
+   * The aria-labelledBy attribute for the text box.
+   */
+  readonly ariaLabelledBy?: string
+
   /**
    * The aria-describedby attribute for the text box.
    */
@@ -89,6 +94,7 @@ export class RefNameTextBox extends React.Component<
           label={this.props.label}
           value={this.state.proposedValue}
           ref={this.textBoxRef}
+          ariaLabelledBy={this.props.ariaLabelledBy}
           ariaDescribedBy={this.props.ariaDescribedBy}
           onValueChanged={this.onValueChange}
           onBlur={this.onBlur}
diff --git a/app/src/ui/lib/text-box.tsx b/app/src/ui/lib/text-box.tsx
@@ -93,6 +93,9 @@ export interface ITextBoxProps {
   /** Optional aria-label attribute */
   readonly ariaLabel?: string
 
+  /** Optional aria-labelledby attribute */
+  readonly ariaLabelledBy?: string
+
   /** Optional aria-describedby attribute - usually for associating a descriptive
    * message to the input such as a validation error, warning, or caption */
   readonly ariaDescribedBy?: string
@@ -312,6 +315,7 @@ export class TextBox extends React.Component<ITextBoxProps, ITextBoxState> {
           onContextMenu={this.onContextMenu}
           spellCheck={this.props.spellcheck === true}
           aria-label={this.props.ariaLabel}
+          aria-labelledby={this.props.ariaLabelledBy}
           aria-controls={this.props.ariaControls}
           aria-describedby={this.props.ariaDescribedBy}
           required={this.props.required}
diff --git a/app/src/ui/preferences/git.tsx b/app/src/ui/preferences/git.tsx
@@ -3,10 +3,12 @@ import { DialogContent } from '../dialog'
 import { SuggestedBranchNames } from '../../lib/helpers/default-branch'
 import { RefNameTextBox } from '../lib/ref-name-text-box'
 import { Ref } from '../lib/ref'
-import { RadioButton } from '../lib/radio-button'
 import { LinkButton } from '../lib/link-button'
 import { Account } from '../../models/account'
 import { GitConfigUserForm } from '../lib/git-config-user-form'
+import { RadioGroup } from '../lib/radio-group'
+
+const otherOption = 'Other…'
 
 interface IGitProps {
   readonly name: string
@@ -118,32 +120,36 @@ export class Git extends React.Component<IGitProps, IGitState> {
     )
   }
 
+  private renderBranchNameOption = (branchName: string) => {
+    return branchName === otherOption ? (
+      <span id="other-branch-name-label">{branchName}</span>
+    ) : (
+      branchName
+    )
+  }
+
   private renderDefaultBranchSetting() {
     const { defaultBranchIsOther } = this.state
 
+    const branchNameOptions = [...SuggestedBranchNames, otherOption]
+    const selectedKey = defaultBranchIsOther
+      ? otherOption
+      : SuggestedBranchNames.find(n => n === this.props.defaultBranch) ??
+        SuggestedBranchNames.at(0) ??
+        otherOption // Should never happen, but TypeScript doesn't know that.
+
     return (
       <div className="default-branch-component">
-        <h2>Default branch name for new repositories</h2>
-
-        {SuggestedBranchNames.map((branchName: string, i: number) => (
-          <RadioButton
-            key={branchName}
-            checked={
-              (!defaultBranchIsOther &&
-                this.props.defaultBranch === branchName) ||
-              (this.props.isLoadingGitConfig && i === 0)
-            }
-            value={branchName}
-            label={branchName}
-            onSelected={this.onDefaultBranchChanged}
-          />
-        ))}
-        <RadioButton
-          key={OtherNameForDefaultBranch}
-          checked={defaultBranchIsOther}
-          value={OtherNameForDefaultBranch}
-          label="Other…"
-          onSelected={this.onDefaultBranchChanged}
+        <h2 id="default-branch-heading">
+          Default branch name for new repositories
+        </h2>
+
+        <RadioGroup<string>
+          ariaLabelledBy="default-branch-heading"
+          selectedKey={selectedKey}
+          radioButtonKeys={branchNameOptions}
+          onSelectionChanged={this.onDefaultBranchChanged}
+          renderRadioButtonLabelContents={this.renderBranchNameOption}
         />
 
         {defaultBranchIsOther && (
@@ -152,6 +158,7 @@ export class Git extends React.Component<IGitProps, IGitState> {
             renderWarningMessage={this.renderWarningMessage}
             onValueChange={this.props.onDefaultBranchChanged}
             ref={this.defaultBranchInputRef}
+            ariaLabelledBy={'other-branch-name-label'}
           />
         )}
 
@@ -186,7 +193,9 @@ export class Git extends React.Component<IGitProps, IGitState> {
       defaultBranchIsOther: !SuggestedBranchNames.includes(defaultBranch),
     })
 
-    this.props.onDefaultBranchChanged(defaultBranch)
+    this.props.onDefaultBranchChanged(
+      defaultBranch === otherOption ? '' : defaultBranch
+    )
   }
 
   // This function is called to open the global git config file in the
diff --git a/app/src/ui/preferences/prompts.tsx b/app/src/ui/preferences/prompts.tsx
@@ -2,7 +2,8 @@ import * as React from 'react'
 import { UncommittedChangesStrategy } from '../../models/uncommitted-changes-strategy'
 import { DialogContent } from '../dialog'
 import { Checkbox, CheckboxValue } from '../lib/checkbox'
-import { RadioButton } from '../lib/radio-button'
+import { RadioGroup } from '../lib/radio-group'
+import { assertNever } from '../../lib/fatal-error'
 
 interface IPromptsPreferencesProps {
   readonly confirmRepositoryRemoval: boolean
@@ -134,106 +135,121 @@ export class Prompts extends React.Component<
     this.props.onShowCommitLengthWarningChanged(event.currentTarget.checked)
   }
 
+  private renderSwitchBranchOptionLabel = (key: UncommittedChangesStrategy) => {
+    switch (key) {
+      case UncommittedChangesStrategy.AskForConfirmation:
+        return 'Ask me where I want the changes to go'
+      case UncommittedChangesStrategy.MoveToNewBranch:
+        return 'Always bring my changes to my new branch'
+      case UncommittedChangesStrategy.StashOnCurrentBranch:
+        return 'Always stash and leave my changes on the current branch'
+      default:
+        return assertNever(key, `Unknown uncommitted changes strategy: ${key}`)
+    }
+  }
+
+  private renderSwitchBranchOptions = () => {
+    const options = [
+      UncommittedChangesStrategy.AskForConfirmation,
+      UncommittedChangesStrategy.MoveToNewBranch,
+      UncommittedChangesStrategy.StashOnCurrentBranch,
+    ]
+
+    const selectedKey =
+      options.find(o => o === this.state.uncommittedChangesStrategy) ??
+      UncommittedChangesStrategy.AskForConfirmation
+
+    return (
+      <div className="advanced-section">
+        <h2 id="switch-branch-heading">
+          If I have changes and I switch branches...
+        </h2>
+
+        <RadioGroup<UncommittedChangesStrategy>
+          ariaLabelledBy="switch-branch-heading"
+          selectedKey={selectedKey}
+          radioButtonKeys={options}
+          onSelectionChanged={this.onUncommittedChangesStrategyChanged}
+          renderRadioButtonLabelContents={this.renderSwitchBranchOptionLabel}
+        />
+      </div>
+    )
+  }
+
   public render() {
     return (
       <DialogContent>
         <div className="advanced-section">
-          <h2>Show a confirmation dialog before...</h2>
-          <Checkbox
-            label="Removing repositories"
-            value={
-              this.state.confirmRepositoryRemoval
-                ? CheckboxValue.On
-                : CheckboxValue.Off
-            }
-            onChange={this.onConfirmRepositoryRemovalChanged}
-          />
-          <Checkbox
-            label="Discarding changes"
-            value={
-              this.state.confirmDiscardChanges
-                ? CheckboxValue.On
-                : CheckboxValue.Off
-            }
-            onChange={this.onConfirmDiscardChangesChanged}
-          />
-          <Checkbox
-            label="Discarding changes permanently"
-            value={
-              this.state.confirmDiscardChangesPermanently
-                ? CheckboxValue.On
-                : CheckboxValue.Off
-            }
-            onChange={this.onConfirmDiscardChangesPermanentlyChanged}
-          />
-          <Checkbox
-            label="Discarding stash"
-            value={
-              this.state.confirmDiscardStash
-                ? CheckboxValue.On
-                : CheckboxValue.Off
-            }
-            onChange={this.onConfirmDiscardStashChanged}
-          />
-          <Checkbox
-            label="Checking out a commit"
-            value={
-              this.state.confirmCheckoutCommit
-                ? CheckboxValue.On
-                : CheckboxValue.Off
-            }
-            onChange={this.onConfirmCheckoutCommitChanged}
-          />
-          <Checkbox
-            label="Force pushing"
-            value={
-              this.state.confirmForcePush ? CheckboxValue.On : CheckboxValue.Off
-            }
-            onChange={this.onConfirmForcePushChanged}
-          />
-          <Checkbox
-            label="Undo commit"
-            value={
-              this.state.confirmUndoCommit
-                ? CheckboxValue.On
-                : CheckboxValue.Off
-            }
-            onChange={this.onConfirmUndoCommitChanged}
-          />
-        </div>
-        <div className="advanced-section">
-          <h2>If I have changes and I switch branches...</h2>
-
-          <RadioButton
-            value={UncommittedChangesStrategy.AskForConfirmation}
-            checked={
-              this.state.uncommittedChangesStrategy ===
-              UncommittedChangesStrategy.AskForConfirmation
-            }
-            label="Ask me where I want the changes to go"
-            onSelected={this.onUncommittedChangesStrategyChanged}
-          />
-
-          <RadioButton
-            value={UncommittedChangesStrategy.MoveToNewBranch}
-            checked={
-              this.state.uncommittedChangesStrategy ===
-              UncommittedChangesStrategy.MoveToNewBranch
-            }
-            label="Always bring my changes to my new branch"
-            onSelected={this.onUncommittedChangesStrategyChanged}
-          />
-
-          <RadioButton
-            value={UncommittedChangesStrategy.StashOnCurrentBranch}
-            checked={
-              this.state.uncommittedChangesStrategy ===
-              UncommittedChangesStrategy.StashOnCurrentBranch
-            }
-            label="Always stash and leave my changes on the current branch"
-            onSelected={this.onUncommittedChangesStrategyChanged}
-          />
+          <h2 id="show-confirm-dialog-heading">
+            Show a confirmation dialog before...
+          </h2>
+          <div role="group" aria-labelledby="show-confirm-dialog-heading">
+            <Checkbox
+              label="Removing repositories"
+              value={
+                this.state.confirmRepositoryRemoval
+                  ? CheckboxValue.On
+                  : CheckboxValue.Off
+              }
+              onChange={this.onConfirmRepositoryRemovalChanged}
+            />
+            <Checkbox
+              label="Discarding changes"
+              value={
+                this.state.confirmDiscardChanges
+                  ? CheckboxValue.On
+                  : CheckboxValue.Off
+              }
+              onChange={this.onConfirmDiscardChangesChanged}
+            />
+            <Checkbox
+              label="Discarding changes permanently"
+              value={
+                this.state.confirmDiscardChangesPermanently
+                  ? CheckboxValue.On
+                  : CheckboxValue.Off
+              }
+              onChange={this.onConfirmDiscardChangesPermanentlyChanged}
+            />
+            <Checkbox
+              label="Discarding stash"
+              value={
+                this.state.confirmDiscardStash
+                  ? CheckboxValue.On
+                  : CheckboxValue.Off
+              }
+              onChange={this.onConfirmDiscardStashChanged}
+            />
+            <Checkbox
+              label="Checking out a commit"
+              value={
+                this.state.confirmCheckoutCommit
+                  ? CheckboxValue.On
+                  : CheckboxValue.Off
+              }
+              onChange={this.onConfirmCheckoutCommitChanged}
+            />
+            <Checkbox
+              label="Force pushing"
+              value={
+                this.state.confirmForcePush
+                  ? CheckboxValue.On
+                  : CheckboxValue.Off
+              }
+              onChange={this.onConfirmForcePushChanged}
+            />
+            <Checkbox
+              label="Undo commit"
+              value={
+                this.state.confirmUndoCommit
+                  ? CheckboxValue.On
+                  : CheckboxValue.Off
+              }
+              onChange={this.onConfirmUndoCommitChanged}
+            />
+          </div>
         </div>
+        {this.renderSwitchBranchOptions()}
         <div className="advanced-section">
           <h2>Commit Length</h2>
           <Checkbox
diff --git a/app/src/ui/repository-settings/fork-settings.tsx b/app/src/ui/repository-settings/fork-settings.tsx
diff --git a/app/src/ui/repository-settings/git-config.tsx b/app/src/ui/repository-settings/git-config.tsx

Original file line number	Diff line number	Diff line change
`@@ -28,7 +28,7 @@ interface IRadioGroupProps<T> {`
`28`	`28`	`readonly onSelectionChanged: (key: T) => void`
`29`	`29`
`30`	`30`	`/** Render radio button label contents */`
`31`		`- readonly renderRadioButtonLabelContents: (key: T) => JSX.Element`
	`31`	`+ readonly renderRadioButtonLabelContents: (key: T) => JSX.Element \| string`
`32`	`32`	`}`
`33`	`33`
`34`	`34`	`/**`