Fix: Tabs Widget (#1532) (#1535)

* Fix issue where placing widgets into the first tab of a tabs widget was disallowed. * Concatenate using spread instead of produce * Remove widget blueprint from widget props * Deal with nested blueprint Co-authored-by: Abhinav Jha <[email protected]>
appsmithorg · Nov 3, 2020 · 25a6fe2 · 25a6fe2
1 parent 1282708
commit 25a6fe2
Show file tree

Hide file tree

Showing 5 changed files with 98 additions and 45 deletions.
diff --git a/app/client/src/components/designSystems/appsmith/TabsComponent.tsx b/app/client/src/components/designSystems/appsmith/TabsComponent.tsx
@@ -8,12 +8,13 @@ import { scrollbarLight } from "constants/DefaultTheme";
 interface TabsComponentProps extends ComponentProps {
   children?: ReactNode;
   shouldScrollContents?: boolean;
-  selectedTabId: string;
+  selectedTabWidgetId: string;
   shouldShowTabs: boolean;
   onTabChange: (tabId: string) => void;
   tabs: Array<{
     id: string;
     label: string;
+    widgetId: string;
   }>;
 }
 
@@ -120,10 +121,10 @@ const TabsComponent = (props: TabsComponentProps) => {
             props.tabs.map((tab, index) => (
               <StyledText
                 onClick={(event: React.MouseEvent<HTMLDivElement>) => {
-                  props.onTabChange(tab.id);
+                  props.onTabChange(tab.widgetId);
                   event.stopPropagation();
                 }}
-                selected={props.selectedTabId === tab.id}
+                selected={props.selectedTabWidgetId === tab.widgetId}
                 key={index}
               >
                 {tab.label}

diff --git a/app/client/src/components/propertyControls/TabControl.tsx b/app/client/src/components/propertyControls/TabControl.tsx
@@ -157,7 +157,7 @@ class TabControl extends BaseControl<ControlProps> {
   };
 
   addOption = () => {
-    const tabs: Array<{
+    let tabs: Array<{
       id: string;
       label: string;
       widgetId: string;
@@ -169,11 +169,11 @@ class TabControl extends BaseControl<ControlProps> {
       "Tab ",
       tabs.map(tab => tab.label),
     );
-    tabs.push({
-      id: newTabId,
-      label: newTabLabel,
-      widgetId: generateReactKey(),
-    });
+    tabs = [
+      ...tabs,
+      { id: newTabId, label: newTabLabel, widgetId: generateReactKey() },
+    ];
+
     this.updateProperty(this.props.propertyName, JSON.stringify(tabs));
   };
 

diff --git a/app/client/src/mockResponses/WidgetConfigResponse.tsx b/app/client/src/mockResponses/WidgetConfigResponse.tsx
@@ -197,7 +197,7 @@ const WidgetConfigResponse: WidgetConfigReducerState = {
           {
             type: "MODIFY_PROPS",
             fn: (widget: WidgetProps & { children?: WidgetProps[] }) => {
-              const tabs = widget.tabs;
+              const tabs = [...widget.tabs];
 
               const newTabs = tabs.map((tab: any) => {
                 tab.widgetId = generateReactKey();

diff --git a/app/client/src/sagas/WidgetOperationSagas.tsx b/app/client/src/sagas/WidgetOperationSagas.tsx
@@ -80,6 +80,7 @@ import { forceOpenPropertyPane } from "actions/widgetActions";
 import { getDataTree } from "selectors/dataTreeSelectors";
 import { DataTreeWidget } from "entities/DataTree/dataTreeFactory";
 import { validateProperty } from "./evaluationsSaga";
+import { WidgetBlueprint } from "reducers/entityReducers/widgetConfigReducer";
 
 function getChildWidgetProps(
   parent: FlattenedWidgetProps,
@@ -89,10 +90,12 @@ function getChildWidgetProps(
   const { leftColumn, topRow, newWidgetId, props, type } = params;
   let { rows, columns, parentColumnSpace, parentRowSpace, widgetName } = params;
   let minHeight = undefined;
-  const defaultConfig: any = WidgetConfigResponse.config[type];
+  const { blueprint = undefined, ...restDefaultConfig } = {
+    ...(WidgetConfigResponse as any).config[type],
+  };
   if (!widgetName) {
     const widgetNames = Object.keys(widgets).map(w => widgets[w].widgetName);
-    widgetName = getNextEntityName(defaultConfig.widgetName, widgetNames);
+    widgetName = getNextEntityName(restDefaultConfig.widgetName, widgetNames);
   }
   if (type === WidgetTypes.CANVAS_WIDGET) {
     columns =
@@ -104,7 +107,13 @@ function getChildWidgetProps(
     if (props) props.children = [];
   }
 
-  const widgetProps = { ...defaultConfig, ...props, columns, rows, minHeight };
+  const widgetProps = {
+    ...restDefaultConfig,
+    ...props,
+    columns,
+    rows,
+    minHeight,
+  };
   const widget = generateWidgetProps(
     parent,
     type,
@@ -127,41 +136,80 @@ function* generateChildWidgets(
   parent: FlattenedWidgetProps,
   params: WidgetAddChild,
   widgets: { [widgetId: string]: FlattenedWidgetProps },
+  propsBlueprint?: WidgetBlueprint,
 ): any {
+  // Get the props for the widget
   const widget = yield getChildWidgetProps(parent, params, widgets);
+
+  // Add the widget to the canvasWidgets
+  // We need this in here as widgets will be used to get the current widget
   widgets[widget.widgetId] = widget;
-  if (widget.blueprint && widget.blueprint.view) {
+
+  // Get the default config for the widget from WidgetConfigResponse
+  const defaultConfig = {
+    ...(WidgetConfigResponse as any).config[widget.type],
+  };
+
+  // If blueprint is provided in the params, use that
+  // else use the blueprint available in WidgetConfigResponse
+  // else there is no blueprint for this widget
+  const blueprint =
+    propsBlueprint || { ...defaultConfig.blueprint } || undefined;
+
+  // If there is a blueprint.view
+  // We need to generate the children based on the view
+  if (blueprint && blueprint.view) {
+    // Get the list of children props in WidgetAddChild format
     const childWidgetList: WidgetAddChild[] = yield call(
       buildWidgetBlueprint,
-      widget.blueprint,
+      blueprint,
       widget.widgetId,
     );
+    // For each child props
     const childPropsList: GeneratedWidgetPayload[] = yield all(
       childWidgetList.map((props: WidgetAddChild) => {
-        return generateChildWidgets(widget, props, widgets);
+        // Generate full widget props
+        // Notice that we're passing the blueprint if it exists.
+        return generateChildWidgets(
+          widget,
+          props,
+          widgets,
+          props.props?.blueprint,
+        );
       }),
     );
+    // Start children array from scratch
     widget.children = [];
     childPropsList.forEach((props: GeneratedWidgetPayload) => {
+      // Push the widgetIds of the children generated above into the widget.children array
       widget.children.push(props.widgetId);
+      // Add the list of widgets generated into the canvasWidgets
       widgets = props.widgets;
     });
   }
 
+  // Finally, add the widget to the canvasWidgets
+  // This is different from above, as this is the final widget props with
+  // a fully populated widget.children property
   widgets[widget.widgetId] = widget;
-  if (
-    widget.blueprint &&
-    widget.blueprint.operations &&
-    widget.blueprint.operations.length > 0
-  ) {
+
+  // Some widgets need to run a few operations like modifying props or adding an action
+  // these operations can be performed on the parent of the widget we're adding
+  // therefore, we pass all widgets to executeWidgetBlueprintOperations
+  // blueprint.operations contain the set of operations to perform to update the canvasWidgets
+  if (blueprint && blueprint.operations && blueprint.operations.length > 0) {
+    // Finalize the canvasWidgets with everything that needs to be updated
     widgets = yield call(
       executeWidgetBlueprintOperations,
-      widget.blueprint.operations,
+      blueprint.operations,
       widgets,
       widget.widgetId,
     );
   }
+  // Add the parentId prop to this widget
   widget.parentId = parent.widgetId;
+  // Remove the blueprint from the widget (if any)
+  // as blueprints are not useful beyont this point.
   delete widget.blueprint;
   return { widgetId: widget.widgetId, widgets };
 }
@@ -847,17 +895,11 @@ function* pasteWidgetSaga() {
       } else {
         // If the widget in which to paste the new widget is a tabs widget
         // Find the currently selected tab canvas widget
-        const { selectedTabId } = yield select(
+        const { selectedTabWidgetId } = yield select(
           getWidgetMetaProps,
           parentWidget.widgetId,
         );
-        const tabs = _.isString(parentWidget.tabs)
-          ? JSON.parse(parentWidget.tabs)
-          : parentWidget.tabs;
-        const childWidgetId =
-          tabs.find((tab: any) => tab.id === selectedTabId)?.widgetId ||
-          parentWidget.children[0];
-        childWidget = widgets[childWidgetId];
+        if (selectedTabWidgetId) childWidget = widgets[selectedTabWidgetId];
       }
       // If the finally selected parent in which to paste the widget
       // is a CANVAS_WIDGET, use its widgetId as the new widget's parent Id

diff --git a/app/client/src/widgets/TabsWidget.tsx b/app/client/src/widgets/TabsWidget.tsx
@@ -23,8 +23,8 @@ class TabsWidget extends BaseWidget<
     };
   }
 
-  onTabChange = (tabId: string) => {
-    this.props.updateWidgetMetaProperty("selectedTabId", tabId, {
+  onTabChange = (tabWidgetId: string) => {
+    this.props.updateWidgetMetaProperty("selectedTabWidgetId", tabWidgetId, {
       dynamicString: this.props.onTabSelected,
       event: {
         type: EventType.ON_TAB_CHANGE,
@@ -34,7 +34,7 @@ class TabsWidget extends BaseWidget<
 
   static getDerivedPropertiesMap() {
     return {
-      selectedTab: `{{_.find(this.tabs, { id: this.selectedTabId }).label}}`,
+      selectedTab: `{{_.find(this.tabs, { widgetId: this.selectedTabWidgetId }).label}}`,
     };
   }
 
@@ -59,12 +59,11 @@ class TabsWidget extends BaseWidget<
   }
 
   renderComponent = () => {
-    const selectedTabId = this.props.selectedTabId;
-
+    const selectedTabWidgetId = this.props.selectedTabWidgetId;
     const childWidgetData: TabContainerWidgetProps = this.props.children
       ?.filter(Boolean)
       .filter(item => {
-        return selectedTabId === item.tabId;
+        return selectedTabWidgetId === item.widgetId;
       })[0];
     if (!childWidgetData) {
       return null;
@@ -171,8 +170,13 @@ class TabsWidget extends BaseWidget<
         const selectedTab = _.find(this.props.tabs, {
           label: this.props.defaultTab,
         });
-        const selectedTabId = selectedTab ? selectedTab.id : undefined;
-        this.props.updateWidgetMetaProperty("selectedTabId", selectedTabId);
+        const selectedTabWidgetId = selectedTab
+          ? selectedTab.widgetId
+          : undefined;
+        this.props.updateWidgetMetaProperty(
+          "selectedTabWidgetId",
+          selectedTabWidgetId,
+        );
       }
     }
   }
@@ -221,18 +225,24 @@ class TabsWidget extends BaseWidget<
         label: this.props.defaultTab,
       });
       // Find the default Tab id
-      const selectedTabId = selectedTab?.id;
+      const selectedTabWidgetId = selectedTab?.widgetId;
       // If we have a legitimate default tab Id and it is not already the selected Tab
-      if (selectedTabId && selectedTabId !== this.props.selectedTabId) {
+      if (
+        selectedTabWidgetId &&
+        selectedTabWidgetId !== this.props.selectedTabWidgetId
+      ) {
         // Select the default tab
-        this.props.updateWidgetMetaProperty("selectedTabId", selectedTabId);
+        this.props.updateWidgetMetaProperty(
+          "selectedTabWidgetId",
+          selectedTabWidgetId,
+        );
       }
-    } else if (!this.props.selectedTabId) {
+    } else if (!this.props.selectedTabWidgetId) {
       // If no tab is selected
       // Select the first tab in the tabs list.
       this.props.updateWidgetMetaProperty(
-        "selectedTabId",
-        this.props.tabs[0].id,
+        "selectedTabWidgetId",
+        this.props.tabs[0].widgetId,
       );
     }
     this.generateTabContainers();
@@ -259,7 +269,7 @@ export interface TabsWidgetProps<T extends TabContainerWidgetProps>
   onTabSelected?: string;
   snapRows?: number;
   defaultTab: string;
-  selectedTabId: string;
+  selectedTabWidgetId: string;
 }
 
 export default TabsWidget;