Merge pull request #1784 from coralproject/use-anchor-in-permalink

[next] Use anchor for better accessibility and restore history

src/core/client/embed/decorators/withSetCommentID.spec.ts

@@ -1,3 +1,7 @@
+import simulant from "simulant";
+import sinon from "sinon";
+
+import { CleanupCallback } from ".";
 import withSetCommentID from "./withSetCommentID";
 
 it("should add commentID", () => {
@@ -10,8 +14,9 @@ it("should add commentID", () => {
       }
     },
   };
-  withSetCommentID(fakePym as any);
+  const cleanup = withSetCommentID(fakePym as any) as CleanupCallback;
   expect(location.toString()).toBe("http://localhost/?commentID=comment-id");
+  cleanup();
   window.history.replaceState(previousState, document.title, previousLocation);
 });
 
@@ -30,7 +35,31 @@ it("should remove commentID", () => {
       }
     },
   };
-  withSetCommentID(fakePym as any);
+  const cleanup = withSetCommentID(fakePym as any) as CleanupCallback;
   expect(location.toString()).toBe("http://localhost/");
+  cleanup();
+  window.history.replaceState(previousState, document.title, previousLocation);
+});
+
+it("should send commentID over pym when history changes", () => {
+  const previousLocation = location.toString();
+  const previousState = window.history.state;
+  window.history.replaceState(
+    previousState,
+    document.title,
+    "http://localhost/?commentID=comment-id"
+  );
+  const fakePym = {
+    onMessage: sinon.stub(),
+    sendMessage: sinon
+      .mock()
+      .once()
+      .withArgs("setCommentID", "comment-id"),
+  };
+  const cleanup = withSetCommentID(fakePym as any) as CleanupCallback;
+  simulant.fire(window as any, "popstate");
+  cleanup();
+  simulant.fire(window as any, "popstate");
+  fakePym.sendMessage.verify();
   window.history.replaceState(previousState, document.title, previousLocation);
 });

src/core/client/embed/decorators/withSetCommentID.ts

@@ -3,6 +3,10 @@ import qs from "query-string";
 import { buildURL } from "../utils";
 import { Decorator } from "./";
 
