Fix #1714, move the share panel and button entirely into its own comp…

…onent Fix #1565, fix share panel alignment when extension notification banner is in place
mozilla-services · Oct 26, 2016 · ab468fd · ab468fd
1 parent 8d48dbd
commit ab468fd
Show file tree

Hide file tree

Showing 3 changed files with 102 additions and 97 deletions.
diff --git a/server/src/pages/shot/share-buttons.js b/server/src/pages/shot/share-buttons.js
@@ -1,10 +1,58 @@
 const React = require("react");
 const sendEvent = require("../../browser-send-event.js");
 
-exports.ShareButtons = class ShareButtons extends React.Component {
+exports.ShareButton = class ShareButton extends React.Component {
+  constructor(props) {
+    super(props);
+    this.state = {
+      display: false
+    };
+
+  }
+
+  render() {
+    let panel = null;
+    if (this.state.display) {
+      panel =  <ShareButtonPanel
+        clipUrl={this.props.clipUrl}
+        closePanel={this.onPanelClose.bind(this)} shot={this.props.shot}
+        staticLink={this.props.staticLink}
+        renderExtensionNotification={this.props.renderExtensionNotification}
+      />;
+    }
+    return <div>
+      <button className="button primary" id="toggle-share" onClick={ this.onClick.bind(this) }>
+        <img className="share-icon" src={ this.props.staticLink("/static/img/share.svg")} />
+        <span>Share</span>
+      </button>
+      {panel}
+    </div>;
+  }
+
+  onClick() {
+    let show = ! this.state.display;
+    this.setState({display: show});
+    if (show) {
+      sendEvent(
+        this.props.isOwner ? "start-share-owner" : "start-share-non-owner",
+        "navbar");
+    } else {
+      sendEvent("cancel-share");
+    }
+  }
+
+  onPanelClose() {
+    this.setState({display: false});
+  }
+
+};
+
+class ShareButtonPanel extends React.Component {
   constructor(props) {
     super(props);
     this.state = {copyText: "Copy"};
+    this.clickMaybeClose = this.clickMaybeClose.bind(this);
+    this.keyMaybeClose = this.keyMaybeClose.bind(this);
   }
 
   onClickShareButton(whichButton) {
@@ -29,12 +77,12 @@ exports.ShareButtons = class ShareButtons extends React.Component {
     sendEvent("share", "focus-url");
   }
 
-  onChange(e) {
-    // Do nothing -- we simply need this event handler to placate React
-  }
-
   render() {
-    return <div id="share-buttons-panel" className="share-panel default-color-scheme">
+    let className = "share-panel default-color-scheme";
+    if (this.props.renderExtensionNotification) {
+      className += " share-panel-with-notification";
+    }
+    return <div id="share-buttons-panel" className={className}>
       <div className="wrapper row-space">
         <a onClick={ this.onClickShareButton.bind(this, "facebook") } target="_blank" href={ "https://www.facebook.com/sharer/sharer.php?u=" + encodeURIComponent(this.props.shot.viewUrl) }>
           <img src={ this.props.staticLink("/static/img/share-facebook.svg") } />
@@ -54,16 +102,47 @@ exports.ShareButtons = class ShareButtons extends React.Component {
         <input className="copy-shot-link-input"
           value={ this.props.shot.viewUrl }
           onClick={ this.onClickInputField.bind(this) }
-          onChange={ this.onChange.bind(this) } />
+          onChange={ function () {} /* react gives a warning otherwise */ } />
         <button
           className="button secondary copy-toggle"
           onClick={ this.onClickCopyButton.bind(this) }>
           { this.state.copyText }
         </button>
       </div>
-      <div className="small">
-        { this.props.isPublic }
-      </div>
     </div>;
   }
-};
+
+  componentDidMount() {
+    document.addEventListener("click", this.clickMaybeClose, false);
+    document.addEventListener("keyup", this.keyMaybeClose, false);
+  }
+
+  componentWillUnmount() {
+    document.removeEventListener("click", this.clickMaybeClose, false);
+    document.removeEventListener("keyup", this.keyMaybeClose, false);
+  }
+
+  clickMaybeClose(event) {
+    if (! this.isPanel(event.target)) {
+      this.props.closePanel();
+    }
+  }
+
+  keyMaybeClose(event) {
+    if ((event.key || event.code) == "Escape") {
+      this.props.closePanel();
+    }
+  }
+
+  /* Returns true if the element is part of the share panel */
+  isPanel(el) {
+    while (el) {
+      if (el.id == "share-buttons-panel") {
+        return true;
+      }
+      el = el.parentNode;
+    }
+    return false;
+  }
+
+}
diff --git a/server/src/pages/shot/view.js b/server/src/pages/shot/view.js
@@ -2,7 +2,7 @@ const React = require("react");
 const ReactDOM = require("react-dom");
 const { Footer } = require("../../footer-view");
 const sendEvent = require("../../browser-send-event.js");
-const { ShareButtons } = require("./share-buttons");
+const { ShareButton } = require("./share-buttons");
 const { TimeDiff, intervalDescription } = require("./time-diff");
 const reactruntime = require("../../reactruntime");
 
@@ -147,12 +147,8 @@ class Body extends React.Component {
     super(props);
     this.state = {
       hidden: false,
-      sharePanelDisplay: false,
       closePageshotBanner: false
     };
-    // Need to bind this so we can add/remove the event listener
-    this.unsharePanelHandler = this.unsharePanelHandler.bind(this);
-    this.sharePanelKeypress = this.sharePanelKeypress.bind(this);
   }
 
   closeGetPageshotBanner() {
@@ -163,18 +159,6 @@ class Body extends React.Component {
     //this.props.controller.requestSavedShot();
   }
 
-  onClickShareButton(e) {
-    let show = ! this.state.sharePanelDisplay;
-    this.setState({sharePanelDisplay: show});
-    if (show) {
-      sendEvent(
-        this.props.isOwner ? "start-share-owner" : "start-share-non-owner",
-        "navbar");
-    } else {
-      sendEvent("cancel-share");
-    }
-  }
-
   onClickClose() {
     this.setState({hidden: true});
   }
@@ -183,24 +167,6 @@ class Body extends React.Component {
     this.setState({hidden: false});
   }
 
-  unsharePanelHandler(e) {
-    let el = e.target;
-    while (el) {
-      if (el.id === "share-buttons-panel" || el.id === "toggle-share") {
-        // A click in the share panel itself
-        return;
-      }
-      el = el.parentNode;
-    }
-    this.onClickShareButton();
-  }
-
-  sharePanelKeypress(e) {
-    if ((e.key || e.code) == "Escape") {
-      this.onClickShareButton();
-    }
-  }
-
   onClickDelete(e) {
     sendEvent("start-delete", "navbar", {useBeacon: true});
     if (window.confirm("Are you sure you want to delete this shot permanently?")) {
@@ -264,7 +230,6 @@ class Body extends React.Component {
     let shotDomain = this.props.shot.url; // FIXME: calculate
 
     let clips = [];
-    let shareButtons = [];
     let clipNames = shot.clipNames();
     if (clipNames.length && ! this.state.hidden) {
       let clipId = clipNames[0];
@@ -326,21 +291,6 @@ class Body extends React.Component {
       myShotsHref = "/";
     }
 
-    let isPublic = null;
-    if (this.props.isOwner && !this.state.closePrivacyNotice && Date.now() - shot.createdDate < 30000) {
-      if (shot.isPublic) {
-        isPublic = <div
-          id="private-notice">
-          This shot is only visible to you until you share the link.
-        </div>;
-      } else {
-        isPublic = <div
-          id="private-notice">
-          You&#39;ve saved a personal version of this shot. This shot is only visible to you until you share the link.
-        </div>;
-      }
-    }
-
     let clipUrl = null;
     if (clipNames.length) {
       let clipId = clipNames[0];
@@ -353,14 +303,6 @@ class Body extends React.Component {
     filenameTitle = filenameTitle.replace(/\s+/g, " ");
     let clipFilename = `Page-Shot-${date.getFullYear()}-${date.getMonth() + 1}-${date.getDate()} ${filenameTitle}`;
 
-    if (this.state.sharePanelDisplay) {
-      shareButtons = <ShareButtons
-                large={ true }
-                clipUrl={ clipUrl }
-                isPublic={ isPublic }
-                { ...this.props } />;
-    }
-
     /*
     {this.props.hasSavedShot ?
       <button id="upload-full-page" className="upload-full-page" onClick={ this.onClickUploadFullPage.bind(this) }>
@@ -369,10 +311,12 @@ class Body extends React.Component {
       : null}
     */
 
+    let renderExtensionNotification = ! (this.props.isExtInstalled || this.state.closePageshotBanner);
+
     return (
       <reactruntime.BodyTemplate {...this.props}>
         <div id="frame" className="inverse-color-scheme full-height column-space">
-          { this.renderExtRequired() }
+          { renderExtensionNotification ? this.renderExtRequired() : null }
         <div className="frame-header default-color-scheme">
           <div className="left">
             <a className="block-button button secondary" href={ myShotsHref } onClick={this.onClickMyShots.bind(this)}>{ myShotsText }</a>
@@ -382,18 +326,14 @@ class Body extends React.Component {
             </div>
           </div>
           <div className="more-shot-actions right">
-            <button className="button primary" id="toggle-share" onClick={ this.onClickShareButton.bind(this) }>
-              <img className="share-icon" src={ this.props.staticLink("/static/img/share.svg")} />
-              <span>Share</span>
-            </button>
+            <ShareButton clipUrl={clipUrl} shot={shot} isOwner={this.props.isOwner} staticLink={this.props.staticLink} renderExtensionNotification={renderExtensionNotification} />
             <a className="button secondary" href={ clipUrl } onClick={ this.onClickDownload.bind(this) }
               title="Download the shot image" download={ `${clipFilename}.png` }>
               <img src={ this.props.staticLink("/static/img/download.svg") } />
             </a>
             { trashOrFlagButton }
           </div>
         </div>
-        { shareButtons }
         { clips }
         { this.props.shot.showPage ? <span id="copy-flag">Copy</span> : null }
         { this.props.shot.showPage ?
@@ -404,9 +344,6 @@ class Body extends React.Component {
   }
 
   renderExtRequired() {
-    if (this.props.isExtInstalled || this.state.closePageshotBanner) {
-      return null;
-    }
     return <div className="default-color-scheme notification">
       <div> Page Shot is an experimental extension for Firefox. <a href={ this.props.backend } onClick={ this.clickedCreate.bind(this) }>Get it here</a></div>
       <a className="close" onClick={ this.closeGetPageshotBanner.bind(this) }></a>
@@ -455,24 +392,6 @@ class Body extends React.Component {
     sendEvent("start-feedback", "footer", {useBeacon: true});
   }
 
-  componentDidMount() {
-    this.componentDidUpdate();
-  }
-
-  componentDidUpdate() {
-    if (this.state.sharePanelDisplay) {
-      document.addEventListener("click", this.unsharePanelHandler, false);
-      document.addEventListener("keyup", this.sharePanelKeypress, false);
-    } else {
-      document.removeEventListener("click", this.unsharePanelHandler, false);
-      document.removeEventListener("keyup", this.sharePanelKeypress, false);
-    }
-  }
-
-  componentWillUnmount() {
-    document.removeEventListener("click", this.unsharePanelHandler, false);
-  }
-
 }
 
 class ExpireWidget extends React.Component {

diff --git a/static/css/frame.scss b/static/css/frame.scss
@@ -119,6 +119,12 @@
   top: 65px;
   width: 320px;
 
+  // When the extension notification box is up the panel gets aligned incorrectly;
+  // we add this extra class to fix this situation and bump the panel down:
+  &.share-panel-with-notification {
+    top: 102px;
+  }
+
   img {
     height: 54px;
     width: 54px;
@@ -145,6 +151,7 @@
     flex: 1 0 70px;
     padding: 0 10px;
   }
+
 }
 
 #toggle-share {