Merge pull request #276 from jorilallo/tom/less-render

Removes a react render loop when hovering over sidebar

frontend/components/Layout/components/SidebarCollections.js

@@ -53,39 +53,32 @@ type Props = {
 }
 
 @observer class CollectionLink extends React.Component {
-  @observable isHovering = false;
   @observable menuOpen = false;
 
-  handleHover = () => (this.isHovering = true);
-  handleBlur = () => {
-    if (!this.menuOpen) this.isHovering = false;
-  };
-
   render() {
     const { history, collection, activeDocument, ui } = this.props;
 
     return (
-      <DropToImport
+      <StyledDropToImport
         key={collection.id}
         history={history}
         collectionId={collection.id}
         activeClassName="activeDropZone"
-        onMouseEnter={this.handleHover}
-        onMouseLeave={this.handleBlur}
+        menuOpen={this.menuOpen}
       >
         <SidebarLink key={collection.id} to={collection.url}>
           <Flex justify="space-between">
             {collection.name}
 
-            {(this.isHovering || this.menuOpen) &&
-              <CollectionAction>
-                <CollectionMenu
-                  history={history}
-                  collection={collection}
-                  onShow={() => (this.menuOpen = true)}
-                  onClose={() => (this.menuOpen = false)}
-                />
-              </CollectionAction>}
+            <CollectionAction>
+              <CollectionMenu
+                history={history}
+                collection={collection}
+                onShow={() => (this.menuOpen = true)}
+                onClose={() => (this.menuOpen = false)}
+                open={this.menuOpen}
+              />
+            </CollectionAction>
           </Flex>
 
           {collection.id === ui.activeCollectionId &&
@@ -101,7 +94,7 @@ type Props = {
               ))}
             </Children>}
         </SidebarLink>
-      </DropToImport>
+      </StyledDropToImport>
     );
   }
 }
@@ -156,14 +149,6 @@ const DocumentLink = observer((props: DocumentLinkProps) => {
   );
 });
 
-const Header = styled(Flex)`
-  font-size: 11px;
-  font-weight: ${fontWeight.semiBold};
-  text-transform: uppercase;
-  color: ${color.slate};
-  letter-spacing: 0.04em;
-`;
-
 const CollectionAction = styled.a`
   color: ${color.slate};
   svg { opacity: .75; }
@@ -173,6 +158,26 @@ const CollectionAction = styled.a`
   }
 `;
 
+const StyledDropToImport = styled(DropToImport)`
+  ${CollectionAction} {
+    display: ${props => (props.menuOpen ? 'inline' : 'none')};
+  }
+
+  &:hover {
+    ${CollectionAction} {
+      display: inline;
+    }
+  }
+`;
+
+const Header = styled(Flex)`
+  font-size: 11px;
+  font-weight: ${fontWeight.semiBold};
+  text-transform: uppercase;
+  color: ${color.slate};
+  letter-spacing: 0.04em;
+`;
+
 const Children = styled(Flex)`
   margin-left: 20px;
 `;

frontend/scenes/Document/Document.js

@@ -10,7 +10,7 @@ import Flex from 'components/Flex';
 import { color, layout } from 'styles/constants';
 import {
   collectionUrl,
-  documentUpdateUrl,
+  updateDocumentUrl,
   documentMoveUrl,
   matchDocumentEdit,
   matchDocumentMove,
@@ -60,7 +60,7 @@ type Props = {
 
   @observable moveModalOpen: boolean = false;
 
-  componentDidMount() {
+  componentWillMount() {
     this.loadDocument(this.props);
   }
 
@@ -98,12 +98,15 @@ type Props = {
       this.newDocument = newDocument;
     } else {
       let document = this.getDocument(props.match.params.documentSlug);
-      if (document) {
-        this.props.ui.setActiveDocument(document);
-      }
 
-      await this.props.documents.fetch(props.match.params.documentSlug);
-      document = this.document;
+      if (document) {
+        this.props.documents.fetch(props.match.params.documentSlug);
+        this.props.ui.setActiveDocument(document);
+      } else {
+        document = await this.props.documents.fetch(
+          props.match.params.documentSlug
+        );
+      }
 
       if (document) {
         this.props.ui.setActiveDocument(document);
@@ -113,7 +116,7 @@ type Props = {
 
         // Update url to match the current one
         this.props.history.replace(
-          documentUpdateUrl(this.props.match.url, document.url)
+          updateDocumentUrl(props.match.url, document.url)
         );
       } else {
         // Render 404 with search
@@ -218,9 +221,9 @@ type Props = {
   render() {
     const isNew = this.props.newDocument;
     const isMoving = this.props.match.path === matchDocumentMove;
-    const isFetching = !this.document;
-    const titleText = get(this.document, 'title', '');
     const document = this.document;
+    const isFetching = !document;
+    const titleText = get(document, 'title', '');
 
     if (this.notFound) {
       return this.renderNotFound();

frontend/utils/routeHelpers.js

@@ -34,10 +34,14 @@ export function documentMoveUrl(doc: Document): string {
  * Replace full url's document part with the new one in case
  * the document slug has been updated
  */
-export function documentUpdateUrl(oldUrl: string, newUrl: string): string {
+export function updateDocumentUrl(oldUrl: string, newUrl: string): string {
   // Update url to match the current one
-  const urlParts = oldUrl.split('/');
-  return [newUrl, urlParts.slice(3)].join('/');
+  const urlParts = oldUrl.trim().split('/');
+  const actions = urlParts.slice(3);
+  if (actions[0]) {
+    return [newUrl, actions].join('/');
+  }
+  return newUrl;
 }
 
 export function newDocumentUrl(collection: Collection): string {