From 30f5f0b59a4f770940586c09dc31c202f98a4026 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Fri, 23 Oct 2020 06:44:04 -0700 Subject: [PATCH] Small design system simplifications Summary: So far we distinguished `Layout.Container` from `Layout.Vertical`, but they did almost exactly the same, so figured to unify them, so smaller API ftw :) Normal containers put children vertical, and if you want to use horizontal, use Layout.Horzontal Also simplified code in Layout file a little bit. Fixed issue I split container where the main container didn't go "underneath" the fixed container Reviewed By: cekkaewnumchai Differential Revision: D24502547 fbshipit-source-id: 517db3692749e670cda8f0cd7cb1c807df818b4d --- .../src/sandy-chrome/DesignComponentDemos.tsx | 145 ++++++++---------- desktop/app/src/sandy-chrome/LeftRail.tsx | 10 +- desktop/app/src/sandy-chrome/LeftSidebar.tsx | 4 +- .../src/sandy-chrome/SandyDesignSystem.tsx | 4 +- .../src/sandy-chrome/SetupDoctorScreen.tsx | 4 +- .../sandy-chrome/appinspect/AppInspect.tsx | 4 +- .../sandy-chrome/appinspect/AppSelector.tsx | 4 +- .../appinspect/LaunchEmulator.tsx | 6 +- .../notification/Notification.tsx | 12 +- desktop/app/src/ui/components/Layout.tsx | 120 ++++++--------- desktop/app/src/ui/components/Toolbar.tsx | 2 +- .../ui/components/searchable/Searchable.tsx | 2 +- desktop/app/src/ui/index.tsx | 2 +- 13 files changed, 137 insertions(+), 182 deletions(-) diff --git a/desktop/app/src/sandy-chrome/DesignComponentDemos.tsx b/desktop/app/src/sandy-chrome/DesignComponentDemos.tsx index 7b64b1bf5..073eaee6a 100644 --- a/desktop/app/src/sandy-chrome/DesignComponentDemos.tsx +++ b/desktop/app/src/sandy-chrome/DesignComponentDemos.tsx @@ -67,8 +67,7 @@ const someText = Some text; const demos: PreviewProps[] = [ { title: 'Layout.Container', - description: - 'Layout.Container can be used to organize the UI in regions. It takes care of paddings and borders. To arrange multiple children use one of the other Layout components. If you need a margin on this component, try to wrap it in other Layout component instead.', + description: `Layout.Container can be used to organize the UI in regions. It takes care of paddings and borders. Children will be arranged vertically. Use Layout.Horizontal instead for arranging children horizontally. If you need a margin on this component, try to wrap it in other Layout component instead.`, props: [ ['rounded', 'boolean (false)', 'Make the corners rounded'], [ @@ -91,6 +90,16 @@ const demos: PreviewProps[] = [ 'boolean (false)', 'Use a standard padding on the top side', ], + [ + 'gap', + 'true / number (0)', + 'Set the spacing between children. If just set, theme.space.small will be used.', + ], + [ + 'center', + 'boolean (false)', + 'If set, all children will use their own naturally width, and they will be centered horizontally in the Container. If not set, all children will be stretched to the width of the Container.', + ], ], demos: { 'Basic container with fixed dimensions': ( @@ -112,6 +121,52 @@ const demos: PreviewProps[] = [
child
), + 'Multiple children, gap={24}': ( + + {aButton} + {someText} + {aBox} + {aDynamicBox} + + ), + 'Multiple children icmw. pad center gap': ( + + {aButton} + {someText} + {aBox} + {aDynamicBox} + + ), + }, + }, + { + title: 'Layout.Horizontal', + description: + 'Use this component to arrange multiple items horizontally. All vanilla Container props can be used as well.', + props: [ + [ + 'center', + 'boolean (false)', + 'If set, all children will use their own height, and they will be centered vertically in the layout. If not set, all children will be stretched to the height of the layout.', + ], + ], + demos: { + 'Basic usage, gap="large"': ( + + {aButton} + {someText} + {aBox} + {aDynamicBox} + + ), + 'Using flags: pad center gap={8} (great for toolbars and such)': ( + + {aButton} + {someText} + {aBox} + {aDynamicBox} + + ), }, }, { @@ -139,87 +194,17 @@ const demos: PreviewProps[] = [ width: 100, border: `2px solid ${theme.primaryColor}`, }}> - + This text is truncated because it is too long and scroll is vertical only... {largeChild} - + ), }, }, - { - title: 'Layout.Horizontal', - description: - 'Use this component to arrange multiple items horizontally. All vanilla Container props can be used as well.', - props: [ - [ - 'gap', - Object.keys(theme.space).join(' | ') + ' | number | true', - 'Set the spacing between children. For `true` theme.space.small should be used. Defaults to 0.', - ], - [ - 'center', - 'boolean (false)', - 'If set, all children will use their own height, and they will be centered vertically in the layout. If not set, all children will be stretched to the height of the layout.', - ], - ], - demos: { - 'Basic usage, gap="large"': ( - - {aButton} - {someText} - {aBox} - {aDynamicBox} - - ), - 'Using flags: pad center gap={8} (great for toolbars and such)': ( - - {aButton} - {someText} - {aBox} - {aDynamicBox} - - ), - }, - }, - { - title: 'Layout.Vertical', - description: - 'Use this component to arrange multiple items vertically. All vanilla Container props can be used as well.', - props: [ - [ - 'gap', - 'number (0)', - 'Set the spacing between children. Typically theme.space.small should be used.', - ], - [ - 'center', - 'boolean (false)', - 'If set, all children will use their own height, and they will be centered vertically in the layout. If not set, all children will be stretched to the height of the layout.', - ], - ], - demos: { - 'Basic usage, gap={24}': ( - - {aButton} - {someText} - {aBox} - {aDynamicBox} - - ), - 'Using flags: pad center gap (great for toolbars and such)': ( - - {aButton} - {someText} - {aBox} - {aDynamicBox} - - ), - }, - }, { title: 'Layout.Top|Left|Right|Bottom', description: @@ -300,11 +285,11 @@ const demos: PreviewProps[] = [ function ComponentPreview({title, demos, description, props}: PreviewProps) { return ( - + {description} - + {Object.entries(demos).map(([name, children]) => (
@@ -330,7 +315,7 @@ function ComponentPreview({title, demos, description, props}: PreviewProps) {
))} - + - + ); } export const DesignComponentDemos = () => ( - + {demos.map((demo) => ( ))} - + ); diff --git a/desktop/app/src/sandy-chrome/LeftRail.tsx b/desktop/app/src/sandy-chrome/LeftRail.tsx index 5107d477c..099e79619 100644 --- a/desktop/app/src/sandy-chrome/LeftRail.tsx +++ b/desktop/app/src/sandy-chrome/LeftRail.tsx @@ -36,7 +36,7 @@ import {ToplevelProps} from './SandyApp'; import {useValue} from 'flipper-plugin'; import {logout} from '../reducers/user'; import config from '../fb-stubs/config'; -import Layout from '../ui/components/Layout'; +import {Layout} from '../ui/components/Layout'; import styled from '@emotion/styled'; const LeftRailButtonElem = styled(Button)<{kind?: 'small'}>(({kind}) => ({ @@ -108,7 +108,7 @@ export function LeftRail({ return ( - + } title="App Inspect" @@ -127,8 +127,8 @@ export function LeftRail({ toplevelSelection={toplevelSelection} setToplevelSelection={setToplevelSelection} /> - - + + @@ -140,7 +140,7 @@ export function LeftRail({ {config.showLogin && } - + ); diff --git a/desktop/app/src/sandy-chrome/LeftSidebar.tsx b/desktop/app/src/sandy-chrome/LeftSidebar.tsx index 2dd8db93f..86b47d5ed 100644 --- 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}) => ( - + {children} - + ); export function SidebarTitle({ diff --git a/desktop/app/src/sandy-chrome/SandyDesignSystem.tsx b/desktop/app/src/sandy-chrome/SandyDesignSystem.tsx index 053075584..4e4385b14 100644 --- a/desktop/app/src/sandy-chrome/SandyDesignSystem.tsx +++ b/desktop/app/src/sandy-chrome/SandyDesignSystem.tsx @@ -19,7 +19,7 @@ const {Title, Text, Link} = Typography; export default function SandyDesignSystem() { return ( - +

Welcome to the Flipper Design System. The Flipper design system is @@ -122,7 +122,7 @@ export default function SandyDesignSystem() { - + ); } diff --git a/desktop/app/src/sandy-chrome/SetupDoctorScreen.tsx b/desktop/app/src/sandy-chrome/SetupDoctorScreen.tsx index 1b1604680..5769ce100 100644 --- a/desktop/app/src/sandy-chrome/SetupDoctorScreen.tsx +++ b/desktop/app/src/sandy-chrome/SetupDoctorScreen.tsx @@ -160,7 +160,7 @@ function CollapsableCategory(props: {checks: Array}) { function HealthCheckList(props: {report: HealthcheckReport}) { useEffect(() => reportUsage('doctor:report:opened'), []); return ( - + {Object.values(props.report.categories).map((category) => ( @@ -187,7 +187,7 @@ function HealthCheckList(props: {report: HealthcheckReport}) { /> ))} - + ); } diff --git a/desktop/app/src/sandy-chrome/appinspect/AppInspect.tsx b/desktop/app/src/sandy-chrome/appinspect/AppInspect.tsx index b6f0943db..920603204 100644 --- a/desktop/app/src/sandy-chrome/appinspect/AppInspect.tsx +++ b/desktop/app/src/sandy-chrome/appinspect/AppInspect.tsx @@ -40,7 +40,7 @@ export function AppInspect() { {appTooltip}}> App Inspect - + } defaultValue="mysite" /> @@ -55,7 +55,7 @@ export function AppInspect() { }} /> - + diff --git a/desktop/app/src/sandy-chrome/appinspect/AppSelector.tsx b/desktop/app/src/sandy-chrome/appinspect/AppSelector.tsx index 932db2ff9..d9a03d310 100644 --- a/desktop/app/src/sandy-chrome/appinspect/AppSelector.tsx +++ b/desktop/app/src/sandy-chrome/appinspect/AppSelector.tsx @@ -79,12 +79,12 @@ export function AppSelector() { - + {client?.query.app ?? ''} {selectedDevice?.displayTitle() || 'Available devices'} - + diff --git a/desktop/app/src/sandy-chrome/appinspect/LaunchEmulator.tsx b/desktop/app/src/sandy-chrome/appinspect/LaunchEmulator.tsx index b82ee82c2..43d99cb0f 100644 --- a/desktop/app/src/sandy-chrome/appinspect/LaunchEmulator.tsx +++ b/desktop/app/src/sandy-chrome/appinspect/LaunchEmulator.tsx @@ -14,7 +14,7 @@ import {renderReactRoot} from '../../utils/renderReactRoot'; import {Store} from '../../reducers'; import {useStore} from '../../utils/useStore'; import {launchEmulator} from '../../devices/AndroidDevice'; -import Layout from '../../ui/components/Layout'; +import {Layout} from '../../ui/components/Layout'; import { launchSimulator, getSimulators, @@ -100,9 +100,9 @@ export function LaunchEmulatorDialog({ title="Launch Emulator" footer={null} bodyStyle={{maxHeight: 400, overflow: 'auto'}}> - + {items.length ? items : } - + ); } diff --git a/desktop/app/src/sandy-chrome/notification/Notification.tsx b/desktop/app/src/sandy-chrome/notification/Notification.tsx index 5daad7784..0d61a289e 100644 --- a/desktop/app/src/sandy-chrome/notification/Notification.tsx +++ b/desktop/app/src/sandy-chrome/notification/Notification.tsx @@ -125,7 +125,7 @@ function NotificationEntry({notification}: {notification: PluginNotification}) { style: {color: theme.primaryColor}, }); return ( - + {icon} {pluginId} @@ -140,7 +140,7 @@ function NotificationEntry({notification}: {notification: PluginNotification}) { Open - + ); } @@ -151,14 +151,14 @@ function NotificationList({ }) { return ( - + {notifications.map((notification) => ( ))} - + ); } @@ -175,12 +175,12 @@ export function Notification() { return ( - + notifications } /> - + diff --git a/desktop/app/src/ui/components/Layout.tsx b/desktop/app/src/ui/components/Layout.tsx index fc8a13470..a8b53b265 100644 --- a/desktop/app/src/ui/components/Layout.tsx +++ b/desktop/app/src/ui/components/Layout.tsx @@ -35,6 +35,14 @@ type ContainerProps = { grow?: boolean; // allow shrinking beyond minally needed size? Makes using ellipsis on children possible shrink?: boolean; + /** + * 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; } & PaddingProps; const Container = styled.div( @@ -49,6 +57,8 @@ const Container = styled.div( height, grow, shrink, + gap, + center, ...rest }) => ({ display: 'flex', @@ -61,6 +71,9 @@ const Container = styled.div( : shrink ? `0 1 0` // allow shrinking smaller than natural size : `0 0 auto`, // (default) use natural size, don't grow or shrink (in parent flex direction) + alignItems: center ? 'center' : 'stretch', + gap: normalizeSpace(gap, theme.space.small), + minWidth: shrink ? 0 : undefined, boxSizing: 'border-box', width, @@ -77,33 +90,9 @@ const Container = styled.div( }), ); -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)((props) => ({ - ...distributionStyle(props), +const Horizontal = styled(Container)({ flexDirection: 'row', -})); - -const Vertical = styled(Container)((props) => ({ - ...distributionStyle(props), - flexDirection: 'column', -})); +}); const ScrollParent = styled.div<{axis?: ScrollAxis}>(({axis}) => ({ flex: 1, @@ -113,7 +102,7 @@ const ScrollParent = styled.div<{axis?: ScrollAxis}>(({axis}) => ({ overflowY: axis === 'x' ? 'hidden' : 'auto', })); -const ScrollChild = styled(Vertical)<{axis?: ScrollAxis}>(({axis}) => ({ +const ScrollChild = styled(Container)<{axis?: ScrollAxis}>(({axis}) => ({ position: 'absolute', minHeight: '100%', minWidth: '100%', @@ -153,6 +142,24 @@ type SplitLayoutProps = { children: [React.ReactNode, React.ReactNode]; }; +function renderSplitLayout( + props: SplitLayoutProps, + direction: 'column' | 'row', + grow: 1 | 2, +) { + // 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 = {child2}; + return ( + + {child1} + {child2} + + ); +} + /** * The Layout component divides all available screenspace over two components: * A fixed top (or left) component, and all remaining space to a bottom component. @@ -164,59 +171,22 @@ type SplitLayoutProps = { * * Use Layout.Top / Right / Bottom / Left to indicate where the fixed element should live. */ -const Layout = { +export const Layout = { Top(props: SplitLayoutProps) { - const isSandy = useIsSandy(); - if (!isSandy) return renderLayout(props, false, false); - let [child1, child2] = props.children; - if (props.scrollable) child2 = {child2}; - return ( - - {child1} - {child2} - - ); + return renderSplitLayout(props, 'column', 2); }, Bottom(props: SplitLayoutProps) { - const isSandy = useIsSandy(); - if (!isSandy) return renderLayout(props, false, true); - let [child1, child2] = props.children; - if (props.scrollable) child1 = {child1}; - return ( - - {child1} - {child2} - - ); + return renderSplitLayout(props, 'column', 1); }, Left(props: SplitLayoutProps) { - const isSandy = useIsSandy(); - if (!isSandy) return renderLayout(props, true, false); - let [child1, child2] = props.children; - if (props.scrollable) child2 = {child2}; - return ( - - {child1} - {child2} - - ); + return renderSplitLayout(props, 'row', 2); }, Right(props: SplitLayoutProps) { - const isSandy = useIsSandy(); - if (!isSandy) return renderLayout(props, true, true); - let [child1, child2] = props.children; - if (props.scrollable) child1 = {child1}; - return ( - - {child1} - {child2} - - ); + return renderSplitLayout(props, 'row', 1); }, Container, ScrollContainer, Horizontal, - Vertical, }; Object.keys(Layout).forEach((key) => { @@ -235,14 +205,14 @@ const SandySplitContainer = styled.div<{ alignItems: props.center ? 'center' : 'stretch', overflow: 'hidden', '> :nth-child(1)': { - flex: props.grow === 1 ? growStyle : fixedStyle, + flex: props.grow === 1 ? splitGrowStyle : splitFixedStyle, + minWidth: props.grow === 1 ? 0 : undefined, }, '> :nth-child(2)': { - flex: props.grow === 2 ? growStyle : fixedStyle, + flex: props.grow === 2 ? splitGrowStyle : splitFixedStyle, + minWidth: props.grow === 2 ? 0 : undefined, }, })); -const fixedStyle = `0 0 auto`; -const growStyle = `1 0 0`; - -export default Layout; +const splitFixedStyle = `0 0 auto`; +const splitGrowStyle = `1 0 0`; diff --git a/desktop/app/src/ui/components/Toolbar.tsx b/desktop/app/src/ui/components/Toolbar.tsx index f19818d96..9de45c186 100644 --- a/desktop/app/src/ui/components/Toolbar.tsx +++ b/desktop/app/src/ui/components/Toolbar.tsx @@ -14,7 +14,7 @@ import FlexBox from './FlexBox'; import styled from '@emotion/styled'; import {useIsSandy} from '../../sandy-chrome/SandyContext'; import {theme} from '../../sandy-chrome/theme'; -import Layout from './Layout'; +import {Layout} from './Layout'; /** * A toolbar. diff --git a/desktop/app/src/ui/components/searchable/Searchable.tsx b/desktop/app/src/ui/components/searchable/Searchable.tsx index 03583335c..861f6fa55 100644 --- a/desktop/app/src/ui/components/searchable/Searchable.tsx +++ b/desktop/app/src/ui/components/searchable/Searchable.tsx @@ -21,7 +21,7 @@ import styled from '@emotion/styled'; import {debounce} from 'lodash'; import ToggleButton from '../ToggleSwitch'; import React from 'react'; -import Layout from '../Layout'; +import {Layout} from '../Layout'; import {theme} from '../../../sandy-chrome/theme'; const SearchBar = styled(Toolbar)({ diff --git a/desktop/app/src/ui/index.tsx b/desktop/app/src/ui/index.tsx index 795e98f30..c27c2ecf9 100644 --- a/desktop/app/src/ui/index.tsx +++ b/desktop/app/src/ui/index.tsx @@ -178,7 +178,7 @@ export {default as CenteredView} from './components/CenteredView'; export {default as Info} from './components/Info'; export {default as Bordered} from './components/Bordered'; export {default as AlternatingRows} from './components/AlternatingRows'; -export {default as Layout} from './components/Layout'; +export {Layout} from './components/Layout'; export {default as Scrollable} from './components/Scrollable'; export * from './components/Highlight';