fix review findings

scm-ui/ui-components/src/contexts/index.ts

@@ -1,3 +0,0 @@
-// @create-index
-
-export { MenuContext, storeMenuCollapsed, isMenuCollapsed } from "./MenuContext";

scm-ui/ui-components/src/index.ts

@@ -67,7 +67,6 @@ export * from "./navigation";
 export * from "./repos";
 export * from "./table";
 export * from "./toast";
-export * from "./contexts";
 
 export {
   File,

scm-ui/ui-components/src/navigation/Section.tsx

@@ -7,32 +7,39 @@ type Props = {
   children: ReactElement[];
   collapsed?: boolean;
   onCollapse?: (newStatus: boolean) => void;
+  scrollTransitionAt?: number;
 };
 
-type StylingProps = {
-  scrollPositionY: number;
+type CollapsedProps = {
   collapsed: boolean;
 };
 
-const SectionContainer = styled.div`
-  position: ${(props: StylingProps) => (props.scrollPositionY > 210 && window.innerWidth > 770 ? "fixed" : "inherit")};
-  top: ${(props: StylingProps) => props.scrollPositionY > 210 && window.innerWidth > 770 && "4.5rem"};
-  width: ${(props: StylingProps) => (props.collapsed ? "5.5rem" : "20.5rem")};
+type PositionProps = CollapsedProps & {
+  scrollPositionY: number;
+  scrollTransitionAt: number;
+};
+
+const SectionContainer = styled.div<PositionProps>`
+  position: ${props =>
+    props.scrollPositionY > props.scrollTransitionAt && window.innerWidth > 770 ? "fixed" : "inherit"};
+  top: ${props => props.scrollPositionY > props.scrollTransitionAt && window.innerWidth > 770 && "2rem"};
+  width: ${props => (props.collapsed ? "5.5rem" : "20.5rem")};
 `;
 
-const SmallButton = styled(Button)`
+const SmallButton = styled(Button)<CollapsedProps>`
   padding-left: 1rem;
   padding-right: 1rem;
+  margin-right: ${(props: CollapsedProps) => (props.collapsed ? "0" : "0.5rem")};
   height: 1.5rem;
 `;
 
-const MenuLabel = styled.p`
+const MenuLabel = styled.p<CollapsedProps>`
   min-height: 2.5rem;
   display: flex;
-  justify-content: ${(props: { collapsed: boolean }) => (props.collapsed ? "center" : "space-between")};
+  justify-content: ${props => (props.collapsed ? "center" : "left")};
 `;
 
-const Section: FC<Props> = ({ label, children, collapsed, onCollapse }) => {
+const Section: FC<Props> = ({ label, children, collapsed, onCollapse, scrollTransitionAt }) => {
   const [scrollPositionY, setScrollPositionY] = useState(0);
 
   useEffect(() => {
@@ -49,14 +56,23 @@ const Section: FC<Props> = ({ label, children, collapsed, onCollapse }) => {
   const arrowIcon = collapsed ? <i className="fas fa-caret-down" /> : <i className="fas fa-caret-right" />;
 
   return (
-    <SectionContainer collapsed={collapsed ? collapsed : false} scrollPositionY={onCollapse ? scrollPositionY : 0}>
+    <SectionContainer
+      collapsed={collapsed ? collapsed : false}
+      scrollPositionY={onCollapse ? scrollPositionY : 0}
+      scrollTransitionAt={scrollTransitionAt ? scrollTransitionAt : 250}
+    >
       <MenuLabel className="menu-label" collapsed={collapsed ? collapsed : false}>
-        {collapsed ? "" : label}
         {onCollapse && (
-          <SmallButton color="info" className="is-medium" action={() => onCollapse(!collapsed)}>
+          <SmallButton
+            color="info"
+            className="is-medium"
+            action={() => onCollapse(!collapsed)}
+            collapsed={collapsed ? collapsed : false}
+          >
             {arrowIcon}
           </SmallButton>
         )}
+        {collapsed ? "" : label}
       </MenuLabel>
       <ul className="menu-list">{childrenWithProps}</ul>
     </SectionContainer>

scm-ui/ui-components/src/navigation/SubNavigation.tsx

@@ -1,6 +1,7 @@
-import React, { ReactNode } from "react";
+import React, { FC, ReactElement, useContext } from "react";
 import { Link, Route } from "react-router-dom";
 import classNames from "classnames";
+import { MenuContext } from "./MenuContext";
 
 type Props = {
   to: string;
@@ -8,57 +9,57 @@ type Props = {
   label: string;
   activeOnlyWhenExact?: boolean;
   activeWhenMatch?: (route: any) => boolean;
-  children?: ReactNode;
+  children?: ReactElement[];
   collapsed?: boolean;
   title?: string;
 };
 
-class SubNavigation extends React.Component<Props> {
-  static defaultProps = {
-    activeOnlyWhenExact: false
+const SubNavigation: FC<Props> = ({
+  to,
+  icon,
+  label,
+  activeOnlyWhenExact,
+  activeWhenMatch,
+  children,
+  collapsed,
+  title
+}) => {
+  const menuContext = useContext(MenuContext);
+
+  const isActive = (route: any) => {
+    return route.match || activeWhenMatch && activeWhenMatch(route);
   };
 
-  isActive(route: any) {
-    const { activeWhenMatch } = this.props;
-    return route.match || (activeWhenMatch && activeWhenMatch(route));
-  }
-
-  renderLink = (route: any) => {
-    const { to, icon, label, collapsed, title } = this.props;
-
+  const renderLink = (route: any) => {
     let defaultIcon = "fas fa-cog";
     if (icon) {
       defaultIcon = icon;
     }
 
-    let children = null;
-    if (this.isActive(route)) {
-      children = <ul className="sub-menu">{this.props.children}</ul>;
+    let childrenList = null;
+    if (isActive(route)) {
+      if (menuContext.menuCollapsed) {
+        menuContext.setMenuCollapsed(false);
+      }
+      childrenList = <ul className="sub-menu">{children}</ul>;
     }
 
     return (
       <li title={collapsed ? title : undefined}>
-        <Link
-          className={classNames(this.isActive(route) ? "is-active" : "", collapsed ? "has-text-centered" : "")}
-          to={to}
-        >
+        <Link className={classNames(isActive(route) ? "is-active" : "", collapsed ? "has-text-centered" : "")} to={to}>
           <i className={classNames(defaultIcon, "fa-fw")} /> {collapsed ? "" : label}
         </Link>
-        {children}
+        {childrenList}
       </li>
     );
   };
 
-  render() {
-    const { to, activeOnlyWhenExact } = this.props;
+  // removes last part of url
+  const parents = to.split("/");
+  parents.splice(-1, 1);
+  const parent = parents.join("/");
 
-    // removes last part of url
-    const parents = to.split("/");
-    parents.splice(-1, 1);
-    const parent = parents.join("/");
-
-    return <Route path={parent} exact={activeOnlyWhenExact} children={this.renderLink} />;
-  }
-}
+  return <Route path={parent} exact={activeOnlyWhenExact} children={renderLink} />;
+};
 
 export default SubNavigation;

scm-ui/ui-components/src/navigation/index.ts

@@ -7,3 +7,4 @@ export { default as SubNavigation } from "./SubNavigation";
 export { default as PrimaryNavigation } from "./PrimaryNavigation";
 export { default as PrimaryNavigationLink } from "./PrimaryNavigationLink";
 export { default as Section } from "./Section";
+export { MenuContext, storeMenuCollapsed, isMenuCollapsed } from "./MenuContext";

scm-ui/ui-components/src/repos/DiffFile.tsx

@@ -101,12 +101,12 @@ class DiffFile extends React.Component<Props, State> {
     }
   };
 
-  toggleSideBySide = (callback: (collapsed: boolean) => void) => {
+  toggleSideBySide = (callback: () => void) => {
     this.setState(
       state => ({
         sideBySide: !state.sideBySide
       }),
-      () => callback(true)
+      () => callback()
     );
   };