From 55b6b021f1168d4d26158cf46b37be78818cc906 Mon Sep 17 00:00:00 2001
From: Michel Weststrate <mweststrate@fb.com>
Date: Tue, 19 May 2020 11:49:39 -0700
Subject: [PATCH] Fix QPL layout regeression

Summary:
Fixed the layout usage in QPL, the `horizontal` was correct, but the elements where swapped (the table was supposed to take all remaining size, and sidebar it's needed space, rather than the reverse).

Made this more explicit in the Layout component, by splitting it up in `Layout.(Top|Left|Right|Bottom)`, so that one has to make an explicit choice here, making it less error prone.

Reviewed By: passy

Differential Revision: D21572438

fbshipit-source-id: 29aa3462a3c96d048825be3157730e26182cb2fa
---
 desktop/app/src/ui/components/Layout.tsx      | 87 ++++++++++++-------
 .../ui/components/searchable/Searchable.tsx   |  4 +-
 2 files changed, 60 insertions(+), 31 deletions(-)

diff --git a/desktop/app/src/ui/components/Layout.tsx b/desktop/app/src/ui/components/Layout.tsx
index a0e11b75b..605fd07a5 100644
--- a/desktop/app/src/ui/components/Layout.tsx
+++ b/desktop/app/src/ui/components/Layout.tsx
@@ -12,24 +12,26 @@ import styled from '@emotion/styled';
 
 type Props = {
   scrollable?: boolean;
-  horizontal?: boolean;
+  children: [React.ReactNode, React.ReactNode];
 };
 
 const FixedContainer = styled('div')({
   flex: 'none',
   height: 'auto',
-  overflow: 'none',
+  overflow: 'hidden',
 });
 FixedContainer.displayName = 'Layout:FixedContainer';
 
-const ScrollContainer = styled('div')<Props>(({scrollable}) => ({
-  overflow: scrollable ? 'auto' : 'hidden',
-  flex: 'auto',
-  display: 'flex',
-}));
+const ScrollContainer = styled('div')<{scrollable: boolean}>(
+  ({scrollable}) => ({
+    overflow: scrollable ? 'auto' : 'hidden',
+    flex: 'auto',
+    display: 'flex',
+  }),
+);
 ScrollContainer.displayName = 'Layout:ScrollContainer';
 
-const Container = styled('div')<Props>(({horizontal}) => ({
+const Container = styled('div')<{horizontal: boolean}>(({horizontal}) => ({
   display: 'flex',
   flex: 'auto',
   flexDirection: horizontal ? 'row' : 'column',
@@ -39,35 +41,62 @@ const Container = styled('div')<Props>(({horizontal}) => ({
 }));
 Container.displayName = 'Layout:Container';
 
+function renderLayout(
+  {children, scrollable}: Props,
+  horizontal: boolean,
+  reverse: boolean,
+) {
+  if (children.length !== 2) {
+    throw new Error('Layout expects exactly 2 children');
+  }
+  const fixedElement = (
+    <FixedContainer>{reverse ? children[1] : children[0]}</FixedContainer>
+  );
+  const dynamicElement = (
+    <ScrollContainer scrollable={!!scrollable}>
+      {reverse ? children[0] : children[1]}
+    </ScrollContainer>
+  );
+  return reverse ? (
+    <Container horizontal={horizontal}>
+      {dynamicElement}
+      {fixedElement}
+    </Container>
+  ) : (
+    <Container horizontal={horizontal}>
+      {fixedElement}
+      {dynamicElement}
+    </Container>
+  );
+}
+
 /**
  * The Layout component divides all available screenspace over two components:
  * A fixed top (or left) component, and all remaining space to a bottom component.
  *
  * The main area will be scrollable by default, but if multiple containers are nested,
  * scrolling can be disabled by using `scrollable={false}`
+ *
+ * Use Layout.Top / Right / Bottom / Left to indicate where the fixed element should live.
  */
-const Layout: React.FC<
-  {
-    children: [React.ReactNode, React.ReactNode];
-  } & Props
-> = ({children, ...props}) => {
-  if (children.length !== 2) {
-    throw new Error('Layout expects exactly 2 children');
-  }
-  const top = children[0];
-  const main = children[1];
-  return (
-    <Container {...props}>
-      <FixedContainer>{top}</FixedContainer>
-      <ScrollContainer {...props}>{main}</ScrollContainer>
-    </Container>
-  );
+const Layout: Record<'Left' | 'Right' | 'Top' | 'Bottom', React.FC<Props>> = {
+  Top(props) {
+    return renderLayout(props, false, false);
+  },
+  Bottom(props) {
+    return renderLayout(props, false, true);
+  },
+  Left(props) {
+    return renderLayout(props, true, false);
+  },
+  Right(props) {
+    return renderLayout(props, true, true);
+  },
 };
 
-Layout.displayName = 'Layout';
-Layout.defaultProps = {
-  scrollable: false,
-  horizontal: false,
-};
+Layout.Top.displayName = 'Layout.Top';
+Layout.Left.displayName = 'Layout.Left';
+Layout.Bottom.displayName = 'Layout.Bottom';
+Layout.Right.displayName = 'Layout.Right';
 
 export default Layout;
diff --git a/desktop/app/src/ui/components/searchable/Searchable.tsx b/desktop/app/src/ui/components/searchable/Searchable.tsx
index a9266875b..987252328 100644
--- a/desktop/app/src/ui/components/searchable/Searchable.tsx
+++ b/desktop/app/src/ui/components/searchable/Searchable.tsx
@@ -472,7 +472,7 @@ const Searchable = (
     render() {
       const {placeholder, actions, ...props} = this.props;
       return (
-        <Layout>
+        <Layout.Top>
           <SearchBar position="top" key="searchbar">
             <SearchBox tabIndex={-1}>
               <SearchIcon
@@ -540,7 +540,7 @@ const Searchable = (
             bodySearchEnabled={this.state.bodySearchEnabled}
             filters={this.state.filters}
           />
-        </Layout>
+        </Layout.Top>
       );
     }
   };