Merge pull request #18035 from desktop/jc-appearance-menu-underline

Add support for toggling link underlines globally

app/src/lib/app-state.ts

@@ -340,6 +340,8 @@ export interface IAppState {
    * rulesets to check their bypass status.
    */
   readonly cachedRepoRulesets: ReadonlyMap<number, IAPIRepoRuleset>
+
+  readonly underlineLinks: boolean
 }
 
 export enum FoldoutType {

app/src/lib/feature-flag.ts

@@ -106,3 +106,5 @@ export const enableDiffCheckMarksAndLinkUnderlines = enableBetaFeatures
 
 export const enableDiffCheckMarks = enableDiffCheckMarksAndLinkUnderlines
 export const enableGroupDiffCheckmarks = enableDevelopmentFeatures
+
+export const enableLinkUnderlines = enableDiffCheckMarksAndLinkUnderlines

app/src/lib/stores/app-store.ts

@@ -238,7 +238,7 @@ import {
 } from './updates/changes-state'
 import { ManualConflictResolution } from '../../models/manual-conflict-resolution'
 import { BranchPruner } from './helpers/branch-pruner'
-import { enableMoveStash } from '../feature-flag'
+import { enableLinkUnderlines, enableMoveStash } from '../feature-flag'
 import { Banner, BannerType } from '../../models/banner'
 import { ComputedAction } from '../../models/computed-action'
 import {
@@ -407,6 +407,9 @@ const lastThankYouKey = 'version-and-users-of-last-thank-you'
 const pullRequestSuggestedNextActionKey =
   'pull-request-suggested-next-action-key'
 
+const underlineLinksKey = 'underline-links'
+const underlineLinksDefault = true
+
 const showDiffCheckMarksDefault = true
 const showDiffCheckMarksKey = 'diff-check-marks-visible'
 
@@ -544,6 +547,8 @@ export class AppStore extends TypedBaseStore<IAppState> {
 
   private cachedRepoRulesets = new Map<number, IAPIRepoRuleset>()
 
+  private underlineLinks: boolean = underlineLinksDefault
+
   public constructor(
     private readonly gitHubUserStore: GitHubUserStore,
     private readonly cloningRepositoriesStore: CloningRepositoriesStore,
@@ -1023,6 +1028,7 @@ export class AppStore extends TypedBaseStore<IAppState> {
       pullRequestSuggestedNextAction: this.pullRequestSuggestedNextAction,
       resizablePaneActive: this.resizablePaneActive,
       cachedRepoRulesets: this.cachedRepoRulesets,
+      underlineLinks: this.underlineLinks,
       showDiffCheckMarks: this.showDiffCheckMarks,
     }
   }
@@ -2212,6 +2218,11 @@ export class AppStore extends TypedBaseStore<IAppState> {
         PullRequestSuggestedNextAction
       ) ?? defaultPullRequestSuggestedNextAction
 
+    // Always false if the feature flag is disabled.
+    this.underlineLinks = enableLinkUnderlines()
+      ? getBoolean(underlineLinksKey, underlineLinksDefault)
+      : false
+
     this.showDiffCheckMarks = getBoolean(
       showDiffCheckMarksKey,
       showDiffCheckMarksDefault
@@ -7929,6 +7940,14 @@ export class AppStore extends TypedBaseStore<IAppState> {
       this.emitUpdate()
     }
   }
+
+  public _updateUnderlineLinks(underlineLinks: boolean) {
+    if (underlineLinks !== this.underlineLinks) {
+      this.underlineLinks = underlineLinks
+      setBoolean(underlineLinksKey, underlineLinks)
+      this.emitUpdate()
+    }
+  }
 }
 
 /**

app/src/models/preferences.ts

@@ -6,4 +6,5 @@ export enum PreferencesTab {
   Notifications,
   Prompts,
   Advanced,
+  Accessibility,
 }

app/src/ui/app.tsx

@@ -1660,6 +1660,7 @@ export class App extends React.Component<IAppProps, IAppState> {
             selectedTheme={this.state.selectedTheme}
             repositoryIndicatorsEnabled={this.state.repositoryIndicatorsEnabled}
             onOpenFileInExternalEditor={this.openFileInExternalEditor}
+            underlineLinks={this.state.underlineLinks}
           />
         )
       case PopupType.RepositorySettings: {
@@ -1917,6 +1918,7 @@ export class App extends React.Component<IAppProps, IAppState> {
             emoji={this.state.emoji}
             newReleases={popup.newReleases}
             onDismissed={onPopupDismissedFn}
+            underlineLinks={this.state.underlineLinks}
           />
         )
       case PopupType.DeletePullRequest:
@@ -2405,6 +2407,7 @@ export class App extends React.Component<IAppProps, IAppState> {
             emoji={this.state.emoji}
             onSubmit={onPopupDismissedFn}
             onDismissed={onPopupDismissedFn}
+            underlineLinks={this.state.underlineLinks}
             accounts={this.state.accounts}
           />
         )
@@ -2532,6 +2535,7 @@ export class App extends React.Component<IAppProps, IAppState> {
             emoji={this.state.emoji}
             onSubmit={onPopupDismissedFn}
             onDismissed={onPopupDismissedFn}
+            underlineLinks={this.state.underlineLinks}
             accounts={this.state.accounts}
           />
         )
@@ -3156,6 +3160,7 @@ export class App extends React.Component<IAppProps, IAppState> {
         showCIStatusPopover={this.state.showCIStatusPopover}
         emoji={this.state.emoji}
         enableFocusTrap={enableFocusTrap}
+        underlineLinks={this.state.underlineLinks}
       />
     )
   }
@@ -3350,7 +3355,12 @@ export class App extends React.Component<IAppProps, IAppState> {
       return null
     }
 
-    const className = this.state.appIsFocused ? 'focused' : 'blurred'
+    const className = classNames(
+      this.state.appIsFocused ? 'focused' : 'blurred',
+      {
+        'underline-links': this.state.underlineLinks,
+      }
+    )
 
     const currentTheme = this.state.showWelcomeFlow
       ? ApplicationTheme.Light

app/src/ui/branches/branches-container.tsx

@@ -54,6 +54,8 @@ interface IBranchesContainerProps {
 
   /** Map from the emoji shortcut (e.g., :+1:) to the image's local path. */
   readonly emoji: Map<string, string>
+
+  readonly underlineLinks: boolean
 }
 
 interface IBranchesContainerState {
@@ -138,6 +140,7 @@ export class BranchesContainer extends React.Component<
         pullRequestItemTop={prListItemTop}
         onMouseEnter={this.onMouseEnterPullRequestQuickView}
         onMouseLeave={this.onMouseLeavePullRequestQuickView}
+        underlineLinks={this.props.underlineLinks}
       />
     )
   }

