From f6e46ee2c5e558cbcfd6cc12cc60aa56f54151b0 Mon Sep 17 00:00:00 2001 From: Abhinav Jha Date: Mon, 22 May 2023 13:29:29 +0530 Subject: [PATCH] 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 --- .../editorComponents/ResizableComponent.tsx | 31 ++++++++++---- .../src/resizable/autolayoutresize/index.tsx | 41 ++++++------------- app/client/src/resizable/common.tsx | 4 +- .../src/resizable/resizenreflow/index.tsx | 41 +++++++------------ 4 files changed, 52 insertions(+), 65 deletions(-) diff --git a/app/client/src/components/editorComponents/ResizableComponent.tsx b/app/client/src/components/editorComponents/ResizableComponent.tsx index bd5fba05ca..59168e7af9 100644 --- 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 ( void; onStop: ( diff --git a/app/client/src/resizable/resizenreflow/index.tsx b/app/client/src/resizable/resizenreflow/index.tsx index 642dc9fdac..ae68c1db7c 100644 --- 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 (