Some fixes in rendering legacy plugins

Summary: Some exploratory testing on all iOS and Android plugins, to see how they behave inside Sandy, and fixed some layout glitches (some were also present without Sandy) General fixes: * Introduced some niceties like searchbox resizing properly, and toolbars wrapping automatically in Sandy, rather than buttons becoming invisible * Containers don't grow anymore by default, but take size of contents * ScrollContainer child is now a Layout.Vertical. Layout.Vertical should be used as default container everywhere (e.g. Tabs, Panels) in the future * Fixed layout issue if a split container had only 1 visible child * DetailsSidebar now scrolls vertically by default * Details sidebar would sometimes render content in-place rather than in the reserved area * AppSelector dropdown and Plugin list will now properly ellipse (...) if there is not enough space Plugin fixes: * Long database / table names in Database plugin would break layout Also fixes https://github.com/facebook/flipper/issues/1611 Reviewed By: passy Differential Revision: D24454188 fbshipit-source-id: c60c867270900a1d4f28587d47067c6ec1072ede
2020-10-22 09:37:26 -07:00
parent 4f7294c96d
commit 966d748ace
17 changed files with 155 additions and 79 deletions
--- a/desktop/app/src/PluginContainer.tsx
+++ b/desktop/app/src/PluginContainer.tsx
@@ -49,6 +49,7 @@ import {ToggleButton, SmallText, Layout} from './ui';
 import {SandyPluginRenderer} from 'flipper-plugin';
 import {isDevicePluginDefinition} from './utils/pluginUtils';
 import ArchivedDevice from './devices/ArchivedDevice';
