Improve styling, accessebility, usability and use mutation

src/core/client/stream/components/Comment/Comment.css

@@ -1,3 +1,3 @@
 .footer {
-  margin-top: calc(1.5 * var(--spacing-unit));
+  margin-top: var(--spacing-unit);
 }

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

@@ -3,7 +3,7 @@ import { StatelessComponent } from "react";
 import { Typography } from "talk-ui/components";
 import * as styles from "./Comment.css";
 
-import PermalinkContainer from "../../containers/PermalinkContainer";
+import PermalinkButtonContainer from "../../containers/PermalinkButtonContainer";
 import Timestamp from "./Timestamp";
 import TopBar from "./TopBar";
 import Username from "./Username";
@@ -27,7 +27,7 @@ const Comment: StatelessComponent<CommentProps> = props => {
       </TopBar>
       <Typography>{props.body}</Typography>
       <div className={styles.footer}>
-        <PermalinkContainer commentID={props.id} />
+        <PermalinkButtonContainer commentID={props.id} />
       </div>
     </div>
   );

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

@@ -1,38 +0,0 @@
-import { Localized } from "fluent-react/compat";
-import React from "react";
-import { Button, ButtonIcon, Popover } from "talk-ui/components";
-import PermalinkPopover from "./PermalinkPopover";
-
-interface PermalinkProps {
-  commentID: string;
-  assetURL: string | null;
-}
-
-class Permalink extends React.Component<PermalinkProps> {
-  public render() {
-    const { commentID, assetURL } = this.props;
-    return (
-      <Popover
-        id="permalink-popover"
-        placement="top"
-        body={({ toggleVisibility }) => (
-          <PermalinkPopover
-            permalinkUrl={`${assetURL}&commentID=${commentID}`}
-            toggleVisibility={toggleVisibility}
-          />
-        )}
-      >
-        {({ toggleVisibility, forwardRef }) => (
-          <Button onClick={toggleVisibility} forwardRef={forwardRef}>
-            <ButtonIcon>share</ButtonIcon>
-            <Localized id="comments-permalink-share">
-              <span>Share</span>
-            </Localized>
-          </Button>
-        )}
-      </Popover>
-    );
-  }
-}
-
-export default Permalink;

src/core/client/stream/components/Permalink/PermalinkPopover.tsx

@@ -1,60 +0,0 @@
-import { Localized } from "fluent-react/compat";
-import React, { CSSProperties } from "react";
-import CopyToClipboard from "react-copy-to-clipboard";
-import { Button, ClickOutside, Flex, TextField } from "talk-ui/components";
-
-interface InnerProps {
-  permalinkUrl: string;
-  style?: CSSProperties;
-  toggleVisibility: () => void;
-}
-
-interface State {
-  copied: boolean;
-}
-
-class PermalinkPopover extends React.Component<InnerProps> {
-  public state: State = {
-    copied: false,
-  };
-
-  public onCopy = async () => {
-    await this.toggleCopied();
-    setTimeout(() => {
-      this.toggleCopied();
-    }, 800);
-  };
-
-  public toggleCopied = () => {
-    this.setState((state: State) => ({
-      copied: !state.copied,
-    }));
-  };
-
-  public render() {
-    const { permalinkUrl, toggleVisibility } = this.props;
-    const { copied } = this.state;
-    return (
-      <ClickOutside onClickOutside={toggleVisibility}>
-        <Flex itemGutter="half">
-          <TextField defaultValue={permalinkUrl} />
-          <CopyToClipboard text={permalinkUrl} onCopy={this.onCopy}>
-            <Button color="primary" variant="filled">
-              {copied ? (
-                <Localized id="comments-permalink-copied">
-                  <span>Copied!</span>
-                </Localized>
-              ) : (
-                <Localized id="comments-permalink-copy">
-                  <span>Copy</span>
-                </Localized>
-              )}
-            </Button>
-          </CopyToClipboard>
-        </Flex>
-      </ClickOutside>
-    );
-  }
-}
-
-export default PermalinkPopover;

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

