From 032b59422195efe2cae17a2d6ab6a4b047ab3938 Mon Sep 17 00:00:00 2001
From: Pritesh Nandgaonkar <prit91@fb.com>
Date: Thu, 23 Jan 2020 07:38:35 -0800
Subject: [PATCH] Refactor Listview to solve a bug

Summary:
There was a bug in ListView where the selected items where not getting updated.

Bug

In the following video I have selected Inspector and logs plugin from export drop down. When I click on litho support form it should select the default plugins which is Inspector, Mobile Config and Logs. But it only shows Inspector and Logs selected

{F226900949}

To fix it:

I call the callback `onChange` when the listview's row get updated, which will updates the props of the ListView component and hence rerendering it with updated selected rows.

Reviewed By: mweststrate

Differential Revision: D19518762

fbshipit-source-id: 39367590cbdc1d6f88afb467b17b71e13703bde3
---
 src/chrome/ExportDataPluginSheet.tsx | 67 ++++++++++++++++------------
 src/chrome/ListView.tsx              | 30 ++++---------
 2 files changed, 47 insertions(+), 50 deletions(-)

diff --git a/src/chrome/ExportDataPluginSheet.tsx b/src/chrome/ExportDataPluginSheet.tsx
index 388cea854..c82f299bb 100644
--- a/src/chrome/ExportDataPluginSheet.tsx
+++ b/src/chrome/ExportDataPluginSheet.tsx
@@ -41,7 +41,7 @@ type StateFromProps = {
 };
 
 type DispatchFromProps = {
-  selectedPlugins: (payload: Array<string>) => void;
+  setSelectedPlugins: (payload: Array<string>) => void;
   setActiveSheet: (payload: ActiveSheet) => void;
   setExportDataToFileActiveSheet: (payload: {
     file: string;
@@ -57,32 +57,43 @@ const Container = styled(FlexColumn)({
   maxHeight: 700,
 });
 
-class ExportDataPluginSheet extends Component<Props> {
-  render() {
-    const {
-      plugins,
-      pluginStates,
-      pluginMessageQueue,
-      onHide,
-      selectedClient,
-    } = this.props;
-    const onHideWithUnsettingShare = () => {
-      this.props.unsetShare();
-      onHide();
-    };
-    const pluginsToExport = getActivePersistentPlugins(
+type State = {
+  availablePluginsToExport: Array<string>;
+};
+
+class ExportDataPluginSheet extends Component<Props, State> {
+  state: State = {availablePluginsToExport: []};
+  static getDerivedStateFromProps(props: Props, _state: State) {
+    const {plugins, pluginStates, pluginMessageQueue, selectedClient} = props;
+    const availablePluginsToExport = getActivePersistentPlugins(
       pluginStates,
       pluginMessageQueue,
       plugins,
       selectedClient,
     );
+    return {
+      availablePluginsToExport,
+    };
+  }
+
+  componentDidMount() {
+    if (this.props.plugins.selectedPlugins.length <= 0) {
+      this.props.setSelectedPlugins(this.state.availablePluginsToExport);
+    }
+  }
+
+  render() {
+    const {onHide} = this.props;
+    const onHideWithUnsettingShare = () => {
+      this.props.unsetShare();
+      onHide();
+    };
     return (
       <Container>
         <ListView
           type="multiple"
           title="Select the plugins for which you want to export the data"
-          onSelect={selectedArray => {
-            this.props.selectedPlugins(selectedArray);
+          onSubmit={() => {
             const {share} = this.props;
             if (!share) {
               console.error(
@@ -107,18 +118,18 @@ class ExportDataPluginSheet extends Component<Props> {
               }
             }
           }}
-          elements={pluginsToExport}
-          selectedElements={pluginsToExport.reduce((acc, plugin) => {
-            if (
-              plugins.selectedPlugins.length <= 0 ||
-              plugins.selectedPlugins.includes(plugin)
-            ) {
-              acc.add(plugin);
+          onChange={selectedArray => {
+            if (selectedArray.length > 0) {
+              this.props.setSelectedPlugins(selectedArray);
+            } else {
+              this.props.setSelectedPlugins(
+                this.state.availablePluginsToExport,
+              );
             }
-            return acc;
-          }, new Set([]) as Set<string>)}
+          }}
+          elements={this.state.availablePluginsToExport}
+          selectedElements={new Set(this.props.plugins.selectedPlugins)}
           onHide={onHideWithUnsettingShare}
-          showNavButtons={true}
         />
       </Container>
     );
@@ -145,7 +156,7 @@ export default connect<StateFromProps, DispatchFromProps, OwnProps, Store>(
     };
   },
   (dispatch: Dispatch<Action<any>>) => ({
-    selectedPlugins: (plugins: Array<string>) => {
+    setSelectedPlugins: (plugins: Array<string>) => {
       dispatch(actionForSelectedPlugins(plugins));
     },
     setActiveSheet: (payload: ActiveSheet) => {
diff --git a/src/chrome/ListView.tsx b/src/chrome/ListView.tsx
index d4c4eb671..b3ad0a7b6 100644
--- a/src/chrome/ListView.tsx
+++ b/src/chrome/ListView.tsx
@@ -33,11 +33,11 @@ type SubType =
     };
 
 type Props = {
-  onSelect: (elements: Array<string>) => void;
+  onSubmit?: () => void;
+  onChange: (elements: Array<string>) => void;
   onHide: () => any;
   elements: Array<string>;
   title?: string;
-  showNavButtons: boolean;
 } & SubType;
 
 const Title = styled(Text)({
@@ -113,16 +113,12 @@ class RowComponent extends Component<RowComponentProps> {
 
 export default class ListView extends Component<Props, State> {
   state: State = {selectedElements: new Set([])};
-  static getDerivedStateFromProps(props: Props, state: State) {
-    if (state.selectedElements.size > 0) {
-      return null;
-    }
+  static getDerivedStateFromProps(props: Props, _state: State) {
     if (props.type === 'multiple') {
       return {selectedElements: props.selectedElements};
     } else if (props.type === 'single') {
       return {selectedElements: new Set([props.selectedElement])};
     }
-
     return null;
   }
 
@@ -138,21 +134,17 @@ export default class ListView extends Component<Props, State> {
     } else {
       if (selected) {
         selectedElements = new Set([...this.state.selectedElements, id]);
-        this.setState({
-          selectedElements: selectedElements,
-        });
+        this.props.onChange([...selectedElements]);
       } else {
         selectedElements = new Set([...this.state.selectedElements]);
         selectedElements.delete(id);
-        this.setState({selectedElements});
+        this.props.onChange([...selectedElements]);
       }
     }
-    if (!this.props.showNavButtons) {
-      this.props.onSelect([...selectedElements]);
-    }
   };
 
   render() {
+    const {onSubmit} = this.props;
     return (
       <Container>
         <FlexColumn>
@@ -170,20 +162,14 @@ export default class ListView extends Component<Props, State> {
             })}
           </RowComponentContainer>
         </FlexColumn>
-        {this.props.showNavButtons && (
+        {onSubmit && (
           <Padder paddingTop={8} paddingBottom={2}>
             <FlexRow>
               <Spacer />
               <Button compact padded onClick={this.props.onHide}>
                 Close
               </Button>
-              <Button
-                compact
-                padded
-                type="primary"
-                onClick={() => {
-                  this.props.onSelect([...this.state.selectedElements]);
-                }}>
+              <Button compact padded type="primary" onClick={onSubmit}>
                 Submit
               </Button>
             </FlexRow>