app/src/ui/dispatcher/dispatcher.ts

@@ -3913,4 +3913,8 @@ export class Dispatcher {
   ) {
     this.appStore.onChecksFailedNotification(repository, pullRequest, checks)
   }
+
+  public setUnderlineLinksSetting(underlineLinks: boolean) {
+    return this.appStore._updateUnderlineLinks(underlineLinks)
+  }
 }

app/src/ui/lib/sandboxed-markdown.tsx

@@ -40,6 +40,8 @@ interface ISandboxedMarkdownProps {
 
   /** The context of which markdown resides - such as PullRequest, PullRequestComment, Commit */
   readonly markdownContext?: MarkdownContext
+
+  readonly underlineLinks: boolean
 }
 
 interface ISandboxedMarkdownState {
@@ -202,7 +204,12 @@ export class SandboxedMarkdown extends React.PureComponent<
         ${scrapeVariable('--text-color')}
         ${scrapeVariable('--background-color')}
       }
+
       ${css}
+
+      .markdown-body a {
+        text-decoration: ${this.props.underlineLinks ? 'underline' : 'inherit'};
+      }
     </style>`
   }
 

app/src/ui/notifications/pull-request-comment-like.tsx

@@ -32,6 +32,8 @@ interface IPullRequestCommentLikeProps {
 
   readonly switchingToPullRequest: boolean
 
+  readonly underlineLinks: boolean
+
   readonly renderFooterContent: () => JSX.Element
 
   readonly onSubmit: () => void
@@ -173,6 +175,7 @@ export abstract class PullRequestCommentLike extends React.Component<IPullReques
         repository={base.gitHubRepository}
         onMarkdownLinkClicked={this.onMarkdownLinkClicked}
         markdownContext={'PullRequestComment'}
+        underlineLinks={this.props.underlineLinks}
       />
     )
   }

app/src/ui/notifications/pull-request-comment.tsx

@@ -19,6 +19,8 @@ interface IPullRequestCommentProps {
   /** Map from the emoji shortcut (e.g., :+1:) to the image's local path. */
   readonly emoji: Map<string, string>
 
+  readonly underlineLinks: boolean
+
   /**
    * Whether or not the dialog should offer to switch to the PR's repository or
    * to checkout the PR branch when applicable (e.g. non-approved reviews).
@@ -83,6 +85,7 @@ export class PullRequestComment extends React.Component<
         renderFooterContent={this.renderFooterContent}
         onSubmit={onSubmit}
         onDismissed={onDismissed}
+        underlineLinks={this.props.underlineLinks}
         accounts={accounts}
       />
     )

app/src/ui/notifications/pull-request-review.tsx

@@ -22,6 +22,8 @@ interface IPullRequestReviewProps {
   /** Map from the emoji shortcut (e.g., :+1:) to the image's local path. */
   readonly emoji: Map<string, string>
 
+  readonly underlineLinks: boolean
+
   /**
    * Whether or not the dialog should offer to switch to the PR's repository or
    * to checkout the PR branch when applicable (e.g. non-approved reviews).
@@ -85,6 +87,7 @@ export class PullRequestReview extends React.Component<
         renderFooterContent={this.renderFooterContent}
         onSubmit={onSubmit}
         onDismissed={onDismissed}
+        underlineLinks={this.props.underlineLinks}
         accounts={this.props.accounts}
       />
     )

app/src/ui/preferences/accessibility.tsx

@@ -0,0 +1,62 @@
+import * as React from 'react'
+import { DialogContent } from '../dialog'
+import { Checkbox, CheckboxValue } from '../lib/checkbox'
+
+interface IAccessibilityPreferencesProps {
+  readonly underlineLinks: boolean
+  readonly onUnderlineLinksChanged: (value: boolean) => void
+}
+
+export class Accessibility extends React.Component<
+  IAccessibilityPreferencesProps,
+  {}
+> {
+  public constructor(props: IAccessibilityPreferencesProps) {
+    super(props)
+  }
+
+  public render() {
+    return (
+      <DialogContent>
+        <div className="advanced-section">
+          <h2>Accessibility</h2>
+          <Checkbox
+            label="Underline links"
+            value={
+              this.props.underlineLinks ? CheckboxValue.On : CheckboxValue.Off
+            }
+            onChange={this.onUnderlineLinksChanged}
+            ariaDescribedBy="underline-setting-description"
+          />
+          <p
+            id="underline-setting-description"
+            className="git-settings-description"
+          >
+            When enabled, GitHub Desktop will underline links in commit
+            messages, comments, and other text fields. This can help make links
+            easier to distinguish. {this.renderExampleLink()}
+          </p>
+        </div>
+      </DialogContent>
+    )
+  }
+
+  private renderExampleLink() {
+    // The example link is rendered with inline style to override the global setting.
+    const style = {
+      textDecoration: this.props.underlineLinks ? 'underline' : 'none',
+    }
+
+    return (
+      <span className="link-button-component" style={style}>
+        This is an example link
+      </span>
+    )
+  }
+
+  private onUnderlineLinksChanged = (
+    event: React.FormEvent<HTMLInputElement>
+  ) => {
+    this.props.onUnderlineLinksChanged(event.currentTarget.checked)
+  }
+}

app/src/ui/preferences/preferences.tsx

@@ -42,6 +42,8 @@ import {
 import { Prompts } from './prompts'
 import { Repository } from '../../models/repository'
 import { Notifications } from './notifications'
+import { Accessibility } from './accessibility'
+import { enableLinkUnderlines } from '../../lib/feature-flag'
 
 interface IPreferencesProps {
   readonly dispatcher: Dispatcher
@@ -67,6 +69,7 @@ interface IPreferencesProps {
   readonly selectedTheme: ApplicationTheme
   readonly repositoryIndicatorsEnabled: boolean
   readonly onOpenFileInExternalEditor: (path: string) => void
+  readonly underlineLinks: boolean
 }
 
 interface IPreferencesState {
@@ -94,6 +97,7 @@ interface IPreferencesState {
   readonly selectedExternalEditor: string | null
   readonly availableShells: ReadonlyArray<Shell>
   readonly selectedShell: Shell
+
   /**
    * If unable to save Git configuration values (name, email)
    * due to an existing configuration lock file this property
@@ -108,6 +112,8 @@ interface IPreferencesState {
 
   readonly isLoadingGitConfig: boolean
   readonly globalGitConfigPath: string | null
+
+  readonly underlineLinks: boolean
 }
 
 /** The app-level preferences component. */
@@ -147,6 +153,7 @@ export class Preferences extends React.Component<
       initiallySelectedTheme: this.props.selectedTheme,
       isLoadingGitConfig: true,
       globalGitConfigPath: null,
+      underlineLinks: this.props.underlineLinks,
     }
   }
 
@@ -263,6 +270,12 @@ export class Preferences extends React.Component<
               <Octicon className="icon" symbol={octicons.gear} />
               Advanced
             </span>
+            {enableLinkUnderlines() && (
+              <span>
+                <Octicon className="icon" symbol={octicons.accessibility} />
+                Accessibility
+              </span>
+            )}
           </TabBar>
 
           {this.renderActiveTab()}
@@ -423,6 +436,14 @@ export class Preferences extends React.Component<
         )
         break
       }
+      case PreferencesTab.Accessibility:
+        View = (
+          <Accessibility
+            underlineLinks={this.state.underlineLinks}
+            onUnderlineLinksChanged={this.onUnderlineLinksChanged}
+          />
+        )
+        break
       default:
         return assertNever(index, `Unknown tab index: ${index}`)
     }
@@ -525,6 +546,10 @@ export class Preferences extends React.Component<
     this.props.dispatcher.setSelectedTheme(theme)
   }
 
+  private onUnderlineLinksChanged = (underlineLinks: boolean) => {
+    this.setState({ underlineLinks })
+  }
+
   private renderFooter() {
     const hasDisabledError = this.state.disallowedCharactersMessage != null
 
@@ -645,6 +670,8 @@ export class Preferences extends React.Component<
       this.state.uncommittedChangesStrategy
     )
 
+    this.props.dispatcher.setUnderlineLinksSetting(this.state.underlineLinks)
+
     this.props.onDismissed()
   }
 

app/src/ui/pull-request-quick-view.tsx

@@ -34,6 +34,8 @@ interface IPullRequestQuickViewProps {
 
   /** Map from the emoji shortcut (e.g., :+1:) to the image's local path. */
   readonly emoji: Map<string, string>
+
+  readonly underlineLinks: boolean
 }
 
 interface IPullRequestQuickViewState {
@@ -209,6 +211,7 @@ export class PullRequestQuickView extends React.Component<
           markdownContext={'PullRequest'}
           onMarkdownLinkClicked={this.onMarkdownLinkClicked}
           onMarkdownParsed={this.onMarkdownParsed}
+          underlineLinks={this.props.underlineLinks}
         />
       </div>
     )

app/src/ui/release-notes/release-notes-dialog.tsx

@@ -15,6 +15,7 @@ interface IReleaseNotesProps {
   readonly onDismissed: () => void
   readonly emoji: Map<string, string>
   readonly newReleases: ReadonlyArray<ReleaseSummary>
+  readonly underlineLinks: boolean
 }
 
 /**
@@ -118,6 +119,7 @@ export class ReleaseNotes extends React.Component<IReleaseNotesProps, {}> {
         markdown={pretext[0].message}
         emoji={this.props.emoji}
         onMarkdownLinkClicked={this.onMarkdownLinkClicked}
+        underlineLinks={this.props.underlineLinks}
       />
     )
   }

app/src/ui/toolbar/branch-dropdown.tsx

@@ -72,6 +72,8 @@ interface IBranchDropdownProps {
    * using the dialog focus management.
    */
   readonly enableFocusTrap: boolean
+
+  readonly underlineLinks: boolean
 }
 
 /**
@@ -101,6 +103,7 @@ export class BranchDropdown extends React.Component<IBranchDropdownProps> {
         emoji={this.props.emoji}
         onDeleteBranch={this.onDeleteBranch}
         onRenameBranch={this.onRenameBranch}
+        underlineLinks={this.props.underlineLinks}
       />
     )
   }

app/styles/_type.scss

@@ -6,3 +6,7 @@
   font-size: var(--font-size-sm);
   color: var(--text-secondary-color);
 }
+
+.underline-links a {
+  text-decoration: underline;
+}