+import {ContentContainer} from './sandy-chrome/ContentContainer';

 const Container = styled(FlexColumn)({
  width: 0,
@@ -436,7 +437,7 @@ class PluginContainer extends PureComponent<Props, State> {
          heading={`Plugin "${
            activePlugin.title || 'Unknown'
          }" encountered an error during render`}>
-          {pluginElement}
+          <ContentContainer>{pluginElement}</ContentContainer>
        </ErrorBoundary>
        <SidebarContainer id="detailsSidebar" />
      </Layout.Right>
--- a/desktop/app/src/chrome/DetailSidebar.tsx
+++ b/desktop/app/src/chrome/DetailSidebar.tsx
@@ -7,14 +7,14 @@
 * @format
 */

-import React, {useEffect, useMemo, useContext} from 'react';
+import React, {useEffect, useState} from 'react';
 import ReactDOM from 'react-dom';
-import {ReactReduxContext} from 'react-redux';
 import Sidebar from '../ui/components/Sidebar';
 import {toggleRightSidebarAvailable} from '../reducers/application';
 import {useDispatch, useStore} from '../utils/useStore';
 import {useIsSandy} from '../sandy-chrome/SandyContext';
 import {ContentContainer} from '../sandy-chrome/ContentContainer';
+import {Layout} from '../ui';

 type OwnProps = {
  children: any;
@@ -24,12 +24,13 @@ type OwnProps = {

 /* eslint-disable react-hooks/rules-of-hooks */
 export default function DetailSidebar({children, width, minWidth}: OwnProps) {
-  const reduxContext = useContext(ReactReduxContext);
-  const domNode = useMemo(() => document.getElementById('detailsSidebar'), []);
+  const [domNode, setDomNode] = useState(
+    document.getElementById('detailsSidebar'),
+  );

-  if (!reduxContext || !domNode) {
+  if (typeof jest !== 'undefined') {
    // For unit tests, make sure to render elements inline
-    return <div id="detailsSidebar">{children}</div>;
+    return <div>{children}</div>;
  }

  const isSandy = useIsSandy();
@@ -49,6 +50,19 @@ export default function DetailSidebar({children, width, minWidth}: OwnProps) {
    [children, rightSidebarAvailable, dispatch],
  );

+  // If the plugin container is mounting and rendering a sidbar immediately, the domNode might not yet be available
+  useEffect(() => {
+    if (!domNode) {
+      const newDomNode = document.getElementById('detailsSidebar');
+      if (!newDomNode) {
+        // if after layouting domNode is still not available, something is wrong...
+        console.error('Failed to obtain detailsSidebar node');
+      } else {
+        setDomNode(newDomNode);
+      }
+    }
+  }, [domNode]);
+
  return (
    (children &&
      rightSidebarVisible &&
@@ -59,7 +73,15 @@ export default function DetailSidebar({children, width, minWidth}: OwnProps) {
          width={width || 300}
          position="right"
          gutter={isSandy}>
-          {isSandy ? <ContentContainer>{children}</ContentContainer> : children}
+          {isSandy ? (
+            <ContentContainer>
+              <Layout.ScrollContainer vertical>
+                {children}
+              </Layout.ScrollContainer>
+            </ContentContainer>
+          ) : (
+            children
+          )}
        </Sidebar>,
        domNode,
      )) ||
--- a/desktop/app/src/chrome/plugin-manager/tests/snapshots/PluginInstaller.node.tsx.snap
+++ b/desktop/app/src/chrome/plugin-manager/tests/snapshots/PluginInstaller.node.tsx.snap
@@ -9,10 +9,10 @@ exports[`load PluginInstaller list 1`] = `
      class="css-1qqef1i-View-FlexBox-FlexRow-ToolbarContainer e13mj6h80"
    >
      <div
-        class="css-awcbnc-View-FlexBox-SearchBox e271nro1"
+        class="css-1dulget-View-FlexBox-SearchBox e271nro1"
      >
        <input
-          class="css-mquw9q-Input-SearchInput e271nro2"
+          class="css-184o3nx-Input-SearchInput e271nro2"
          placeholder="Search Flipper plugins..."
          type="text"
          value=""
@@ -304,10 +304,10 @@ exports[`load PluginInstaller list with one plugin installed 1`] = `
      class="css-1qqef1i-View-FlexBox-FlexRow-ToolbarContainer e13mj6h80"
    >
      <div
-        class="css-awcbnc-View-FlexBox-SearchBox e271nro1"
+        class="css-1dulget-View-FlexBox-SearchBox e271nro1"
      >
        <input
-          class="css-mquw9q-Input-SearchInput e271nro2"
+          class="css-184o3nx-Input-SearchInput e271nro2"
          placeholder="Search Flipper plugins..."
          type="text"
          value=""
--- a/desktop/app/src/reducers/connections.tsx
+++ b/desktop/app/src/reducers/connections.tsx
@@ -124,7 +124,7 @@ export type Action =
    }
  | {
      type: 'SELECT_CLIENT';
-      payload: string;
+      payload: string | null;
    }
  | RegisterPluginAction
  | {
@@ -404,7 +404,7 @@ export const starPlugin = (payload: {
  payload,
 });

-export const selectClient = (clientId: string): Action => ({
+export const selectClient = (clientId: string | null): Action => ({
  type: 'SELECT_CLIENT',
  payload: clientId,
 });
--- a/desktop/app/src/sandy-chrome/ContentContainer.tsx
+++ b/desktop/app/src/sandy-chrome/ContentContainer.tsx
@@ -11,6 +11,7 @@ import {Layout, styled} from '../ui';
 import {theme} from './theme';

 export const ContentContainer = styled(Layout.Container)({
+  flex: 1,
  overflow: 'hidden',
  background: theme.backgroundDefault,
  border: `1px solid ${theme.dividerColor}`,
--- a/desktop/app/src/sandy-chrome/LeftSidebar.tsx
+++ b/desktop/app/src/sandy-chrome/LeftSidebar.tsx
@@ -15,9 +15,9 @@ import {Button, Tooltip, Typography} from 'antd';
 import {InfoCircleOutlined} from '@ant-design/icons';

 export const LeftSidebar: React.FC = ({children}) => (
-  <Layout.Container borderRight padv="small">
+  <Layout.Vertical borderRight padv="small" grow shrink>
    {children}
-  </Layout.Container>
+  </Layout.Vertical>
 );

 export function SidebarTitle({
--- a/desktop/app/src/sandy-chrome/appinspect/AppSelector.tsx
+++ b/desktop/app/src/sandy-chrome/appinspect/AppSelector.tsx
@@ -63,7 +63,13 @@ export function AppSelector() {
  const client = clients.find((client) => client.id === selectedApp);

  return (
-    <Radio.Group value={selectedApp} size="small">
+    <Radio.Group
+      value={selectedApp}
+      size="small"
+      style={{
+        display: 'flex',
+        flex: 1,
+      }}>
      <Dropdown
        overlay={
          <Menu selectedKeys={selectedApp ? [selectedApp] : []}>
@@ -73,7 +79,7 @@ export function AppSelector() {
        <AppInspectButton title="Select the device / app to inspect">
          <Layout.Horizontal gap center>
            <AppIcon appname={client?.query.app} />
-            <Layout.Vertical>
+            <Layout.Vertical grow shrink>
              <Text strong>{client?.query.app ?? ''}</Text>
              <Text>
                {selectedDevice?.displayTitle() || 'Available devices'}
@@ -91,8 +97,9 @@ const AppInspectButton = styled(Button)({
  background: theme.backgroundTransparentHover,
  height: 52,
  border: 'none',
-  width: '100%',
  fontWeight: 'normal',
+  flex: `1 1 0`,
+  overflow: 'hidden', // required for ellipsis
  paddingLeft: theme.space.small,
  paddingRight: theme.space.small,
  textAlign: 'left',
@@ -128,7 +135,10 @@ function computeEntries(
        key={device.serial}
        style={{fontWeight: 'bold'}}
        onClick={() => {
+          batch(() => {
            dispatch(selectDevice(device));
+            dispatch(selectClient(null));
+          });
        }}>
        {device.displayTitle()}
      </Menu.Item>
--- a/desktop/app/src/sandy-chrome/appinspect/PluginList.tsx
+++ b/desktop/app/src/sandy-chrome/appinspect/PluginList.tsx
@@ -454,6 +454,10 @@ export function computePluginLists(
 const PluginMenu = styled(Menu)({
  userSelect: 'none',
  border: 'none',
+  '.ant-typography': {
+    overflow: 'hidden',
+    textOverflow: 'ellipsis',
+  },
  '.ant-menu-inline .ant-menu-item, .ant-menu-inline .ant-menu-submenu-title ': {
    width: '100%', // reset to remove weird bonus pixel from ANT
  },
--- a/desktop/app/src/ui/components/Layout.tsx
+++ b/desktop/app/src/ui/components/Layout.tsx
@@ -31,6 +31,10 @@ type ContainerProps = {
  rounded?: boolean;
  width?: number;
  height?: number;
+  // grow to available space?
+  grow?: boolean;
+  // allow shrinking beyond minally needed size? Makes using ellipsis on children possible
+  shrink?: boolean;
 } & PaddingProps;

 const Container = styled.div<ContainerProps>(
@@ -43,17 +47,19 @@ const Container = styled.div<ContainerProps>(
    rounded,
    width,
    height,
+    grow,
+    shrink,
    ...rest
  }) => ({
-    boxSizing: 'border-box',
-    minWidth: `0`, // ensures the Container can shrink smaller than it's largest
-    width,
-    height,
    display: 'flex',
    flexDirection: 'column',
+    flex: grow && shrink ? `1 1 0` : grow ? `1 0 auto` : shrink ? `0 1 0` : 0,
+    minWidth: shrink ? 0 : undefined,
+    boxSizing: 'border-box',
+    width,
+    height,
    padding: normalizePadding(rest),
    borderRadius: rounded ? theme.containerBorderRadius : undefined,
-    flex: 1,
    borderStyle: 'solid',
    borderColor: theme.dividerColor,
    borderWidth: bordered
@@ -64,6 +70,34 @@ const Container = styled.div<ContainerProps>(
  }),
 );

+type DistributionProps = ContainerProps & {
+  /**
+   * Gab between individual items
+   */
+  gap?: Spacing;
+  /**
+   * If set, items will be aligned in the center, if false (the default) items will be stretched.
+   */
+  center?: boolean;
+};
+
+function distributionStyle({gap, center}: DistributionProps) {
+  return {
+    gap: normalizeSpace(gap, theme.space.small),
+    alignItems: center ? 'center' : 'stretch',
+  };
+}
+
+const Horizontal = styled(Container)<DistributionProps>((props) => ({
+  ...distributionStyle(props),
+  flexDirection: 'row',
+}));
+
+const Vertical = styled(Container)<DistributionProps>((props) => ({
+  ...distributionStyle(props),
+  flexDirection: 'column',
+}));
+
 const ScrollParent = styled.div<{axis?: ScrollAxis}>(({axis}) => ({
  flex: 1,
  boxSizing: 'border-box',
@@ -72,7 +106,7 @@ const ScrollParent = styled.div<{axis?: ScrollAxis}>(({axis}) => ({
  overflowY: axis === 'x' ? 'hidden' : 'auto',
 }));

-const ScrollChild = styled.div<{axis?: ScrollAxis}>(({axis}) => ({
+const ScrollChild = styled(Vertical)<{axis?: ScrollAxis}>(({axis}) => ({
  position: 'absolute',
  minHeight: '100%',
  minWidth: '100%',
@@ -100,32 +134,6 @@ const ScrollContainer = ({
  ) as any;
 };

-type DistributionProps = ContainerProps & {
-  /**
-   * Gab between individual items
-   */
-  gap?: Spacing;
-  /**
-   * If set, items will be aligned in the center, if false (the default) items will be stretched.
-   */
-  center?: boolean;
-};
-
-const Horizontal = styled(Container)<DistributionProps>(({gap, center}) => ({
-  display: 'flex',
-  flexDirection: 'row',
-  gap: normalizeSpace(gap, theme.space.small),
-  alignItems: center ? 'center' : 'stretch',
-  minWidth: 'auto', // corrects 0 on Container
-}));
-
-const Vertical = styled(Container)<DistributionProps>(({gap, center}) => ({
-  display: 'flex',
-  flexDirection: 'column',
-  gap: normalizeSpace(gap, theme.space.small),
-  alignItems: center ? 'center' : 'stretch',
-}));
-
 type SplitLayoutProps = {
  /**
   * If set, the dynamically sized pane will get scrollbars when needed
@@ -219,10 +227,10 @@ const SandySplitContainer = styled.div<{
  flexDirection: props.flexDirection,
  alignItems: props.center ? 'center' : 'stretch',
  overflow: 'hidden',
-  '> :first-child': {
+  '> :nth-child(1)': {
    flex: props.grow === 1 ? growStyle : fixedStyle,
  },
-  '> :last-child': {
+  '> :nth-child(2)': {
    flex: props.grow === 2 ? growStyle : fixedStyle,
  },
 }));
--- a/desktop/app/src/ui/components/Panel.tsx
+++ b/desktop/app/src/ui/components/Panel.tsx
@@ -7,7 +7,7 @@
 * @format
 */

-import React from 'react';
+import React, {CSSProperties} from 'react';
 import styled from '@emotion/styled';
 import FlexColumn from './FlexColumn';
 import FlexBox from './FlexBox';
@@ -70,6 +70,7 @@ export default class Panel extends React.Component<
     * padding is applied to the heading.
     */
    accessory?: React.ReactNode;
+    style?: CSSProperties;
  },
  {
    collapsed: boolean;
@@ -88,8 +89,10 @@ export default class Panel extends React.Component<
  static PanelContainer = styled(FlexColumn)<{
    floating?: boolean;
    collapsed?: boolean;
+    grow?: boolean;
  }>((props) => ({
    flexShrink: 0,
+    flexGrow: props.grow ? 1 : undefined,
    padding: props.floating ? 10 : 0,
    borderBottom: props.collapsed ? 'none' : BORDER,
  }));
@@ -141,6 +144,7 @@ export default class Panel extends React.Component<
      heading,
      collapsable,
      accessory,
+      style,
    } = this.props;
    const {collapsed} = this.state;
    return (
@@ -148,7 +152,8 @@ export default class Panel extends React.Component<
        className={className}
        floating={floating}
        grow={grow}
-        collapsed={collapsed}>
+        collapsed={collapsed}
+        style={style}>
        <Panel.PanelHeader
          floating={floating}
          padded={padded || typeof heading === 'string'}
--- a/desktop/app/src/ui/components/Select.tsx
+++ b/desktop/app/src/ui/components/Select.tsx
@@ -7,7 +7,7 @@
 * @format
 */

-import {Component} from 'react';
+import {Component, CSSProperties} from 'react';
 import Text from './Text';
 import styled from '@emotion/styled';
 import React from 'react';
@@ -54,6 +54,7 @@ export default class Select extends Component<{

  /** Whether the user can interact with the select and change the selcted option */
  disabled?: boolean;
+  style?: CSSProperties;
 }> {
  selectID: string = Math.random().toString(36);

@@ -67,7 +68,15 @@ export default class Select extends Component<{
  };

  render() {
-    const {className, options, selected, label, grow, disabled} = this.props;
+    const {
+      className,
+      options,
+      selected,
+      label,
+      grow,
+      disabled,
+      style,
+    } = this.props;

    let select = (
      <SelectMenu
@@ -76,7 +85,8 @@ export default class Select extends Component<{
        onChange={this.onChange}
        className={className}
        disabled={disabled}
-        value={selected || ''}>
+        value={selected || ''}
+        style={style}>
        {Object.keys(options).map((key, index) => (
          <option value={key} key={index}>
            {options[key]}
--- a/desktop/app/src/ui/components/Tabs.tsx
+++ b/desktop/app/src/ui/components/Tabs.tsx
@@ -104,6 +104,7 @@ const TabContent = styled.div({
  height: '100%',
  overflow: 'auto',
  width: '100%',
+  display: 'flex',
 });
 TabContent.displayName = 'Tabs:TabContent';

--- a/desktop/app/src/ui/components/Toolbar.tsx
+++ b/desktop/app/src/ui/components/Toolbar.tsx
@@ -12,9 +12,9 @@ import {colors} from './colors';
 import FlexRow from './FlexRow';
 import FlexBox from './FlexBox';
 import styled from '@emotion/styled';
-import {Space} from 'antd';
 import {useIsSandy} from '../../sandy-chrome/SandyContext';
 import {theme} from '../../sandy-chrome/theme';
+import Layout from './Layout';

 /**
 * A toolbar.
@@ -42,8 +42,8 @@ const ToolbarContainer = styled(FlexRow)<{
 }));
 ToolbarContainer.displayName = 'ToolbarContainer';

-const SandyToolbarContainer = styled(Space)({
-  width: '100%',
+const SandyToolbarContainer = styled(Layout.Horizontal)({
+  flexWrap: 'wrap',
  padding: theme.space.small,
  boxShadow: `inset 0px -1px 0px ${theme.dividerColor}`,
 });
@@ -65,7 +65,9 @@ export default function Toolbar({
 }) {
  const isSandy = useIsSandy();
  return isSandy ? (
-    <SandyToolbarContainer style={style}>{children}</SandyToolbarContainer>
+    <SandyToolbarContainer style={style} gap={theme.space.small} center>
+      {children}
+    </SandyToolbarContainer>
  ) : (
    <ToolbarContainer style={style} {...rest}>
      {children}
--- a/desktop/app/src/ui/components/searchable/Searchable.tsx
+++ b/desktop/app/src/ui/components/searchable/Searchable.tsx
@@ -11,7 +11,6 @@ import {Filter} from '../filter/types';
 import {TableColumns} from '../table/types';
 import {PureComponent} from 'react';
 import Toolbar from '../Toolbar';
-import FlexRow from '../FlexRow';
 import Input from '../Input';
 import {colors} from '../colors';
 import Text from '../Text';
@@ -23,6 +22,7 @@ import {debounce} from 'lodash';
 import ToggleButton from '../ToggleSwitch';
 import React from 'react';
 import Layout from '../Layout';
+import {theme} from '../../../sandy-chrome/theme';

 const SearchBar = styled(Toolbar)({
  height: 42,
@@ -33,13 +33,14 @@ SearchBar.displayName = 'Searchable:SearchBar';
 export const SearchBox = styled(FlexBox)<{isInvalidInput?: boolean}>(
  (props) => {
    return {
+      flex: `1 0 0`,
+      minWidth: 150,
+      height: 30,
      backgroundColor: colors.white,
      borderRadius: '999em',
      border: `1px solid ${
        !props.isInvalidInput ? colors.light15 : colors.red
      }`,
-      height: '100%',
-      width: '100%',
      alignItems: 'center',
      paddingLeft: 4,
    };
@@ -60,6 +61,7 @@ export const SearchInput = styled(Input)<{
  height: 'auto',
  lineHeight: '100%',
  marginLeft: 2,
+  marginRight: 8,
  width: '100%',
  color: props.regex && !props.isValidInput ? colors.red : colors.black,
  '&::-webkit-input-placeholder': {
@@ -97,9 +99,8 @@ export const SearchIcon = styled(Glyph)({
 });
 SearchIcon.displayName = 'Searchable:SearchIcon';

-const Actions = styled(FlexRow)({
+const Actions = styled(Layout.Horizontal)({
  marginLeft: 8,
-  flexShrink: 0,
 });
 Actions.displayName = 'Searchable:Actions';

@@ -533,7 +534,9 @@ export default function Searchable(
                }
              />
            ) : null}
-            {actions != null && <Actions>{actions}</Actions>}
+            {actions != null && (
+              <Actions gap={theme.space.small}>{actions}</Actions>
+            )}
          </SearchBar>
          <Component
            {...props}
--- a/desktop/plugins/databases/index.tsx
+++ b/desktop/plugins/databases/index.tsx
@@ -1283,12 +1283,14 @@ export default class DatabasesPlugin extends FlipperPlugin<
                this.state.databases[this.state.selectedDatabase - 1]?.name
              }
              onChange={this.onDatabaseSelected}
+              style={{maxWidth: 300}}
            />
            <BoldSpan style={{marginLeft: 16, marginRight: 16}}>Table</BoldSpan>
            <Select
              options={tableOptions}
              selected={this.state.selectedDatabaseTable}
              onChange={this.onDatabaseTableSelected}
+              style={{maxWidth: 300}}
            />
            <div />
            <Button onClick={this.onRefreshClicked}>Refresh</Button>
--- a/desktop/plugins/network/index.tsx
+++ b/desktop/plugins/network/index.tsx
@@ -531,9 +531,7 @@ export function Component() {
          searchTerm={searchTerm}
          isMockResponseSupported={isMockResponseSupported}
        />
-        <DetailSidebar width={500}>
        <Sidebar />
-        </DetailSidebar>
      </NetworkRouteContext.Provider>
    </FlexColumn>
  );
@@ -759,6 +757,7 @@ function Sidebar() {
  }

  return (
+    <DetailSidebar width={500}>
      <RequestDetails
        key={selectedId}
        request={requestWithId}
@@ -766,6 +765,7 @@ function Sidebar() {
        bodyFormat={detailBodyFormat}
        onSelectFormat={instance.onSelectFormat}
      />
+    </DetailSidebar>
  );
 }

--- a/desktop/static/icons.json
+++ b/desktop/static/icons.json
@@ -64,6 +64,7 @@
  "caution-triangle": [
    12,
    16,
+    20,
    24
  ],
  "caution": [
@@ -512,5 +513,11 @@
  ],
  "app-flash": [
    16
+  ],
+  "sample-lo": [
+    20
+  ],
+  "point": [
+    20
  ]
 }