@@ -0,0 +1,63 @@
+import { Localized } from "fluent-react/compat";
+import React from "react";
+import { oncePerFrame } from "talk-common/utils";
+import { Button, ButtonIcon, ClickOutside, Popover } from "talk-ui/components";
+
+import PermalinkPopover from "./PermalinkPopover";
+
+interface PermalinkProps {
+  commentID: string;
+  assetURL: string | null;
+}
+
+class Permalink extends React.Component<PermalinkProps> {
+  // Helpers that prevents calling toggleVisibility more then once per frame.
+  // In essence this means we'll only process an event only once.
+  // This might happen, when clicking on the button which will
+  // cause its onClick to happen as well as onClickOutside.
+  private toggleVisibilityOncePerFrame = oncePerFrame(
+    (toggleVisibility: () => void) => toggleVisibility()
+  );
+
+  public render() {
+    const { commentID, assetURL } = this.props;
+    const popoverID = "permalink-popover";
+    return (
+      <Popover
+        id={popoverID}
+        placement="top-start"
+        description="A dialog showing a permalink to the comment"
+        body={({ toggleVisibility }) => (
+          <ClickOutside
+            onClickOutside={() =>
+              this.toggleVisibilityOncePerFrame(toggleVisibility)
+            }
+          >
+            <PermalinkPopover
+              permalinkURL={`${assetURL}&commentID=${commentID}`}
+              toggleVisibility={toggleVisibility}
+            />
+          </ClickOutside>
+        )}
+      >
+        {({ toggleVisibility, forwardRef, visible }) => (
+          <Button
+            onClick={() => this.toggleVisibilityOncePerFrame(toggleVisibility)}
+            aria-controls={popoverID}
+            forwardRef={forwardRef}
+            variant="ghost"
+            active={visible}
+            size="small"
+          >
+            <ButtonIcon>share</ButtonIcon>
+            <Localized id="comments-permalink-share">
+              <span>Share</span>
+            </Localized>
+          </Button>
+        )}
+      </Popover>
+    );
+  }
+}
+
+export default Permalink;

src/core/client/stream/components/PermalinkButton/PermalinkPopover.css

@@ -0,0 +1,7 @@
+.root {
+  width: 300px;
+}
+
+.textField {
+  flex-grow: 1;
+}

src/core/client/stream/components/PermalinkButton/PermalinkPopover.tsx

@@ -0,0 +1,60 @@
+import { Localized } from "fluent-react/compat";
+import React from "react";
+import CopyToClipboard from "react-copy-to-clipboard";
+
+import { Button, Flex, TextField } from "talk-ui/components";
+
+import * as styles from "./PermalinkPopover.css";
+
+interface InnerProps {
+  permalinkURL: string;
+  toggleVisibility: () => void;
+}
+
+interface State {
+  copied: boolean;
+}
+
+class PermalinkPopover extends React.Component<InnerProps> {
+  public state: State = {
+    copied: false,
+  };
+
+  private onCopy = async () => {
+    await this.toggleCopied();
+    setTimeout(() => {
+      this.toggleCopied();
+    }, 800);
+  };
+
+  private toggleCopied = () => {
+    this.setState((state: State) => ({
+      copied: !state.copied,
+    }));
+  };
+
+  public render() {
+    const { permalinkURL } = this.props;
+    const { copied } = this.state;
+    return (
+      <Flex itemGutter="half" className={styles.root}>
+        <TextField defaultValue={permalinkURL} className={styles.textField} />
+        <CopyToClipboard text={permalinkURL} onCopy={this.onCopy}>
+          <Button color="primary" variant="filled" size="small">
+            {copied ? (
+              <Localized id="comments-permalink-copied">
+                <span>Copied!</span>
+              </Localized>
+            ) : (
+              <Localized id="comments-permalink-copy">
+                <span>Copy</span>
+              </Localized>
+            )}
+          </Button>
+        </CopyToClipboard>
+      </Flex>
+    );
+  }
+}
+
+export default PermalinkPopover;

src/core/client/stream/components/PermalinkButton/index.ts

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

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

@@ -1,9 +1,9 @@
-import * as React from "react";
-import { StatelessComponent } from "react";
+import React, { StatelessComponent } from "react";
 
 import Logo from "talk-stream/components/Logo";
 import { Button, Flex, Typography } from "talk-ui/components";
