Scrolling improvements

Summary:
Split container had a convenient property `scrollable`, that automatically applies a Scroll container to the main content.

But I noticed it leads to bad design choices because it is so convenient. So sometimes scrolling would be unnecessarily in two directions because of this.

Or, since the scroll container wraps around the whole content, toolbars would scroll out of view. By forcing scrolling to be put explicitly in the component tree, we encourage plugin developers to think about where they actually want to have that scroll, and in which direction.

Also added options to use the Container padding properties on ScrollContainer, which is great since we can keep the scrollbars outside the padding, and apply it to the content only, to prevent an accidental mistake where people would put a scroll container in a padded container, that would put the scrollbar inside the padding.

Reviewed By: cekkaewnumchai

Differential Revision: D24502546

fbshipit-source-id: 524004a1c5f33a185f9b959251b72875dd623cb3

desktop/app/src/chrome/ConsoleLogs.tsx

@@ -92,7 +92,7 @@ export function ConsoleLogs() {
   const styles = useMemo(() => buildTheme(isSandy), [isSandy]);
 
   return (
-    <Layout.Top scrollable>
+    <Layout.Top>
       <Toolbar>
         <ButtonGroup>
           <Button onClick={clearLogs} icon="trash">
@@ -101,12 +101,14 @@ export function ConsoleLogs() {
           <Button dropdown={dropdown}>Log Levels</Button>
         </ButtonGroup>
       </Toolbar>
-      <Console
-        logs={logs}
-        filter={logLevels}
-        variant={isDarkMode || !isSandy ? 'dark' : 'light'}
-        styles={styles}
-      />
+      <Layout.ScrollContainer vertical>
+        <Console
+          logs={logs}
+          filter={logLevels}
+          variant={isDarkMode || !isSandy ? 'dark' : 'light'}
+          styles={styles}
+        />
+      </Layout.ScrollContainer>
     </Layout.Top>
   );
 }

desktop/app/src/sandy-chrome/DesignComponentDemos.tsx

@@ -179,6 +179,11 @@ const demos: PreviewProps[] = [
         'boolean',
         'specifies in which directions the container should scroll. If none is specified the container will scroll in both directions',
       ],
+      [
+        'padv / padh / pad',
+        'see Container',
+        'Padding will be applied to the child',
+      ],
     ],
     demos: {
       'Basic usage': (
@@ -246,34 +251,34 @@ const demos: PreviewProps[] = [
           {aFixedHeightBox}
         </Layout.Bottom>
       ),
-      'Layout.Top + scrollable': (
+      'Layout.Top + Layout.ScrollContainer': (
         <Layout.Container style={{height: 150}}>
-          <Layout.Top scrollable>
+          <Layout.Top>
             {aFixedHeightBox}
-            {largeChild}
+            <Layout.ScrollContainer>{largeChild}</Layout.ScrollContainer>
           </Layout.Top>
         </Layout.Container>
       ),
-      'Layout.Left + scrollable': (
+      'Layout.Left + Layout.ScrollContainer': (
         <Layout.Container style={{height: 150}}>
-          <Layout.Left scrollable>
+          <Layout.Left>
             {aFixedWidthBox}
-            {largeChild}
+            <Layout.ScrollContainer>{largeChild}</Layout.ScrollContainer>
           </Layout.Left>
         </Layout.Container>
       ),
-      'Layout.Right + scrollable': (
+      'Layout.Right + Layout.ScrollContainer': (
         <Layout.Container style={{height: 150}}>
-          <Layout.Right scrollable>
-            {largeChild}
+          <Layout.Right>
+            <Layout.ScrollContainer>{largeChild}</Layout.ScrollContainer>
             {aFixedWidthBox}
           </Layout.Right>
         </Layout.Container>
       ),
-      'Layout.Bottom + scrollable': (
+      'Layout.Bottom + Layout.ScrollContainer': (
         <Layout.Container style={{height: 150}}>
-          <Layout.Bottom scrollable>
-            {largeChild}
+          <Layout.Bottom>
+            <Layout.ScrollContainer>{largeChild}</Layout.ScrollContainer>
             {aFixedHeightBox}
           </Layout.Bottom>
         </Layout.Container>

desktop/app/src/sandy-chrome/LeftSidebar.tsx

@@ -15,7 +15,11 @@ import {Button, Tooltip, Typography} from 'antd';
 import {InfoCircleOutlined} from '@ant-design/icons';
 
 export const LeftSidebar: React.FC = ({children}) => (
-  <Layout.Container borderRight padv="small" grow shrink>
+  <Layout.Container
+    borderRight
+    style={{paddingTop: theme.space.small}}
+    grow
+    shrink>
     {children}
   </Layout.Container>
 );

desktop/app/src/sandy-chrome/appinspect/AppInspect.tsx

@@ -57,15 +57,13 @@ export function AppInspect() {
             </Layout.Horizontal>
           </Layout.Container>
         </Layout.Container>
-        <Layout.Container padv={theme.space.large}>
-          <Layout.ScrollContainer vertical>
-            {selectedDevice ? (
-              <PluginList />
-            ) : (
-              <Alert message="No device or app selected" type="info" />
-            )}
-          </Layout.ScrollContainer>
-        </Layout.Container>
+        <Layout.ScrollContainer vertical padv={theme.space.large}>
+          {selectedDevice ? (
+            <PluginList />
+          ) : (
+            <Alert message="No device or app selected" type="info" />
+          )}
+        </Layout.ScrollContainer>
       </Layout.Top>
     </LeftSidebar>
   );

desktop/app/src/ui/components/Layout.tsx

@@ -116,25 +116,26 @@ const ScrollContainer = ({
   children,
   horizontal,
   vertical,
+  padv,
+  padh,
+  pad,
   ...rest
 }: React.HTMLAttributes<HTMLDivElement> & {
   horizontal?: boolean;
   vertical?: boolean;
-}) => {
+} & PaddingProps) => {
   const axis =
     horizontal && !vertical ? 'x' : !horizontal && vertical ? 'y' : 'both';
   return (
     <ScrollParent axis={axis} {...rest}>
-      <ScrollChild axis={axis}>{children}</ScrollChild>
+      <ScrollChild axis={axis} padv={padv} padh={padh} pad={pad}>
+        {children}
+      </ScrollChild>
     </ScrollParent>
   ) as any;
 };
 
 type SplitLayoutProps = {
-  /**
-   * If set, the dynamically sized pane will get scrollbars when needed
-   */
-  scrollable?: boolean;
   /**
    * If set, items will be centered over the orthogonal direction, if false (the default) items will be stretched.
    */
@@ -150,8 +151,7 @@ function renderSplitLayout(
   // eslint-disable-next-line
   const isSandy = useIsSandy();
   if (!isSandy) return renderLayout(props, direction === 'row', grow === 1);
-  let [child1, child2] = props.children;
-  if (props.scrollable) child2 = <ScrollContainer>{child2}</ScrollContainer>;
+  const [child1, child2] = props.children;
   return (
     <SandySplitContainer {...props} flexDirection={direction} grow={grow}>
       {child1}

desktop/plugins/sections/src/index.tsx

@@ -256,7 +256,7 @@ export function Component() {
             </Button>
           )}
         </Toolbar>
-        <Layout.Top scrollable={false}>
+        <Layout.Top>
           <Sidebar position="top" minHeight={80} height={80}>
             <EventTable
               generations={filteredGenerations}
@@ -270,7 +270,7 @@ export function Component() {
               }}
             />
           </Sidebar>
-          <Layout.Top scrollable={true}>
+          <Layout.Top>
             <Sidebar position="top" minHeight={400} height={400}>
               <TreeContainer>
                 <TreeHierarchy
@@ -282,12 +282,14 @@ export function Component() {
               </TreeContainer>
             </Sidebar>
             {focusedTreeGeneration && (
-              <StackTrace
-                data={focusedTreeGeneration.stack_trace}
-                skipStackTraceFormat={
-                  focusedTreeGeneration.skip_stack_trace_format
-                }
-              />
+              <Layout.ScrollContainer>
+                <StackTrace
+                  data={focusedTreeGeneration.stack_trace}
+                  skipStackTraceFormat={
+                    focusedTreeGeneration.skip_stack_trace_format
+                  }
+                />
+              </Layout.ScrollContainer>
             )}
           </Layout.Top>
         </Layout.Top>