fix: resizable max height (#23143)

## Description #### Reported issue In a container widget, when we switch to auto height with limits (in the property pane), then switch back to FIXED (in the property pane), we are no longer able to vertically resize the container widget to be larger. #### Results of RCA This was happening because, we set a `maxDynamicHeight` when we switch to `auto height with limits`. When we switched back to `fixed`, the same limits were being respected by the resizing library. This resulted in the height being capped at what was set in the `maxDynamicHeight`. #### Code design issue: There was also an issue where the resizing library was "aware" of the "dynamic height". Dynamic height is a platform construct and the resizing library being aware of this construct is a breach in the interface. #### Solution: We now pass the `maxHeightInPx` and `autoHeight` to the resizing library. `maxHeightInPx` is the maximum height -- the resizing library can -- allow in vertical resize. `autoHeight` ignores the resizing library's ability to set the height (in this scenario we handle the height using the dynamic height feature in the platform) This allows us to keep the resizing library agnostic of the platform dynamic height feature and lets the platform code decide the max height possible. #### PR fixes following issue(s) Fixes #22439 #### Media #### Type of change - Bug fix (non-breaking change which fixes an issue) ## Testing #### How Has This Been Tested? - [x] Manual - [ ] Jest - [ ] Cypress - Limitations in the testing infra makes it difficult to add a test for this #### Test Plan #### Issues raised during DP testing ## Checklist: #### Dev activity - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Test-plan-implementation#speedbreaker-features-to-consider-for-every-change) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans/_edit#areas-of-interest) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [ ] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed
2023-05-22 13:29:29 +05:30 · 2023-05-22 13:29:29 +05:30 · f6e46ee2c5
commit f6e46ee2c5
parent 5bc439d478
4 changed files with 52 additions and 65 deletions
--- a/app/client/src/components/editorComponents/ResizableComponent.tsx
+++ b/app/client/src/components/editorComponents/ResizableComponent.tsx
@ -3,7 +3,11 @@ import { batchUpdateMultipleWidgetProperties } from "actions/controlActions";
 import { focusWidget } from "actions/widgetActions";
 import { EditorContext } from "components/editorComponents/EditorContextProvider";
 import type { OccupiedSpace } from "constants/CanvasEditorConstants";
-import { DefaultDimensionMap, GridDefaults } from "constants/WidgetConstants";
+import {
+  DefaultDimensionMap,
+  GridDefaults,
+  WidgetHeightLimits,
+} from "constants/WidgetConstants";
 import { get, omit } from "lodash";
 import type { XYCord } from "pages/common/CanvasArenas/hooks/useRenderBlocksOnCanvas";
 import React, { memo, useContext, useMemo } from "react";
@ -71,7 +75,6 @@ export const ResizableComponent = memo(function ResizableComponent(
  const { updateWidget } = useContext(EditorContext);
  const dispatch = useDispatch();
  const isAutoLayout = useSelector(getIsAutoLayout);
-
  const Resizable = isAutoLayout ? AutoLayoutResizable : FixedLayoutResizable;
  const isSnipingMode = useSelector(snipingModeSelector);
  const isPreviewMode = useSelector(previewModeSelector);
@ -355,10 +358,21 @@ export const ResizableComponent = memo(function ResizableComponent(
    return !isAutoHeightEnabledForWidget(props) && isEnabled;
  }, [props, isAutoHeightEnabledForWidget, isEnabled]);

-  const fixedHeight =
-    isAutoHeightEnabledForWidgetWithLimits(props) ||
-    !isAutoHeightEnabledForWidget(props) ||
-    !props.isCanvas;
+  // What is the max resizable height for this widget, in pixels?
+  let maxHeightInPx =
+    WidgetHeightLimits.MAX_HEIGHT_IN_ROWS *
+    GridDefaults.DEFAULT_GRID_ROW_HEIGHT; // Maximum possible height
+  // If the widget has auto height with limits, we need to respect the set limits.
+  if (isAutoHeightEnabledForWidgetWithLimits(props)) {
+    maxHeightInPx =
+      (props.maxDynamicHeight || WidgetHeightLimits.MAX_HEIGHT_IN_ROWS) *
+      GridDefaults.DEFAULT_GRID_ROW_HEIGHT;
+  }
+
+  // Is auto height enabled for widget (without limits)
+  const autoHeight =
+    isAutoHeightEnabledForWidget(props) &&
+    !isAutoHeightEnabledForWidgetWithLimits(props);

  const allowResize: boolean =
    !isMultiSelected || (isAutoLayout && !props.isFlexChild);
@ -370,22 +384,23 @@ export const ResizableComponent = memo(function ResizableComponent(
    !isAppSettingsPaneWithNavigationTabOpen &&
    !isDragging &&
    (isHovered || isSelected);
+
  return (
    <Resizable
      allowResize={allowResize}
+      autoHeight={autoHeight}
      componentHeight={dimensions.height}
      componentWidth={dimensions.width}
      direction={props.direction}
      enableHorizontalResize={isEnabled}
      enableVerticalResize={isVerticalResizeEnabled}
-      fixedHeight={fixedHeight}
      getResizedPositions={getResizedPositions}
      gridProps={gridProps}
      handles={handles}
      isFlexChild={props.isFlexChild}
      isHovered={isHovered}
      isMobile={props.isMobile || false}
-      maxDynamicHeight={props.maxDynamicHeight}
+      maxHeightInPx={maxHeightInPx}
      onStart={handleResizeStart}
      onStop={updateSize}
      originalPositions={originalPositions}
--- a/app/client/src/resizable/autolayoutresize/index.tsx
+++ b/app/client/src/resizable/autolayoutresize/index.tsx
@ -4,11 +4,7 @@ import {
  isResizingDisabled,
 } from "components/editorComponents/ResizableUtils";
 import type { OccupiedSpace } from "constants/CanvasEditorConstants";
-import {
-  GridDefaults,
-  WIDGET_PADDING,
-  WidgetHeightLimits,
-} from "constants/WidgetConstants";
+import { GridDefaults, WIDGET_PADDING } from "constants/WidgetConstants";
 import React, { useEffect, useMemo, useRef, useState } from "react";
 import type { CSSProperties } from "react";
 import { useDispatch, useSelector } from "react-redux";
@ -685,33 +681,22 @@ function AutoLayoutResizable(props: ResizableProps) {
      }}
      from={{
        width: props.componentWidth,
-        height: props.fixedHeight
-          ? Math.min(
-              (props.maxDynamicHeight ||
-                WidgetHeightLimits.MAX_HEIGHT_IN_ROWS) *
-                GridDefaults.DEFAULT_GRID_ROW_HEIGHT,
-              props.componentHeight,
-            )
-          : "auto",
-        maxHeight:
-          (props.maxDynamicHeight || WidgetHeightLimits.MAX_HEIGHT_IN_ROWS) *
-          GridDefaults.DEFAULT_GRID_ROW_HEIGHT,
+        height: props.autoHeight
+          ? "auto"
+          : Math.min(props.maxHeightInPx, props.componentHeight),
+        maxHeight: props.maxHeightInPx,
      }}
      immediate={newDimensions.reset ? true : false}
      to={{
        width: widgetWidth,
-        height: props.fixedHeight
-          ? Math.min(
-              (props.maxDynamicHeight ||
-                WidgetHeightLimits.MAX_HEIGHT_IN_ROWS) *
-                GridDefaults.DEFAULT_GRID_ROW_HEIGHT,
-              widgetHeight,
-            )
-          : "auto",
-
-        maxHeight:
-          (props.maxDynamicHeight || WidgetHeightLimits.MAX_HEIGHT_IN_ROWS) *
-          GridDefaults.DEFAULT_GRID_ROW_HEIGHT,
+        // If height is automatically set, use `auto`, widgetHeight is not considered
+        // other wise, limit the height based on the max.
+        // We could also use the isVerticalDisabled flag here, but that would mean that
+        // the auto height with limits will stop working correctly
+        height: props.autoHeight
+          ? "auto"
+          : Math.min(props.maxHeightInPx, widgetHeight),
+        maxHeight: props.maxHeightInPx,
        transform: `translate3d(${
          newDimensions.reflectPosition ? newDimensions.x : 0
        }px,${newDimensions.reflectPosition ? newDimensions.y : 0}px,0)`,
--- a/app/client/src/resizable/common.tsx
+++ b/app/client/src/resizable/common.tsx
@ -239,8 +239,8 @@ export type ResizableProps = {
    canResizeHorizontally: boolean;
    canResizeVertically: boolean;
  };
-  fixedHeight: boolean;
-  maxDynamicHeight?: number;
+  maxHeightInPx: number; // Maximum height in pixels, the child can have.
+  autoHeight: boolean; // true if we don't want a pixel height specified for the child
  originalPositions: OccupiedSpace;
  onStart: (affectsWidth?: boolean) => void;
  onStop: (
--- a/app/client/src/resizable/resizenreflow/index.tsx
+++ b/app/client/src/resizable/resizenreflow/index.tsx
@ -1,10 +1,6 @@
 import { computeRowCols } from "components/editorComponents/ResizableUtils";
 import { isHandleResizeAllowed } from "components/editorComponents/ResizableUtils";
-import {
-  GridDefaults,
-  WIDGET_PADDING,
-  WidgetHeightLimits,
-} from "constants/WidgetConstants";
+import { WIDGET_PADDING } from "constants/WidgetConstants";
 import { Spring } from "react-spring";
 import React, { useEffect, useMemo, useRef, useState } from "react";
 import type { CSSProperties } from "react";
@ -400,6 +396,7 @@ export function ReflowResizable(props: ResizableProps) {
      props.showResizeBoundary ? "show-boundary" : ""
    } ${pointerEvents ? "" : "pointer-event-none"}`;
  }, [props.className, pointerEvents, props.showResizeBoundary]);
+
  return (
    <Spring
      config={{
@ -409,33 +406,23 @@ export function ReflowResizable(props: ResizableProps) {
      }}
      from={{
        width: props.componentWidth,
-        height: props.fixedHeight
-          ? Math.min(
-              (props.maxDynamicHeight ||
-                WidgetHeightLimits.MAX_HEIGHT_IN_ROWS) *
-                GridDefaults.DEFAULT_GRID_ROW_HEIGHT,
-              props.componentHeight,
-            )
-          : "auto",
-        maxHeight:
-          (props.maxDynamicHeight || WidgetHeightLimits.MAX_HEIGHT_IN_ROWS) *
-          GridDefaults.DEFAULT_GRID_ROW_HEIGHT,
+        height: props.autoHeight
+          ? "auto"
+          : Math.min(props.maxHeightInPx, props.componentHeight),
+        maxHeight: props.maxHeightInPx,
      }}
      immediate={newDimensions.reset ? true : false}
      to={{
        width: widgetWidth,
-        height: props.fixedHeight
-          ? Math.min(
-              (props.maxDynamicHeight ||
-                WidgetHeightLimits.MAX_HEIGHT_IN_ROWS) *
-                GridDefaults.DEFAULT_GRID_ROW_HEIGHT,
-              widgetHeight,
-            )
-          : "auto",
+        // If height is automatically set, use `auto`, widgetHeight is not considered
+        // other wise, limit the height based on the max.
+        // We could also use the isVerticalDisabled flag here, but that would mean that
+        // the auto height with limits will stop working correctly
+        height: props.autoHeight
+          ? "auto"
+          : Math.min(props.maxHeightInPx, widgetHeight),

-        maxHeight:
-          (props.maxDynamicHeight || WidgetHeightLimits.MAX_HEIGHT_IN_ROWS) *
-          GridDefaults.DEFAULT_GRID_ROW_HEIGHT,
+        maxHeight: props.maxHeightInPx,
        transform: `translate3d(${
          (newDimensions.reflectPosition ? newDimensions.x : 0) -
          RESIZE_BORDER_BUFFER / 2