-import CommentContainer from "../../containers/CommentContainer";
+
+import CommentContainer from "../containers/CommentContainer";
 import * as styles from "./PermalinkView.css";
 
 export interface PermalinkViewProps {

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

@@ -1,9 +1,9 @@
 import React, { StatelessComponent } from "react";
 import { graphql } from "react-relay";
 import { withLocalStateContainer } from "talk-framework/lib/relay";
-import { AppQueryLocal as Local } from "talk-stream/__generated__/AppQueryLocal.graphql";
+import { PermalinkButtonContainerLocal as Local } from "talk-stream/__generated__/PermalinkButtonContainerLocal.graphql";
 
-import Permalink from "../components/Permalink/Permalink";
+import PermalinkButton from "../components/PermalinkButton";
 
 interface InnerProps {
   local: Local;
@@ -15,13 +15,13 @@ export const PermalinkContainer: StatelessComponent<InnerProps> = ({
   commentID,
 }) => {
   return local.assetURL ? (
-    <Permalink assetURL={local.assetURL} commentID={commentID} />
+    <PermalinkButton assetURL={local.assetURL} commentID={commentID} />
   ) : null;
 };
 
 const enhanced = withLocalStateContainer<Local>(
   graphql`
-    fragment PermalinkContainerLocal on Local {
+    fragment PermalinkButtonContainerLocal on Local {
       assetURL
     }
   `

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

@@ -10,7 +10,7 @@ import {
   SetCommentIDMutation,
   withSetCommentIDMutation,
 } from "talk-stream/mutations";
-import PermalinkView from "../components/Permalink/PermalinkView";
+import PermalinkView from "../components/PermalinkView";
 
 interface PermalinkViewContainerProps {
   data: Data;
@@ -22,9 +22,7 @@ class PermalinkViewContainer extends React.Component<
   PermalinkViewContainerProps
 > {
   private showAllComments = () => {
-    const { local } = this.props;
-    window.location.href = local.assetURL!;
-    // mutation
+    this.props.setCommentID({ id: null });
   };
   public render() {
     const { data, local } = this.props;

src/core/client/stream/mutations/SetCommentIDMutation.ts

@@ -3,15 +3,15 @@ import { createMutationContainer } from "talk-framework/lib/relay";
 import { LOCAL_ID } from "talk-framework/lib/relay/withLocalStateContainer";
 
 export interface SetCommentIDInput {
-  commentID: string | null;
+  id: string | null;
 }
 
-export type SetCommentIDMutation = (input: SetCommentIDInput) => void;
+export type SetCommentIDMutation = (input: SetCommentIDInput) => Promise<void>;
 
 async function commit(environment: Environment, input: SetCommentIDInput) {
   return commitLocalUpdate(environment, store => {
     const record = store.get(LOCAL_ID)!;
-    record.setValue(input.commentID, "commentID");
+    record.setValue(input.id, "commentID");
   });
 }
 

src/core/client/stream/mutations/SetNetworkStatusMutation.ts

@@ -8,7 +8,9 @@ export interface SetNetworkStatusInput {
   isOffline: boolean;
 }
 
-export type SetNetworkStatusMutation = (input: SetNetworkStatusInput) => void;
+export type SetNetworkStatusMutation = (
+  input: SetNetworkStatusInput
+) => Promise<void>;
 
 async function commit(environment: Environment, input: SetNetworkStatusInput) {
   return commitLocalUpdate(environment, store => {

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

@@ -19,7 +19,7 @@ exports[`renders loading 1`] = `
 `;
 
 exports[`renders permalink view 1`] = `
-<Relay(PermalinkViewContainer)
+<withContext(createMutationContainer(Relay(withContext(LocalStateContainer))))
   data={Object {}}
 />
 `;

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

@@ -2,7 +2,7 @@ import React from "react";
 import { findDOMNode } from "react-dom";
 
 export interface ClickOutsideProps {
-  onClickOutside: () => void;
+  onClickOutside: (e?: MouseEvent) => void;
   children: React.ReactNode;
 }
 
@@ -13,7 +13,7 @@ class ClickOutside extends React.Component<ClickOutsideProps> {
     const { onClickOutside } = this.props;
     if (!e || !this.domNode!.contains(e.target as HTMLInputElement)) {
       // tslint:disable-next-line:no-unused-expression
-      onClickOutside && onClickOutside();
+      onClickOutside && onClickOutside(e);
     }
   };
 

src/core/client/ui/components/Popover/Popover.css

@@ -6,7 +6,7 @@
   border-radius: 1px;
   color: #222;
   display: flex;
-  padding: 6px 10px;
+  padding: calc(0.5 * var(--spacing-unit));
 
   &:after,
   &::before {

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

@@ -27,10 +27,21 @@ type Placement =
   | "left"
   | "left-start";
 
+interface BodyRenderProps {
+  toggleVisibility: () => void;
+  visible: boolean;
+}
+
+interface ChildrenRenderProps {
+  toggleVisibility: () => void;
+  forwardRef?: RefHandler;
+  visible: boolean;
+}
+
 interface PopoverProps {
-  body: (props: RenderProps) => React.ReactNode | React.ReactElement<any>;
-  children: (props: RenderProps) => React.ReactNode;
-  description?: string;
+  body: (props: BodyRenderProps) => React.ReactNode | React.ReactElement<any>;
+  children: (props: ChildrenRenderProps) => React.ReactNode;
+  description: string;
   id: string;
   onClose?: () => void;
   className?: string;
@@ -41,11 +52,6 @@ interface State {
   visible: false;
 }
 
-interface RenderProps {
-  toggleVisibility: () => void;
-  forwardRef?: RefHandler;
-}
-
 class Popover extends React.Component<PopoverProps> {
   public state: State = {
     visible: false,
@@ -97,24 +103,20 @@ class Popover extends React.Component<PopoverProps> {
             children({
               forwardRef: props.ref,
               toggleVisibility: this.toggleVisibility,
+              visible: this.state.visible,
             })
           }
         </Reference>
-        <Popper
-          placement={placement}
-          modifiers={{ preventOverflow: { enabled: false } }}
-          eventsEnabled
-          positionFixed={false}
-        >
-          {(props: PopperArrowProps) =>
-            visible && (
-              <div
-                id={id}
-                role="popup"
-                aria-labelledby={`${id}-ariainfo`}
-                aria-hidden={!visible}
-              >
-                <AriaInfo id={`${id}-ariainfo`}>{description}</AriaInfo>
+        <Popper placement={placement} eventsEnabled positionFixed={false}>
+          {(props: PopperArrowProps) => (
+            <div
+              id={id}
+              role="popup"
+              aria-labelledby={`${id}-ariainfo`}
+              aria-hidden={!visible}
+            >
+              <AriaInfo id={`${id}-ariainfo`}>{description}</AriaInfo>
+              {visible && (
                 <div
                   style={props.style}
                   className={cn(styles.root, className)}
@@ -123,12 +125,13 @@ class Popover extends React.Component<PopoverProps> {
                   {typeof body === "function"
                     ? body({
                         toggleVisibility: this.toggleVisibility,
+                        visible: this.state.visible,
                       })
                     : body}
                 </div>
-              </div>
-            )
-          }
+              )}
+            </div>
+          )}
         </Popper>
       </Manager>
     );

src/core/client/ui/components/TextField/TextField.css

@@ -4,5 +4,5 @@
   border: 1px solid #979797;
   box-sizing: border-box;
   border-radius: 1px;
-  padding: 5px 20px;
+  padding: calc(0.5 * var(--spacing-unit));
 }

src/core/common/utils/index.ts

@@ -1,2 +1,3 @@
 export { default as timeout } from "./timeout";
 export { default as pascalCase } from "./pascalCase";
+export { default as oncePerFrame } from "./oncePerFrame";

src/core/common/utils/oncePerFrame.ts

@@ -0,0 +1,18 @@
+/**
+ * Function decorator that prevents calling `fn` more then once per frame.
+ * If called more than once, the last return value gets returned.
+ */
+const oncePerFrame = <T extends (...args: any[]) => any>(fn: T) => {
+  let toggledThisFrame = false;
+  let lastResult: any = null;
+  return ((...args: any[]) => {
+    if (toggledThisFrame) {
+      return lastResult;
+    }
+    toggledThisFrame = true;
+    lastResult = fn(...args);
+    setTimeout(() => (toggledThisFrame = false), 0);
+  }) as T;
+};
+
+export default oncePerFrame;