From f889dc5e40dd561cf3bba9865964ca284ae1bf0e Mon Sep 17 00:00:00 2001 From: Pritesh Nandgaonkar Date: Mon, 16 Mar 2020 05:20:11 -0700 Subject: [PATCH] Refactor Plugin Selection sheet and add snapshot test Summary: This diff fixes a UI bug in export data sheet where the plugin names are stuck to the left and also the bottom buttons are stuck together with no padding. I have also added snapshot test for the same. In order to write test and reduce the complexity I changed the `ExportDataPluginSheet`'s connect method. Bug: {F231521086} Reviewed By: mweststrate Differential Revision: D20459692 fbshipit-source-id: 1047d6b38738691d682ad6e4ccec45c05e14cbbe --- desktop/src/chrome/ExportDataPluginSheet.tsx | 43 +-- desktop/src/chrome/ListView.tsx | 16 +- .../__tests__/ExportDataPluginSheet.node.tsx | 108 +++++++ .../ExportDataPluginSheet.node.tsx.snap | 291 ++++++++++++++++++ 4 files changed, 424 insertions(+), 34 deletions(-) create mode 100644 src/chrome/__tests__/ExportDataPluginSheet.node.tsx create mode 100644 src/chrome/__tests__/__snapshots__/ExportDataPluginSheet.node.tsx.snap diff --git a/desktop/src/chrome/ExportDataPluginSheet.tsx b/desktop/src/chrome/ExportDataPluginSheet.tsx index c24621bf5..fedb20036 100644 --- a/desktop/src/chrome/ExportDataPluginSheet.tsx +++ b/desktop/src/chrome/ExportDataPluginSheet.tsx @@ -25,7 +25,7 @@ import { import ListView from './ListView'; import {Dispatch, Action} from 'redux'; import {unsetShare} from '../reducers/application'; -import {FlexColumn, styled} from 'flipper'; +import {FlexColumn, styled} from '../ui'; import Client from '../Client'; type OwnProps = { @@ -34,10 +34,8 @@ type OwnProps = { type StateFromProps = { share: ShareType | null; - plugins: PluginState; - pluginStates: PluginStatesState; - pluginMessageQueue: PluginMessageQueueState; - selectedClient: Client | undefined; + selectedPlugins: Array; + availablePluginsToExport: Array<{id: string; label: string}>; }; type DispatchFromProps = { @@ -58,25 +56,7 @@ const Container = styled(FlexColumn)({ padding: 8, }); -type State = { - availablePluginsToExport: Array<{id: string; label: string}>; -}; - -class ExportDataPluginSheet extends Component { - state: State = {availablePluginsToExport: []}; - static getDerivedStateFromProps(props: Props, _state: State): State { - const {plugins, pluginStates, pluginMessageQueue, selectedClient} = props; - const availablePluginsToExport = getActivePersistentPlugins( - pluginStates, - pluginMessageQueue, - plugins, - selectedClient, - ); - return { - availablePluginsToExport, - }; - } - +class ExportDataPluginSheet extends Component { render() { const {onHide} = this.props; const onHideWithUnsettingShare = () => { @@ -88,6 +68,7 @@ class ExportDataPluginSheet extends Component { { const {share} = this.props; if (!share) { @@ -116,8 +97,8 @@ class ExportDataPluginSheet extends Component { onChange={selectedArray => { this.props.setSelectedPlugins(selectedArray); }} - elements={this.state.availablePluginsToExport} - selectedElements={new Set(this.props.plugins.selectedPlugins)} + elements={this.props.availablePluginsToExport} + selectedElements={new Set(this.props.selectedPlugins)} onHide={onHideWithUnsettingShare} /> @@ -136,12 +117,16 @@ export default connect( const selectedClient = clients.find(o => { return o.id === selectedApp; }); - return { - share, - plugins, + const availablePluginsToExport = getActivePersistentPlugins( pluginStates, pluginMessageQueue, + plugins, selectedClient, + ); + return { + share, + selectedPlugins: plugins.selectedPlugins, + availablePluginsToExport, }; }, (dispatch: Dispatch>) => ({ diff --git a/desktop/src/chrome/ListView.tsx b/desktop/src/chrome/ListView.tsx index f190558b3..e653ab798 100644 --- a/desktop/src/chrome/ListView.tsx +++ b/desktop/src/chrome/ListView.tsx @@ -46,6 +46,7 @@ type Props = { onHide: () => any; elements: Array; title?: string; + leftPadding?: number; } & SubType; const Title = styled(Text)({ @@ -94,6 +95,7 @@ type RowComponentProps = { disabled: boolean; toolTipMessage?: string; type: SelectionType; + leftPadding?: number; }; class RowComponent extends Component { @@ -106,6 +108,7 @@ class RowComponent extends Component { disabled, toolTipMessage, type, + leftPadding, } = this.props; return ( @@ -116,7 +119,7 @@ class RowComponent extends Component { paddingRight={0} paddingTop={8} paddingBottom={8} - paddingLeft={0}> + paddingLeft={leftPadding || 0}> {label} @@ -191,7 +194,7 @@ export default class ListView extends Component { }; render() { - const {onSubmit, type} = this.props; + const {onSubmit, type, leftPadding} = this.props; return ( @@ -208,6 +211,7 @@ export default class ListView extends Component { onChange={this.handleChange} disabled={unselectable != null} toolTipMessage={unselectable?.toolTipMessage} + leftPadding={leftPadding} /> ); })} @@ -217,9 +221,11 @@ export default class ListView extends Component { - + + + {} +TestPlugin.title = 'TestPlugin'; +TestPlugin.id = 'TestPlugin'; +TestPlugin.defaultPersistedState = {msg: 'Test plugin'}; +class TestDevicePlugin extends FlipperDevicePlugin {} +TestDevicePlugin.title = 'TestDevicePlugin'; +TestDevicePlugin.id = 'TestDevicePlugin'; +TestDevicePlugin.defaultPersistedState = {msg: 'TestDevicePlugin'}; + +function getStore(selectedPlugins: Array) { + const logger = { + track: () => {}, + info: () => {}, + warn: () => {}, + error: () => {}, + debug: () => {}, + trackTimeSince: () => {}, + }; + let mockStore = configureStore([])(); + const selectedDevice = new BaseDevice( + 'serial', + 'emulator', + 'TestiPhone', + 'iOS', + ); + const client = new Client( + generateClientIdentifier(selectedDevice, 'app'), + {app: 'app', os: 'iOS', device: 'TestiPhone', device_id: 'serial'}, + null, + logger, + // @ts-ignore + mockStore, + ['TestPlugin', 'TestDevicePlugin'], + ); + + const pluginStates: {[key: string]: any} = {}; + pluginStates[`${client.id}#TestDevicePlugin`] = { + msg: 'Test Device plugin', + }; + pluginStates[`${client.id}#TestPlugin`] = { + msg: 'Test plugin', + }; + mockStore = configureStore([])({ + application: {share: {closeOnFinish: false, type: 'link'}}, + plugins: { + clientPlugins: new Map([['TestPlugin', TestPlugin]]), + devicePlugins: new Map([['TestDevicePlugin', TestDevicePlugin]]), + gatekeepedPlugins: [], + disabledPlugins: [], + failedPlugins: [], + selectedPlugins, + }, + pluginStates, + pluginMessageQueue: [], + connections: {selectedApp: client.id, clients: [client]}, + }); + return mockStore; +} + +test('SettingsSheet snapshot with nothing enabled', async () => { + let root: ReactTestRenderer; + await act(async () => { + root = create( + + {}} /> + , + ); + }); + + expect(root!.toJSON()).toMatchSnapshot(); +}); + +test('SettingsSheet snapshot with one plugin enabled', async () => { + let root: ReactTestRenderer; + await act(async () => { + root = create( + + {}} /> + , + ); + }); + + expect(root!.toJSON()).toMatchSnapshot(); +}); diff --git a/src/chrome/__tests__/__snapshots__/ExportDataPluginSheet.node.tsx.snap b/src/chrome/__tests__/__snapshots__/ExportDataPluginSheet.node.tsx.snap new file mode 100644 index 000000000..5e00f6ab4 --- /dev/null +++ b/src/chrome/__tests__/__snapshots__/ExportDataPluginSheet.node.tsx.snap @@ -0,0 +1,291 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`SettingsSheet snapshot with nothing enabled 1`] = ` +
+
+
+ + Select the plugins for which you want to export the data + +
+
+
+
+
+ + TestDevicePlugin + +
+ +
+
+
+
+
+
+
+
+
+ + TestPlugin + +
+ +
+
+
+
+
+
+
+
+
+
+
+
+ Close +
+
+
+
+ Submit +
+
+
+
+
+
+`; + +exports[`SettingsSheet snapshot with one plugin enabled 1`] = ` +
+
+
+ + Select the plugins for which you want to export the data + +
+
+
+
+
+ + TestDevicePlugin + +
+ +
+
+
+
+
+
+
+
+
+ + TestPlugin + +
+ +
+
+
+
+
+
+
+
+
+
+
+
+ Close +
+
+
+
+ Submit +
+
+
+
+
+
+`;