app: add and use a shared Link component (#890)

Summary:
This will enable us to style links consistently across our application.

Our previous link colors for base and `:visited` were so similar that I
didn’t actually realize that they were different. In this change, I’ve
kept the same base color, and selected a more contrasting `:visited`
color. I also added an `:active` color, which is good for usability
(color chosen via <http://paletton.com/>’s “triad” suggestion).

Example screenshot, with active, visited, and base links:
![Screenshot as in this commit][img-underline]

I also considered implementing the link underlines with `border-bottom`
instead of `text-decoration` (an oft-touted suggestion that has always
smelled fishy to me, but [the W3 does it][w3], so I guess it’s okay).
That would look like this:

![Screenshot with `border-bottom` underlines][img-border]

…but I did not do so ([rationale in a comment on #890][rationale]).

[w3]: https://www.w3.org/TR/WCAG20-TECHS/F73.html
[img-underline]: https://user-images.githubusercontent.com/4317806/45987381-f154f580-c025-11e8-8b0f-63c1e1ddce02.png
[img-border]: https://user-images.githubusercontent.com/4317806/45926176-a081b780-bed4-11e8-96f2-d0d24d11c8f7.png
[rationale]: https://github.com/sourcecred/sourcecred/pull/890#issuecomment-424146773

We can certainly change these decisions later—that’s one of the purposes
of having this abstraction—so I’m not inclined to bikeshed on them too
much in this commit.

Implementation adapted from:
<80263b190e/src/components/Link.js>

Test Plan:
Check that `<a>` elements and React Router link elements are used only
in `Link` and snapshots:

```
$ git grep --name-only -Fw '<a'
src/app/Link.js
src/assets/logo/discourse_512.png
src/plugins/github/__snapshots__/render.test.js.snap
$ git grep '"react-router"' | grep 'Link'
src/app/Link.js:import {Link as RouterLink} from "react-router";
src/app/Link.test.js:import {Link as RouterLink} from "react-router";
src/app/createRelativeHistory.test.js:import {Router, Route, Link} from "react-router";
src/app/withAssets.test.js:import {IndexRoute, Link, Router, Route} from "react-router";
```

Check that the primary color now appears in just one spot:

```
$ git grep -i 0872a2
src/app/Link.js:    ...colorAttributes("#0872A2"),
```

Then, run `yarn start` and click all the links. Note in particular that
the SVG icons in the header have the correct colors in the active state
as well as the base state.

wchargin-branch: app-link

src/app/ExternalRedirect.js

@@ -2,6 +2,8 @@
 
 import React from "react";
 
+import Link from "./Link";
+
 export default class ExternalRedirect extends React.Component<{|
   +redirectTo: string,
 |}> {
@@ -11,7 +13,7 @@ export default class ExternalRedirect extends React.Component<{|
         <h1>Redirecting…</h1>
         <p>
           Redirecting to:{" "}
-          <a href={this.props.redirectTo}>{this.props.redirectTo}</a>
+          <Link href={this.props.redirectTo}>{this.props.redirectTo}</Link>
         </p>
       </div>
     );

src/app/HomePage.js

@@ -1,10 +1,9 @@
 // @flow
 
-import {StyleSheet, css} from "aphrodite/no-important";
+import React from "react";
-import React, {type Node} from "react";
-import {Link} from "react-router";
 
 import type {Assets} from "./assets";
+import Link from "./Link";
 
 export default class HomePage extends React.Component<{|+assets: Assets|}> {
   render() {
@@ -44,12 +43,13 @@ export default class HomePage extends React.Component<{|+assets: Assets|}> {
         <p>
           Despite all the value provided by open-source projects, many are
           chronically underfunded. For example, NumPy{" "}
-          <A href={urls.numpyFunding}>received no funding at all until 2017</A>,
+          <Link href={urls.numpyFunding}>
-          and{" "}
+            received no funding at all until 2017
-          <A href={urls.opensslFunding}>
+          </Link>, and{" "}
+          <Link href={urls.opensslFunding}>
             a world where OpenSSL was funded might have been a world without
             Heartbleed
-          </A>.
+          </Link>.
         </p>
 
         <p>
@@ -115,12 +115,14 @@ export default class HomePage extends React.Component<{|+assets: Assets|}> {
         <h2>How cred works</h2>
         <p>
           Cred is computed by first creating a contribution{" "}
-          <A href={urls.graph}>graph</A>
+          <Link href={urls.graph}>graph</Link>
           , which contains every contribution to the project and the relations
           among them. For example, GitHub issues, Git commits, and individual
           files and functions can be included in the graph. Then, SourceCred
-          runs a modified version of <A href={urls.pagerank}>PageRank</A> on
+          runs a modified version of <Link href={urls.pagerank}>
-          that graph to produce a cred attribution. The attribution is highly
+            PageRank
+          </Link>{" "}
+          on that graph to produce a cred attribution. The attribution is highly
           configurable; project maintainers can add new heuristics and adjust
           weights.
         </p>
@@ -145,13 +147,11 @@ export default class HomePage extends React.Component<{|+assets: Assets|}> {
         <h2>Roadmap</h2>
         <p>
           SourceCred is under active development.{" "}
-          <Link className={css(styles.link)} to="/prototype">
+          <Link to="/prototype">We have a prototype</Link> that ingests data
-            We have a prototype
+          from Git and GitHub, computes cred, and allows the user to explore and
-          </Link>{" "}
+          experiment on the results. We have a long way to go to realize
-          that ingests data from Git and GitHub, computes cred, and allows the
+          SourceCred’s full vision, but the prototype can already surface some
-          user to explore and experiment on the results. We have a long way to
+          interesting insights!
-          go to realize SourceCred’s full vision, but the prototype can already
-          surface some interesting insights!
         </p>
 
         <p>
@@ -165,7 +165,7 @@ export default class HomePage extends React.Component<{|+assets: Assets|}> {
         <p>
           In the longer term, we will continue to add signal to cred
           attribution. For example, we plan to parse the{" "}
-          <A href={urls.ast}>AST</A> of a project’s code so that we can
+          <Link href={urls.ast}>AST</Link> of a project’s code so that we can
           attribute cred at the level of individual functions, and create a
           “spotlight” mechanic that will let contributors flow more cred to
           their peers’ important contributions. As SourceCred improves, we have
@@ -179,30 +179,23 @@ export default class HomePage extends React.Component<{|+assets: Assets|}> {
           decentralized. We don’t think communities should have to give their
           data to us, or entrust us with control over their cred. The lead
           developers are grateful to be supported by{" "}
-          <A href={urls.protocolLabs}>Protocol Labs</A>.
+          <Link href={urls.protocolLabs}>Protocol Labs</Link>.
         </p>
 
         <p>
           If you think this vision is exciting, we’d love for you to get
-          involved! You can join our <A href={urls.discord}>Discord</A> and
+          involved! You can join our <Link href={urls.discord}>Discord</Link>{" "}
-          check out our <A href={urls.github}>GitHub</A>—many of our issues are
+          and check out our <Link href={urls.github}>GitHub</Link>—many of our
-          marked <A href={urls.contributionsWelcome}>contributions welcome</A>.
+          issues are marked{" "}
+          <Link href={urls.contributionsWelcome}>contributions welcome</Link>.
           If you want to try running SourceCred on open-source projects you care
-          about, check out <A href={urls.readme}>our README</A>.
+          about, check out <Link href={urls.readme}>our README</Link>.
         </p>
       </div>
     );
   }
 }
 
-function A(props: {|+href: string, +children: Node|}) {
-  return (
-    <a className={css(styles.link)} href={props.href}>
-      {props.children}
-    </a>
-  );
-}
-
 function Dt(props) {
   return <dt style={{fontWeight: "bold"}}>{props.children}</dt>;
 }
@@ -210,15 +203,3 @@ function Dt(props) {
 function Dd(props) {
   return <dd style={{marginBottom: 15}}>{props.children}</dd>;
 }
-
-const styles = StyleSheet.create({
-  link: {
-    // TODO(@wchargin): Create a `<Link>` component to share these
-    // styles, abstracting over router-links (`to`) and external links
-    // (`href`).
-    color: "#0872A2",
-    ":visited": {
-      color: "#084598",
-    },
-  },
-});

src/app/Link.js

@@ -0,0 +1,58 @@
+// @flow
+
+import React, {Component} from "react";
+import {Link as RouterLink} from "react-router";
+import {StyleSheet, css} from "aphrodite/no-important";
+
+/**
+ * A styled link component for both client-side router links and normal
+ * external links.
+ *
+ * For a client-side link, specify `to={routePath}`. For a normal anchor
+ * tag, specify `href={href}`.
+ *
+ * To add Aphrodite styles: if you would normally write
+ *
+ *     <a className={css(x, y, z)} />
+ *
+ * then specify `styles={[x, y, z]}`.
+ *
+ * All other properties, including `children`, are forwarded directly.
+ */
+type LinkProps = $ReadOnly<{
+  ...React$ElementConfig<"a">,
+  ...{|+to: string|} | {|+href: string|},
+  +styles?: $ReadOnlyArray<
+    Object | false | null | void
+  > /* Aphrodite styles, as passed to `css` */,
+}>;
+export default class Link extends Component<LinkProps> {
+  render() {
+    const linkClass = css(styles.link, this.props.styles);
+    const className = this.props.className
+      ? `${linkClass} ${this.props.className}`
+      : linkClass;
+    const Tag = "to" in this.props ? RouterLink : "a";
+    return (
+      <Tag {...this.props} className={className}>
+        {this.props.children}
+      </Tag>
+    );
+  }
+}
+
+const colorAttributes = (color) => ({
+  color: color,
+  fill: color, // for child SVGs
+});
+const styles = StyleSheet.create({
+  link: {
+    ...colorAttributes("#0872A2"),
+    ":visited": {
+      ...colorAttributes("#3A066A"),
+    },
+    ":active": {
+      ...colorAttributes("#FF3201"),
+    },
+  },
+});

src/app/Link.test.js

@@ -0,0 +1,72 @@
+// @flow
+import {StyleSheet} from "aphrodite/no-important";
+import {shallow} from "enzyme";
+import React from "react";
+import {Link as RouterLink} from "react-router";
+
+import Link from "./Link";
+
+require("./testUtil").configureAphrodite();
+require("./testUtil").configureEnzyme();
+
+describe("src/app/Link", () => {
+  const styles = StyleSheet.create({
+    x: {fontWeight: "bold"},
+  });
+
+  // Static type checks
+  void [
+    // Must specify either `href` or `to`
+    <Link href="https://example.com/">click me</Link>,
+    <Link to="/prototype">click me, too</Link>,
+    // $ExpectFlowError
+    <Link>missing to/href</Link>,
+
+    // May specify styles
+    <Link href="#" styles={[styles.x, styles.y /* nonexistent */]} />,
+
+    // May specify extra properties
+    <Link href="#" onClick={() => void alert("hi")} tabIndex={3} />,
+  ];
+
+  it("renders a styled external link", () => {
+    const element = shallow(<Link href="https://example.com/">click me</Link>);
+    expect(element.type()).toBe("a");
+    expect(element.prop("href")).toEqual("https://example.com/");
+    expect(element.children().text()).toEqual("click me");
+    expect(typeof element.prop("className")).toBe("string");
+  });
+
+  it("renders a styled router link", () => {
+    const element = shallow(<Link to="/prototype">check it out</Link>);
+    expect(element.type()).toEqual(RouterLink);
+    expect(element.prop("to")).toEqual("/prototype");
+    expect(element.children().text()).toEqual("check it out");
+    expect(typeof element.prop("className")).toBe("string");
+  });
+
+  it("has deterministic className", () => {
+    const e1 = shallow(<Link href="#" />);
+    const e2 = shallow(<Link href="#" />);
+    expect(e2.prop("className")).toEqual(e1.prop("className"));
+  });
+
+  it("adds specified Aphrodite styles", () => {
+    const e1 = shallow(<Link href="#" />);
+    const e2 = shallow(<Link href="#" styles={[styles.x]} />);
+    expect(e2.prop("className")).not.toEqual(e1.prop("className"));
+  });
+
+  it("forwards class name", () => {
+    const e1 = shallow(<Link href="#" />);
+    const e2 = shallow(<Link href="#" className="ohai" />);
+    expect(e2.prop("className")).toEqual(e1.prop("className") + " ohai");
+  });
+
+  it("forwards other props, like `onClick` and `tabIndex`", () => {
+    const fn = () => {};
+    const element = shallow(<Link href="#" onClick={fn} tabIndex={77} />);
+    expect(element.prop("onClick")).toBe(fn);
+    expect(element.prop("tabIndex")).toBe(77);
+  });
+});

src/app/Page.js

@@ -1,10 +1,10 @@
 // @flow
 
 import React, {type Node} from "react";
-import {Link} from "react-router";
 import {StyleSheet, css} from "aphrodite/no-important";
 
 import type {Assets} from "./assets";
+import Link from "./Link";
 import GithubLogo from "./GithubLogo";
 import TwitterLogo from "./TwitterLogo";
 import DiscordLogo from "./DiscordLogo";
@@ -24,10 +24,7 @@ export default class Page extends React.Component<{|
             <nav className={css(style.nav)}>
               <ul className={css(style.navList)}>
                 <li className={css(style.navItem, style.navItemLeft)}>
-                  <Link
+                  <Link to="/" styles={[style.navLink, style.navLinkTitle]}>
-                    to="/"
-                    className={css(style.navLink, style.navLinkTitle)}
-                  >
                     SourceCred
                   </Link>
                 </li>
@@ -37,44 +34,44 @@ export default class Page extends React.Component<{|
                       key={path}
                       className={css(style.navItem, style.navItemRight)}
                     >
-                      <Link to={path} className={css(style.navLink)}>
+                      <Link to={path} styles={[style.navLink]}>
                         {navTitle}
                       </Link>
                     </li>
                   ))
                 )}
                 <li className={css(style.navItem, style.navItemRight)}>
-                  <a
+                  <Link
-                    className={css(style.navLink)}
+                    styles={[style.navLink]}
                     href="https://github.com/sourcecred/sourcecred"
                   >
                     <GithubLogo
                       altText="SourceCred Github"
                       className={css(style.navLogoSmall)}
                     />
-                  </a>
+                  </Link>
                 </li>
                 <li className={css(style.navItem, style.navItemRight)}>
-                  <a
+                  <Link
-                    className={css(style.navLink)}
+                    styles={[style.navLink]}
                     href="https://twitter.com/sourcecred"
                   >
                     <TwitterLogo
                       altText="SourceCred Twitter"
                       className={css(style.navLogoSmall)}
                     />
-                  </a>
+                  </Link>
                 </li>
                 <li className={css(style.navItem, style.navItemRightSmall)}>
-                  <a
+                  <Link
-                    className={css(style.navLink)}
+                    styles={[style.navLink]}
                     href="https://discordapp.com/invite/tsBTgc9"
                   >
                     <DiscordLogo
                       altText="Join the SourceCred Discord"
                       className={css(style.navLogoMedium)}
                     />
-                  </a>
+                  </Link>
                 </li>
               </ul>
             </nav>
@@ -131,14 +128,10 @@ const style = StyleSheet.create({
     display: "flex",
   },
   navLink: {
-    color: "#0872A2",
-    fill: "#0872A2",
     fontFamily: "Roboto Condensed",
     fontSize: 18,
     textDecoration: "none",
     ":hover": {
-      color: "#084598",
-      fill: "#084598",
       textDecoration: "underline",
     },
   },

src/app/credExplorer/App.js

@@ -6,6 +6,7 @@ import type {Assets} from "../assets";
 import type {LocalStore} from "../localStore";
 import CheckedLocalStore from "../checkedLocalStore";
 import BrowserLocalStore from "../browserLocalStore";
+import Link from "../Link";
 
 import {defaultStaticAdapters} from "../adapters/defaultPlugins";
 import {PagerankTable} from "./pagerankTable/Table";
@@ -100,15 +101,13 @@ export function createApp(
       return (
         <div style={{maxWidth: 900, margin: "0 auto", padding: "0 10px"}}>
           <p style={{textAlign: "right"}}>
-            <a
+            <Link href="https://discuss.sourcecred.io/t/a-gentle-introduction-to-cred/20">
-              href={
-                "https://discuss.sourcecred.io/t/a-gentle-introduction-to-cred/20"
-              }
-            >
               what is this?
-            </a>
+            </Link>
             {spacer()}
-            <a href={process.env.SOURCECRED_FEEDBACK_URL}>feedback</a>
+            <Link href={process.env.SOURCECRED_FEEDBACK_URL || ""}>
+              feedback
+            </Link>
           </p>
           <div style={{marginBottom: 10}}>
             <RepositorySelect

src/app/credExplorer/App.test.js

@@ -107,7 +107,12 @@ describe("app/credExplorer/App", () => {
 
   it("should have a feedback link with a valid URL", () => {
     const {el} = example();
-    const link = el.find("a").filterWhere((x) => x.text().includes("feedback"));
+    const link = el.find("Link").filterWhere((x) =>
+      x
+        .children()
+        .text()
+        .includes("feedback")
+    );
     expect(link).toHaveLength(1);
     expect(link.prop("href")).toMatch(/https?:\/\//);
   });

src/plugins/github/__snapshots__/render.test.js.snap

@@ -3,6 +3,7 @@
 exports[`plugins/github/render renders the right description for a comment 1`] = `
 <span>
   <a
+    class="link_1cmobhd"
     href="https://github.com/sourcecred/example-github/pull/5#discussion_r171460198"
     rel="nofollow noopener"
     target="_blank"
@@ -12,6 +13,7 @@ exports[`plugins/github/render renders the right description for a comment 1`] =
    on 
   <span>
     <a
+      class="link_1cmobhd"
       href="https://github.com/sourcecred/example-github/pull/5#pullrequestreview-100313899"
       rel="nofollow noopener"
       target="_blank"
@@ -21,6 +23,7 @@ exports[`plugins/github/render renders the right description for a comment 1`] =
      on 
     <span>
       <a
+        class="link_1cmobhd"
         href="https://github.com/sourcecred/example-github/pull/5"
         rel="nofollow noopener"
         target="_blank"
@@ -53,6 +56,7 @@ exports[`plugins/github/render renders the right description for a commit 1`] =
 <span>
   Commit 
   <a
+    class="link_1cmobhd"
     href="https://github.com/sourcecred/example-github/commit/0a223346b4e6dec0127b1e6aa892c4ee0424b66a"
     rel="nofollow noopener"
     target="_blank"
@@ -65,6 +69,7 @@ exports[`plugins/github/render renders the right description for a commit 1`] =
 exports[`plugins/github/render renders the right description for a issue 1`] = `
 <span>
   <a
+    class="link_1cmobhd"
     href="https://github.com/sourcecred/example-github/issues/2"
     rel="nofollow noopener"
     target="_blank"
@@ -78,6 +83,7 @@ exports[`plugins/github/render renders the right description for a issue 1`] = `
 exports[`plugins/github/render renders the right description for a pull 1`] = `
 <span>
   <a
+    class="link_1cmobhd"
     href="https://github.com/sourcecred/example-github/pull/5"
     rel="nofollow noopener"
     target="_blank"
@@ -106,6 +112,7 @@ exports[`plugins/github/render renders the right description for a pull 1`] = `
 
 exports[`plugins/github/render renders the right description for a repo 1`] = `
 <a
+  class="link_1cmobhd"
   href="https://github.com/sourcecred/example-github"
   rel="nofollow noopener"
   target="_blank"
@@ -117,6 +124,7 @@ exports[`plugins/github/render renders the right description for a repo 1`] = `
 exports[`plugins/github/render renders the right description for a review 1`] = `
 <span>
   <a
+    class="link_1cmobhd"
     href="https://github.com/sourcecred/example-github/pull/5#pullrequestreview-100313899"
     rel="nofollow noopener"
     target="_blank"
@@ -126,6 +134,7 @@ exports[`plugins/github/render renders the right description for a review 1`] =
    on 
   <span>
     <a
+      class="link_1cmobhd"
       href="https://github.com/sourcecred/example-github/pull/5"
       rel="nofollow noopener"
       target="_blank"
@@ -155,6 +164,7 @@ exports[`plugins/github/render renders the right description for a review 1`] =
 
 exports[`plugins/github/render renders the right description for a userlike 1`] = `
 <a
+  class="link_1cmobhd"
   href="https://github.com/wchargin"
   rel="nofollow noopener"
   target="_blank"

src/plugins/github/render.js

@@ -3,11 +3,13 @@
 import React, {type Node as ReactNode} from "react";
 import * as R from "./relationalView";
 
+import Link from "../../app/Link";
+
 function EntityUrl(props) {
   return (
-    <a href={props.entity.url()} target="_blank" rel="nofollow noopener">
+    <Link href={props.entity.url()} target="_blank" rel="nofollow noopener">
       {props.children}
-    </a>
+    </Link>
   );
 }
 

src/plugins/github/render.test.js

@@ -5,6 +5,7 @@ import {exampleEntities} from "./example/example";
 import {description} from "./render";
 import enzymeToJSON from "enzyme-to-json";
 
+require("../../app/testUtil").configureAphrodite();
 require("../../app/testUtil").configureEnzyme();
 
 describe("plugins/github/render", () => {