+function getCurrentCommentID() {
+  return qs.parse(location.search).commentID;
+}
+
 const withSetCommentID: Decorator = pym => {
   // Add the permalink comment id to the query.
   pym.onMessage("setCommentID", (id: string) => {
@@ -15,8 +19,20 @@ const withSetCommentID: Decorator = pym => {
     const url = buildURL({ search });
 
     // Change the url.
-    window.history.replaceState({}, document.title, url);
+    window.history.pushState({}, document.title, url);
   });
+
+  // Send new commentID when history state changes.
+  const sendSetCommentID = (e: Event) => {
+    const commentID = getCurrentCommentID();
+    pym.sendMessage("setCommentID", commentID);
+  };
+  window.addEventListener("popstate", sendSetCommentID);
+
+  // Cleanup.
+  return () => {
+    window.removeEventListener("popstate", sendSetCommentID);
+  };
 };
 
 export default withSetCommentID;

src/core/client/framework/utils/buildURL.spec.ts

@@ -0,0 +1,18 @@
+import buildURL from "./buildURL";
+
+it("should default to window.location", () => {
+  const url = buildURL();
+  expect(url).toBe("http://localhost/");
+});
+
+it("should build from parameters", () => {
+  const url = buildURL({
+    protocol: "https",
+    hostname: "hostname",
+    port: "8080",
+    pathname: "/pathname",
+    search: "search",
+    hash: "#hash",
+  });
+  expect(url).toBe("https//hostname:8080/pathname?search#hash");
+});

src/core/client/framework/utils/buildURL.ts

@@ -0,0 +1,17 @@
+export default function buildURL({
+  protocol = window.location.protocol,
+  hostname = window.location.hostname,
+  port = window.location.port,
+  pathname = window.location.pathname,
+  search = window.location.search,
+  hash = window.location.hash,
+} = {}) {
+  if (search && search[0] !== "?") {
+    search = `?${search}`;
+  } else if (search === "?") {
+    search = "";
+  }
+  return `${protocol}//${hostname}${
+    port ? `:${port}` : ""
+  }${pathname}${search}${hash}`;
+}

src/core/client/framework/utils/index.ts

@@ -0,0 +1,2 @@
+export { default as buildURL } from "./buildURL";
+export { default as parseURL } from "./parseURL";

src/core/client/framework/utils/parseURL.spec.ts

@@ -0,0 +1,20 @@
+import parseURL from "./parseURL";
+
+it("should parse url", () => {
+  const testCases: [[string, ReturnType<typeof parseURL>]] = [
+    [
+      "https://coralproject.net",
+      {
+        protocol: "https:",
+        hostname: "coralproject.net",
+        port: "",
+        pathname: "/",
+        search: "",
+        hash: "",
+      },
+    ],
+  ];
+  testCases.forEach(([url, expected]) => {
+    expect(parseURL(url)).toEqual(expected);
+  });
+});

src/core/client/framework/utils/parseURL.ts

@@ -0,0 +1,14 @@
+import { pick } from "lodash";
+
+export default function parseURL(url: string) {
+  const parser = document.createElement("a");
+  parser.href = url;
+  return pick(parser, [
+    "protocol",
+    "hostname",
+    "port",
+    "pathname",
+    "search",
+    "hash",
+  ]);
+}

src/core/client/stream/components/PermalinkView.tsx

@@ -1,4 +1,4 @@
-import React, { StatelessComponent } from "react";
+import React, { MouseEvent, StatelessComponent } from "react";
 
 import { Button, Flex, Typography } from "talk-ui/components";
 
@@ -7,25 +7,28 @@ import * as styles from "./PermalinkView.css";
 
 export interface PermalinkViewProps {
   comment: {} | null;
-  assetURL: string | null;
-  onShowAllComments: () => void;
+  showAllCommentsHref: string | null;
+  onShowAllComments: (e: MouseEvent<any>) => void;
 }
 
 const PermalinkView: StatelessComponent<PermalinkViewProps> = ({
-  assetURL,
+  showAllCommentsHref,
   comment,
   onShowAllComments,
 }) => {
   return (
     <div className={styles.root}>
-      {assetURL && (
+      {showAllCommentsHref && (
         <Button
           id="talk-comments-permalinkView-showAllComments"
           variant="outlined"
           color="primary"
           onClick={onShowAllComments}
           className={styles.button}
+          href={showAllCommentsHref}
+          target="_parent"
           fullWidth
+          anchor
         >
           Show all Comments
         </Button>

src/core/client/stream/containers/PermalinkViewContainer.tsx

@@ -1,54 +1,67 @@
-import React from "react";
+import { Child as PymChild } from "pym.js";
+import qs from "query-string";
+import React, { MouseEvent } from "react";
 import { graphql } from "react-relay";
+import { withContext } from "talk-framework/lib/bootstrap";
 import { withFragmentContainer } from "talk-framework/lib/relay";
-import { PermalinkViewContainer_asset as AssetData } from "talk-stream/__generated__/PermalinkViewContainer_asset.graphql";
+import { buildURL, parseURL } from "talk-framework/utils";
 import { PermalinkViewContainer_comment as CommentData } from "talk-stream/__generated__/PermalinkViewContainer_comment.graphql";
 import {
   SetCommentIDMutation,
   withSetCommentIDMutation,
 } from "talk-stream/mutations";
+
 import PermalinkView from "../components/PermalinkView";
 
 interface PermalinkViewContainerProps {
   comment: CommentData | null;
-  asset: AssetData | null;
   setCommentID: SetCommentIDMutation;
+  pym: PymChild | undefined;
 }
 
 class PermalinkViewContainer extends React.Component<
   PermalinkViewContainerProps
 > {
-  private showAllComments = () => {
+  private showAllComments = (e: MouseEvent<any>) => {
     this.props.setCommentID({ id: null });
+    e.preventDefault();
   };
+  private getShowAllCommentsHref() {
+    const { pym } = this.props;
+    const urlParts = parseURL((pym && pym.parentUrl) || window.location.href);
+    const search = qs.stringify({
+      ...qs.parse(urlParts.search),
+      commentID: undefined,
+    });
+    // Remove the commentId url param.
+    return buildURL({ ...urlParts, search });
+  }
   public render() {
-    const { comment, asset } = this.props;
+    const { comment } = this.props;
     return (
       <PermalinkView
         comment={comment}
-        assetURL={(asset && asset.url) || null}
+        showAllCommentsHref={this.getShowAllCommentsHref()}
         onShowAllComments={this.showAllComments}
       />
     );
   }
 }
 
-const enhanced = withSetCommentIDMutation(
-  withFragmentContainer<{
-    comment: CommentData | null;
-    asset: AssetData | null;
-  }>({
-    comment: graphql`
-      fragment PermalinkViewContainer_comment on Comment {
-        ...CommentContainer
-      }
-    `,
-    asset: graphql`
-      fragment PermalinkViewContainer_asset on Asset {
-        url
-      }
-    `,
-  })(PermalinkViewContainer)
+const enhanced = withContext(ctx => ({
+  pym: ctx.pym,
+}))(
+  withSetCommentIDMutation(
+    withFragmentContainer<{
+      comment: CommentData | null;
+    }>({
+      comment: graphql`
+        fragment PermalinkViewContainer_comment on Comment {
+          ...CommentContainer
+        }
+      `,
+    })(PermalinkViewContainer)
+  )
 );
 
 export default enhanced;

src/core/client/stream/index.tsx

@@ -1,4 +1,4 @@
-import pym from "pym.js";
+import { Child as PymChild } from "pym.js";
 import React from "react";
 import { StatelessComponent } from "react";
 import ReactDOM from "react-dom";
@@ -12,10 +12,14 @@ import {
 import AppContainer from "./containers/AppContainer";
 import { initLocalState } from "./local";
 import localesData from "./locales";
+import { withSetCommentID } from "./pym";
+
+const pymFeatures = [withSetCommentID];
 
 // This is called when the context is first initialized.
-async function init({ relayEnvironment }: TalkContext) {
-  await initLocalState(relayEnvironment);
+async function init(context: TalkContext) {
+  await initLocalState(context.relayEnvironment);
+  pymFeatures.forEach(f => f(context));
 }
 
 async function main() {
@@ -24,7 +28,7 @@ async function main() {
     init,
     localesData,
     userLocales: navigator.languages,
-    pym: new pym.Child({ polling: 100 }),
+    pym: new PymChild({ polling: 100 }),
   });
 
   const Index: StatelessComponent = () => (

src/core/client/stream/pym/index.ts

@@ -0,0 +1 @@
+export { default as withSetCommentID } from "./withSetCommentID";

src/core/client/stream/pym/withSetCommentID.spec.ts

@@ -0,0 +1,45 @@
+import { Environment, RecordSource } from "relay-runtime";
+
+import { LOCAL_ID } from "talk-framework/lib/relay";
+import { createRelayEnvironment } from "talk-framework/testHelpers";
+
+import withSetCommentID from "./withSetCommentID";
+
+let relayEnvironment: Environment;
+const source: RecordSource = new RecordSource();
+
+beforeAll(() => {
+  relayEnvironment = createRelayEnvironment({
+    source,
+  });
+});
+
+it("Sets comment id", () => {
+  const id = "comment1-id";
+  const context = {
+    pym: {
+      onMessage: (eventName: string, cb: (id: string) => void) => {
+        expect(eventName).toBe("setCommentID");
+        cb(id);
+      },
+    },
+    relayEnvironment,
+  };
+  withSetCommentID(context as any);
+  expect(source.get(LOCAL_ID)!.commentID).toEqual(id);
+});
+
+it("Sets comment id to null when empty", () => {
+  const id = "";
+  const context = {
+    pym: {
+      onMessage: (eventName: string, cb: (data: string) => void) => {
+        expect(eventName).toBe("setCommentID");
+        cb(id);
+      },
+    },
+    relayEnvironment,
+  };
+  withSetCommentID(context as any);
+  expect(source.get(LOCAL_ID)!.commentID).toEqual(null);
+});

src/core/client/stream/pym/withSetCommentID.ts

@@ -0,0 +1,19 @@
+import { commitLocalUpdate } from "react-relay";
+
+import { TalkContext } from "talk-framework/lib/bootstrap";
+import { LOCAL_ID } from "talk-framework/lib/relay";
+
+export default function withSetCommentID({
+  relayEnvironment,
+  pym,
+}: TalkContext) {
+  // Sets comment id through pym.
+  pym!.onMessage("setCommentID", raw => {
+    commitLocalUpdate(relayEnvironment, s => {
+      const id = raw || null;
+      if (s.get(LOCAL_ID)!.getValue("commentID") !== id) {
+        s.get(LOCAL_ID)!.setValue(id, "commentID");
+      }
+    });
+  });
+}

src/core/client/stream/queries/PermalinkViewQuery.spec.tsx

@@ -6,7 +6,6 @@ import { render } from "./PermalinkViewQuery";
 it("renders permalink view container", () => {
   const data = {
     props: {
-      asset: {},
       comment: {},
     } as any,
     error: null,

src/core/client/stream/queries/PermalinkViewQuery.tsx

@@ -27,29 +27,23 @@ export const render = ({
     return <div>{error.message}</div>;
   }
   if (props) {
-    return (
-      <PermalinkViewContainer asset={props.asset} comment={props.comment} />
-    );
+    return <PermalinkViewContainer comment={props.comment} />;
   }
   return <div>Loading</div>;
 };
 
 const PermalinkViewQuery: StatelessComponent<InnerProps> = ({
-  local: { commentID, assetID },
+  local: { commentID },
 }) => (
   <QueryRenderer<PermalinkViewQueryVariables, PermalinkViewQueryResponse>
     query={graphql`
-      query PermalinkViewQuery($assetID: ID!, $commentID: ID!) {
-        asset(id: $assetID) {
-          ...PermalinkViewContainer_asset
-        }
+      query PermalinkViewQuery($commentID: ID!) {
         comment(id: $commentID) {
           ...PermalinkViewContainer_comment
         }
       }
     `}
     variables={{
-      assetID,
       commentID,
     }}
     render={render}
@@ -59,7 +53,6 @@ const PermalinkViewQuery: StatelessComponent<InnerProps> = ({
 const enhanced = withLocalStateContainer<Local>(
   graphql`
     fragment PermalinkViewQueryLocal on Local {
-      assetID
       commentID
     }
   `

src/core/client/stream/queries/__snapshots__/PermalinkViewQuery.spec.tsx.snap

@@ -13,8 +13,7 @@ exports[`renders loading 1`] = `
 `;
 
 exports[`renders permalink view container 1`] = `
-<withContext(createMutationContainer(Relay(PermalinkViewContainer)))
-  asset={Object {}}
+<withContext(withContext(createMutationContainer(Relay(PermalinkViewContainer))))
   comment={Object {}}
 />
 `;

src/core/client/stream/test/__snapshots__/permalinkView.spec.tsx.snap

@@ -7,8 +7,9 @@ exports[`renders permalink view 1`] = `
   <div
     className="PermalinkView-root"
   >
-    <button
+    <a
       className="BaseButton-root Button-root PermalinkView-button Button-sizeRegular Button-colorPrimary Button-variantOutlined Button-fullWidth"
+      href="http://localhost/"
       id="talk-comments-permalinkView-showAllComments"
       onBlur={[Function]}
       onClick={[Function]}
@@ -17,9 +18,10 @@ exports[`renders permalink view 1`] = `
       onMouseOut={[Function]}
       onMouseOver={[Function]}
       onTouchEnd={[Function]}
+      target="_parent"
     >
       Show all Comments
-    </button>
+    </a>
     <div
       className="Flex-root Flex-alignFlexStart Flex-directionColumn"
     >

src/core/client/stream/test/__snapshots__/permalinkViewAssetNotFound.spec.tsx.snap

@@ -1,17 +0,0 @@
-// Jest Snapshot v1, https://goo.gl/fbAQLP
-
-exports[`renders permalink view with unknown asset 1`] = `
-<div
-  className="Flex-root App-root Flex-justifyCenter Flex-alignCenter"
->
-  <div
-    className="PermalinkView-root"
-  >
-    <p
-      className="Typography-root Typography-bodyCopy Typography-colorTextPrimary"
-    >
-      Comment not found
-    </p>
-  </div>
-</div>
-`;

src/core/client/stream/test/__snapshots__/permalinkViewCommentNotFound.spec.tsx.snap

@@ -7,8 +7,9 @@ exports[`renders permalink view with unknown comment 1`] = `
   <div
     className="PermalinkView-root"
   >
-    <button
+    <a
       className="BaseButton-root Button-root PermalinkView-button Button-sizeRegular Button-colorPrimary Button-variantOutlined Button-fullWidth"
+      href="http://localhost/"
       id="talk-comments-permalinkView-showAllComments"
       onBlur={[Function]}
       onClick={[Function]}
@@ -17,9 +18,10 @@ exports[`renders permalink view with unknown comment 1`] = `
       onMouseOut={[Function]}
       onMouseOver={[Function]}
       onTouchEnd={[Function]}
+      target="_parent"
     >
       Show all Comments
-    </button>
+    </a>
     <p
       className="Typography-root Typography-bodyCopy Typography-colorTextPrimary"
     >

src/core/client/stream/test/permalinkView.spec.tsx

@@ -75,11 +75,15 @@ it("renders permalink view", async () => {
 });
 
 it("show all comments", async () => {
+  const mockEvent = {
+    preventDefault: sinon.mock().once(),
+  };
   testRenderer.root
     .findByProps({
       id: "talk-comments-permalinkView-showAllComments",
     })
-    .props.onClick();
+    .props.onClick(mockEvent);
   await timeout();
   expect(testRenderer.toJSON()).toMatchSnapshot();
+  mockEvent.preventDefault.verify();
 });

src/core/client/stream/test/permalinkViewAssetNotFound.spec.tsx

@@ -1,45 +0,0 @@
-import React from "react";
-import TestRenderer from "react-test-renderer";
-import { RecordProxy } from "relay-runtime";
-
-import { timeout } from "talk-common/utils";
-import { TalkContext, TalkContextProvider } from "talk-framework/lib/bootstrap";
-import { createRelayEnvironment } from "talk-framework/testHelpers";
-import AppContainer from "talk-stream/containers/AppContainer";
-
-const resolvers = {
-  Query: {
-    comment: () => null,
-    asset: () => null,
-  },
-};
-
-const environment = createRelayEnvironment({
-  network: {
-    // Set this to true, to see graphql responses.
-    logNetwork: false,
-    resolvers,
-    projectName: "tenant",
-  },
-  initLocalState: (localRecord: RecordProxy) => {
-    localRecord.setValue("unknown-asset-id", "assetID");
-    localRecord.setValue("unknown-comment-id", "commentID");
-  },
-});
-
-const context: TalkContext = {
-  relayEnvironment: environment,
-  localeMessages: [],
-};
-
-const testRenderer = TestRenderer.create(
-  <TalkContextProvider value={context}>
-    <AppContainer />
-  </TalkContextProvider>
-);
-
-it("renders permalink view with unknown asset", async () => {
-  // Wait for loading.
-  await timeout();
-  expect(testRenderer.toJSON()).toMatchSnapshot();
-});

src/core/client/stream/test/permalinkViewCommentNotFound.spec.tsx

@@ -71,11 +71,15 @@ it("renders permalink view with unknown comment", async () => {
 });
 
 it("show all comments", async () => {
+  const mockEvent = {
+    preventDefault: sinon.mock().once(),
+  };
   testRenderer.root
     .findByProps({
       id: "talk-comments-permalinkView-showAllComments",
     })
-    .props.onClick();
+    .props.onClick(mockEvent);
   await timeout();
   expect(testRenderer.toJSON()).toMatchSnapshot();
+  mockEvent.preventDefault.verify();
 });

src/core/client/ui/components/BaseButton/BaseButton.tsx

@@ -15,6 +15,9 @@ import * as styles from "./BaseButton.css";
 interface InnerProps extends ButtonHTMLAttributes<HTMLButtonElement> {
   /** If set renders an anchor tag instead */
   anchor?: boolean;
+  href?: string;
+  target?: string;
+
   /**
    * This prop can be used to add custom classnames.
    * It is handled by the `withStyles `HOC.

src/core/client/ui/components/Button/Button.tsx

@@ -11,6 +11,10 @@ import * as styles from "./Button.css";
 // This should extend from BaseButton instead but we can't because of this bug
 // TODO: add bug link.
 interface InnerProps extends ButtonHTMLAttributes<HTMLButtonElement> {
+  /** If set renders an anchor tag instead */
+  anchor?: boolean;
+  href?: string;
+  target?: string;
   /**
    * This prop can be used to add custom classnames.
    * It is handled by the `withStyles `HOC.