From 8c2825e862e501bf7b1fa0740e9ba272d0328234 Mon Sep 17 00:00:00 2001 From: Abhinav Jha Date: Wed, 8 Feb 2023 13:31:26 +0530 Subject: [PATCH] fix: Modal widget background issues (#20446) ## Description The following issues were caused because of the recent changes with respect to auto height instant update. In the recent change, we removed a few wrappers around containers which seemed unnecessary. This led to the fact that in deploy mode, these wrappers were not present. The issue with this was that, these wrappers were responsible for the modal widget's background color styling. This also led to an issue where the background color was not applied in edit mode. To fix this, we've added a wrapper in the component and removed all styling logic from the widget. This is because, the component is responsible for the actual rendering and what users see. The widgets should act as an interface, and as a result, should not deal with styling. Fixes #20434 Fixes #20405 Fixes #20436 ## Type of change - Bug fix (non-breaking change which fixes an issue) --- .../fixtures/modalWidgetBGcolorDSL.json | 327 ++++++++++++++++++ .../Widgets/Modal_background_spec.ts | 27 ++ .../Widgets/Others/Canvas_scrolling_spec.js | 7 +- .../cypress/support/Objects/CommonLocators.ts | 1 + .../ColorPickerComponentV2.tsx | 6 +- .../pages/AppViewer/BrandingBadgeMobile.tsx | 2 +- app/client/src/pages/AppViewer/index.tsx | 2 +- app/client/src/utils/WidgetSizeUtils.test.ts | 34 ++ app/client/src/utils/WidgetSizeUtils.ts | 1 + .../utils/hooks/useClickToSelectWidget.tsx | 43 +-- .../widgets/ModalWidget/component/index.tsx | 49 ++- .../src/widgets/ModalWidget/widget/index.tsx | 26 +- 12 files changed, 459 insertions(+), 66 deletions(-) create mode 100644 app/client/cypress/fixtures/modalWidgetBGcolorDSL.json create mode 100644 app/client/cypress/integration/Regression_TestSuite/ClientSideTests/Widgets/Modal_background_spec.ts diff --git a/app/client/cypress/fixtures/modalWidgetBGcolorDSL.json b/app/client/cypress/fixtures/modalWidgetBGcolorDSL.json new file mode 100644 index 0000000000..2698866aed --- /dev/null +++ b/app/client/cypress/fixtures/modalWidgetBGcolorDSL.json @@ -0,0 +1,327 @@ +{"dsl": { + "widgetName": "MainContainer", + "backgroundColor": "none", + "rightColumn": 4896, + "snapColumns": 64, + "detachFromLayout": true, + "widgetId": "0", + "topRow": 0, + "bottomRow": 2400, + "containerStyle": "none", + "snapRows": 124, + "parentRowSpace": 1, + "type": "CANVAS_WIDGET", + "canExtend": true, + "version": 76, + "minHeight": 1292, + "dynamicTriggerPathList": [], + "parentColumnSpace": 1, + "dynamicBindingPathList": [], + "leftColumn": 0, + "children": [ + { + "resetFormOnClick": false, + "boxShadow": "none", + "widgetName": "Button1", + "onClick": "{{showModal('Modal1')}}", + "buttonColor": "{{appsmith.theme.colors.primaryColor}}", + "displayName": "Button", + "iconSVG": "/static/media/icon.cca026338f1c8eb6df8ba03d084c2fca.svg", + "searchTags": [ + "click", + "submit" + ], + "topRow": 23, + "bottomRow": 27, + "parentRowSpace": 10, + "type": "BUTTON_WIDGET", + "hideCard": false, + "animateLoading": true, + "parentColumnSpace": 26.765625, + "dynamicTriggerPathList": [ + { + "key": "onClick" + } + ], + "leftColumn": 17, + "dynamicBindingPathList": [ + { + "key": "buttonColor" + }, + { + "key": "borderRadius" + } + ], + "text": "Submit", + "isDisabled": false, + "key": "bg0viduptw", + "isDeprecated": false, + "rightColumn": 33, + "isDefaultClickDisabled": true, + "widgetId": "40a5ldch2f", + "isVisible": true, + "recaptchaType": "V3", + "version": 1, + "parentId": "0", + "renderMode": "CANVAS", + "isLoading": false, + "disabledWhenInvalid": false, + "borderRadius": "{{appsmith.theme.borderRadius.appBorderRadius}}", + "buttonVariant": "PRIMARY", + "placement": "CENTER" + }, + { + "boxShadow": "none", + "widgetName": "Modal1", + "isCanvas": true, + "displayName": "Modal", + "iconSVG": "/static/media/icon.4975978e9a961fb0bfb4e38de7ecc7c5.svg", + "searchTags": [ + "dialog", + "popup", + "notification" + ], + "topRow": 0, + "bottomRow": 240, + "parentRowSpace": 1, + "type": "MODAL_WIDGET", + "hideCard": false, + "shouldScrollContents": true, + "animateLoading": true, + "parentColumnSpace": 1, + "dynamicTriggerPathList": [], + "leftColumn": 0, + "dynamicBindingPathList": [ + { + "key": "borderRadius" + } + ], + "children": [ + { + "widgetName": "Canvas1", + "displayName": "Canvas", + "topRow": 0, + "bottomRow": 240, + "parentRowSpace": 1, + "type": "CANVAS_WIDGET", + "canExtend": true, + "hideCard": true, + "shouldScrollContents": false, + "minHeight": 0, + "parentColumnSpace": 1, + "leftColumn": 0, + "dynamicBindingPathList": [], + "children": [ + { + "boxShadow": "none", + "widgetName": "IconButton1", + "onClick": "{{closeModal('Modal1')}}", + "buttonColor": "{{appsmith.theme.colors.primaryColor}}", + "displayName": "Icon Button", + "iconSVG": "/static/media/icon.1a0c634ac75f9fa6b6ae7a8df882a3ba.svg", + "searchTags": [ + "click", + "submit" + ], + "topRow": 0, + "bottomRow": 4, + "type": "ICON_BUTTON_WIDGET", + "hideCard": false, + "animateLoading": true, + "leftColumn": 58, + "dynamicBindingPathList": [ + { + "key": "buttonColor" + }, + { + "key": "borderRadius" + } + ], + "iconSize": 24, + "isDisabled": false, + "key": "j7aja1t2g5", + "isDeprecated": false, + "rightColumn": 64, + "iconName": "cross", + "widgetId": "pjzeprhz9n", + "isVisible": true, + "version": 1, + "parentId": "v49krwgo9n", + "renderMode": "CANVAS", + "isLoading": false, + "borderRadius": "{{appsmith.theme.borderRadius.appBorderRadius}}", + "buttonVariant": "TERTIARY" + }, + { + "widgetName": "Text1", + "displayName": "Text", + "iconSVG": "/static/media/icon.97c59b523e6f70ba6f40a10fc2c7c5b5.svg", + "searchTags": [ + "typography", + "paragraph", + "label" + ], + "topRow": 1, + "bottomRow": 5, + "type": "TEXT_WIDGET", + "hideCard": false, + "animateLoading": true, + "overflow": "NONE", + "fontFamily": "{{appsmith.theme.fontFamily.appFont}}", + "leftColumn": 1, + "dynamicBindingPathList": [ + { + "key": "truncateButtonColor" + }, + { + "key": "fontFamily" + }, + { + "key": "borderRadius" + } + ], + "shouldTruncate": false, + "truncateButtonColor": "{{appsmith.theme.colors.primaryColor}}", + "text": "Modal Title", + "key": "b5ov7jc32w", + "isDeprecated": false, + "rightColumn": 41, + "textAlign": "LEFT", + "dynamicHeight": "AUTO_HEIGHT", + "widgetId": "emh0ct7lss", + "isVisible": true, + "fontStyle": "BOLD", + "textColor": "#231F20", + "version": 1, + "parentId": "v49krwgo9n", + "renderMode": "CANVAS", + "isLoading": false, + "borderRadius": "{{appsmith.theme.borderRadius.appBorderRadius}}", + "maxDynamicHeight": 9000, + "fontSize": "1.25rem", + "minDynamicHeight": 4 + }, + { + "resetFormOnClick": false, + "boxShadow": "none", + "widgetName": "Button2", + "onClick": "{{closeModal('Modal1')}}", + "buttonColor": "{{appsmith.theme.colors.primaryColor}}", + "displayName": "Button", + "iconSVG": "/static/media/icon.cca026338f1c8eb6df8ba03d084c2fca.svg", + "searchTags": [ + "click", + "submit" + ], + "topRow": 18, + "bottomRow": 22, + "type": "BUTTON_WIDGET", + "hideCard": false, + "animateLoading": true, + "leftColumn": 31, + "dynamicBindingPathList": [ + { + "key": "buttonColor" + }, + { + "key": "borderRadius" + } + ], + "text": "Close", + "isDisabled": false, + "key": "bg0viduptw", + "isDeprecated": false, + "rightColumn": 47, + "isDefaultClickDisabled": true, + "widgetId": "uixlp0a1jo", + "buttonStyle": "PRIMARY", + "isVisible": true, + "recaptchaType": "V3", + "version": 1, + "parentId": "v49krwgo9n", + "renderMode": "CANVAS", + "isLoading": false, + "disabledWhenInvalid": false, + "borderRadius": "{{appsmith.theme.borderRadius.appBorderRadius}}", + "buttonVariant": "SECONDARY", + "placement": "CENTER" + }, + { + "resetFormOnClick": false, + "boxShadow": "none", + "widgetName": "Button3", + "buttonColor": "{{appsmith.theme.colors.primaryColor}}", + "displayName": "Button", + "iconSVG": "/static/media/icon.cca026338f1c8eb6df8ba03d084c2fca.svg", + "searchTags": [ + "click", + "submit" + ], + "topRow": 18, + "bottomRow": 22, + "type": "BUTTON_WIDGET", + "hideCard": false, + "animateLoading": true, + "leftColumn": 47, + "dynamicBindingPathList": [ + { + "key": "buttonColor" + }, + { + "key": "borderRadius" + } + ], + "text": "Confirm", + "isDisabled": false, + "key": "bg0viduptw", + "isDeprecated": false, + "rightColumn": 63, + "isDefaultClickDisabled": true, + "widgetId": "no3msqhcnx", + "buttonStyle": "PRIMARY_BUTTON", + "isVisible": true, + "recaptchaType": "V3", + "version": 1, + "parentId": "v49krwgo9n", + "renderMode": "CANVAS", + "isLoading": false, + "disabledWhenInvalid": false, + "borderRadius": "{{appsmith.theme.borderRadius.appBorderRadius}}", + "buttonVariant": "PRIMARY", + "placement": "CENTER" + } + ], + "isDisabled": false, + "key": "12l6uvszhz", + "isDeprecated": false, + "rightColumn": 0, + "detachFromLayout": true, + "widgetId": "v49krwgo9n", + "isVisible": true, + "version": 1, + "parentId": "edruh6epmy", + "renderMode": "CANVAS", + "isLoading": false + } + ], + "key": "m1lvx5wq7n", + "height": 240, + "isDeprecated": false, + "rightColumn": 0, + "backgroundColor": "#fde047", + "detachFromLayout": true, + "dynamicHeight": "AUTO_HEIGHT", + "widgetId": "edruh6epmy", + "canOutsideClickClose": true, + "canEscapeKeyClose": true, + "version": 2, + "parentId": "0", + "renderMode": "CANVAS", + "isLoading": false, + "borderRadius": "{{appsmith.theme.borderRadius.appBorderRadius}}", + "maxDynamicHeight": 9000, + "width": 456, + "minDynamicHeight": 24 + } + ] +}} \ No newline at end of file diff --git a/app/client/cypress/integration/Regression_TestSuite/ClientSideTests/Widgets/Modal_background_spec.ts b/app/client/cypress/integration/Regression_TestSuite/ClientSideTests/Widgets/Modal_background_spec.ts new file mode 100644 index 0000000000..d4570d069f --- /dev/null +++ b/app/client/cypress/integration/Regression_TestSuite/ClientSideTests/Widgets/Modal_background_spec.ts @@ -0,0 +1,27 @@ +import { ObjectsRegistry } from "../../../../support/Objects/Registry"; + +const { AggregateHelper, CommonLocators, DeployMode } = ObjectsRegistry; + +describe("Modal Widget background color spec", () => { + before(() => { + cy.fixture("modalWidgetBGcolorDSL").then((val: any) => { + AggregateHelper.AddDsl(val); + }); + }); + + it("1. Should have background color in edit mode and deploy mode", () => { + cy.get(CommonLocators._widgetInCanvas("buttonwidget")).click(); + cy.get(CommonLocators._modalWrapper).should( + "have.css", + "background-color", + "rgb(253, 224, 71)", + ); + DeployMode.DeployApp(); + cy.get(CommonLocators._widgetInDeployed("buttonwidget")).click(); + cy.get(CommonLocators._modalWrapper).should( + "have.css", + "background-color", + "rgb(253, 224, 71)", + ); + }); +}); diff --git a/app/client/cypress/integration/Regression_TestSuite/ClientSideTests/Widgets/Others/Canvas_scrolling_spec.js b/app/client/cypress/integration/Regression_TestSuite/ClientSideTests/Widgets/Others/Canvas_scrolling_spec.js index 3845d7a6a4..6b62ec55bc 100644 --- a/app/client/cypress/integration/Regression_TestSuite/ClientSideTests/Widgets/Others/Canvas_scrolling_spec.js +++ b/app/client/cypress/integration/Regression_TestSuite/ClientSideTests/Widgets/Others/Canvas_scrolling_spec.js @@ -16,13 +16,12 @@ describe("Modal Widget Functionality", function() { cy.get("span:contains('Close')") .closest("div") .should("not.be.visible"); - cy.get(".t--modal-widget") - .scrollTo("bottom") - .wait(1000); + cy.get(".t--modal-widget").then(($el) => $el[0].scrollTo(0, 500)); + cy.get("span:contains('Close')") .closest("div") .should("be.visible"); - cy.get(".t--modal-widget").scrollTo("top"); + cy.get(".t--modal-widget").then(($el) => $el[0].scrollTo(0, 0)); cy.get("span:contains('Close')") .closest("div") .should("not.be.visible"); diff --git a/app/client/cypress/support/Objects/CommonLocators.ts b/app/client/cypress/support/Objects/CommonLocators.ts index f4a2d736c2..425b31643d 100644 --- a/app/client/cypress/support/Objects/CommonLocators.ts +++ b/app/client/cypress/support/Objects/CommonLocators.ts @@ -172,4 +172,5 @@ export class CommonLocators { _editorVariable = ".cm-variable"; _consoleString = ".cm-string"; _commentString = ".cm-comment"; + _modalWrapper = "[data-cy='modal-wrapper']"; } diff --git a/app/client/src/components/propertyControls/ColorPickerComponentV2.tsx b/app/client/src/components/propertyControls/ColorPickerComponentV2.tsx index 30a1c243f2..3ff8abaa07 100644 --- a/app/client/src/components/propertyControls/ColorPickerComponentV2.tsx +++ b/app/client/src/components/propertyControls/ColorPickerComponentV2.tsx @@ -27,7 +27,6 @@ import { TAILWIND_COLORS } from "constants/ThemeConstants"; import useDSEvent from "utils/hooks/useDSEvent"; import { DSEventTypes } from "utils/AppsmithUtils"; import { getBrandColors } from "@appsmith/selectors/tenantSelectors"; -import tinycolor from "tinycolor2"; const FocusTrap = require("focus-trap-react"); const MAX_COLS = 10; @@ -552,10 +551,7 @@ const ColorPickerComponent = React.forwardRef( autoFocus={props.autoFocus} inputRef={inputGroupRef} leftIcon={ - + } onChange={handleChangeColor} onClick={handleInputClick} diff --git a/app/client/src/pages/AppViewer/BrandingBadgeMobile.tsx b/app/client/src/pages/AppViewer/BrandingBadgeMobile.tsx index a95303d8f1..88d2d031ef 100644 --- a/app/client/src/pages/AppViewer/BrandingBadgeMobile.tsx +++ b/app/client/src/pages/AppViewer/BrandingBadgeMobile.tsx @@ -5,7 +5,7 @@ import { ReactComponent as AppsmithLogo } from "assets/svg/appsmith-logo-no-pad. function BrandingBadge() { return ( {!hideWatermark && ( { expect(result).toBe(300); }); +it("Ignores the detached children of the canvas correctly", () => { + const canvasWidgets = { + x: { + ...DUMMY_WIDGET, + widgetId: "x", + bottomRow: 20, + topRow: 10, + type: "CANVAS_WIDGET", + children: ["m"], + }, + m: { + ...DUMMY_WIDGET, + widgetId: "m", + parentId: "x", + children: ["n", "o"], + type: "CANVAS_WIDGET", + }, + n: { + ...DUMMY_WIDGET, + detachFromLayout: true, + parentId: "m", + bottomRow: 30, + }, + o: { + ...DUMMY_WIDGET, + parentId: "m", + bottomRow: 5, + }, + }; + + const result = getCanvasBottomRow("m", canvasWidgets); + expect(result).toBe(100); +}); + it("Computes the bottomRow of the canvas within a Modal correctly", () => { const canvasWidgets = { x: { diff --git a/app/client/src/utils/WidgetSizeUtils.ts b/app/client/src/utils/WidgetSizeUtils.ts index e00d0e5ad6..a164c582ee 100644 --- a/app/client/src/utils/WidgetSizeUtils.ts +++ b/app/client/src/utils/WidgetSizeUtils.ts @@ -163,6 +163,7 @@ export function getCanvasBottomRow( if (Array.isArray(children) && children.length > 0) { const bottomRow = children.reduce((prev, next) => { + if (canvasWidgets[next].detachFromLayout) return prev; return canvasWidgets[next].bottomRow > prev ? canvasWidgets[next].bottomRow : prev; diff --git a/app/client/src/utils/hooks/useClickToSelectWidget.tsx b/app/client/src/utils/hooks/useClickToSelectWidget.tsx index 55f62bd746..027e623863 100644 --- a/app/client/src/utils/hooks/useClickToSelectWidget.tsx +++ b/app/client/src/utils/hooks/useClickToSelectWidget.tsx @@ -10,43 +10,20 @@ import { } from "selectors/widgetSelectors"; import styled from "styled-components"; import { stopEventPropagation } from "utils/AppsmithUtils"; -import { scrollCSS } from "widgets/WidgetUtils"; import { useWidgetSelection } from "./useWidgetSelection"; import { SelectionRequestType } from "sagas/WidgetSelectUtils"; -import { Colors } from "constants/Colors"; -const ContentWrapper = styled.div<{ - backgroundColor?: string; - borderRadius?: string; -}>` +const ContentWrapper = styled.div` width: 100%; height: 100%; - background: ${({ backgroundColor }) => `${backgroundColor || Colors.WHITE}`}; - border-radius: ${({ borderRadius }) => borderRadius}; - ${scrollCSS} -`; - -const ScrollWrapper = styled.div<{ - backgroundColor?: string; - borderRadius?: string; -}>` - width: 100%; - height: 100%; - background: ${({ backgroundColor }) => `${backgroundColor || Colors.WHITE}`}; - border-radius: ${({ borderRadius }) => borderRadius}; - overflow: hidden; `; export function ClickContentToOpenPropPane({ - backgroundColor, - borderRadius, children, widgetId, }: { widgetId: string; children?: ReactNode; - backgroundColor?: string; - borderRadius?: string; }) { const { focusWidget } = useWidgetSelection(); @@ -72,19 +49,13 @@ export function ClickContentToOpenPropPane({ }; return ( - - - {children} - - + {children} + ); } diff --git a/app/client/src/widgets/ModalWidget/component/index.tsx b/app/client/src/widgets/ModalWidget/component/index.tsx index a3dbd0727a..63fa4fe74b 100644 --- a/app/client/src/widgets/ModalWidget/component/index.tsx +++ b/app/client/src/widgets/ModalWidget/component/index.tsx @@ -26,6 +26,8 @@ import { AppState } from "@appsmith/reducers"; import { useWidgetDragResize } from "utils/hooks/dragResizeHooks"; import AnalyticsUtil from "utils/AnalyticsUtil"; import { closeTableFilterPane } from "actions/widgetActions"; +import { Colors } from "constants/Colors"; +import { scrollCSS } from "widgets/WidgetUtils"; const Container = styled.div<{ width?: number; @@ -79,14 +81,23 @@ const Container = styled.div<{ } } `; -const Content = styled.div<{ - height?: number; - scroll: boolean; - ref: RefObject; -}>` +const Content = styled.div<{ $scroll: boolean }>` overflow-x: hidden; width: 100%; height: 100%; + ${scrollCSS} + ${(props) => (!props.$scroll ? `overflow: hidden;` : ``)} +`; + +const Wrapper = styled.div<{ + $background?: string; + $borderRadius?: string; +}>` + overflow: hidden; + width: 100%; + height: 100%; + background: ${({ $background }) => `${$background || Colors.WHITE}`}; + border-radius: ${({ $borderRadius }) => $borderRadius}; `; const ComponentContainer = styled.div<{ @@ -125,6 +136,9 @@ export type ModalComponentProps = { widgetId: string; widgetName: string; isDynamicHeightEnabled: boolean; + background?: string; + borderRadius?: string; + settingsComponent?: ReactNode; }; /* eslint-disable react/display-name */ @@ -237,15 +251,24 @@ export default function ModalComponent(props: ModalComponentProps) { showLightBorder snapGrid={{ x: 1, y: 1 }} > - - {props.children} - + + {props.children} + + ); }; diff --git a/app/client/src/widgets/ModalWidget/widget/index.tsx b/app/client/src/widgets/ModalWidget/widget/index.tsx index d42f40a871..3987fe2a81 100644 --- a/app/client/src/widgets/ModalWidget/widget/index.tsx +++ b/app/client/src/widgets/ModalWidget/widget/index.tsx @@ -17,6 +17,9 @@ import { ValidationTypes } from "constants/WidgetValidation"; import { isAutoHeightEnabledForWidget } from "widgets/WidgetUtils"; import { Stylesheet } from "entities/AppTheming"; import { SelectionRequestType } from "sagas/WidgetSelectUtils"; +import WidgetNameComponent from "components/editorComponents/WidgetNameComponent"; +import { EVAL_ERROR_PATH } from "utils/DynamicBindingUtils"; +import { get } from "lodash"; const minSize = 100; @@ -201,11 +204,7 @@ export class ModalWidget extends BaseWidget { makeModalSelectable(content: ReactNode): ReactNode { // substitute coz the widget lacks draggable and position containers. return ( - + {content} ); @@ -231,8 +230,22 @@ export class ModalWidget extends BaseWidget { const isResizeEnabled = !isDragging && isWidgetFocused && isEditMode && !isSnipingMode; + const settingsComponent = isEditMode ? ( + + ) : null; + return ( { portalContainer={portalContainer} resizeModal={this.onModalResize} scrollContents={!!this.props.shouldScrollContents} + settingsComponent={settingsComponent} widgetId={this.props.widgetId} widgetName={this.props.widgetName} width={this.getModalWidth(this.props.width)} @@ -260,7 +274,7 @@ export class ModalWidget extends BaseWidget { getCanvasView() { let children = this.getChildren(); children = this.makeModalSelectable(children); - children = this.showWidgetName(children, true); + // children = this.showWidgetName(children, true); return this.makeModalComponent(children, true); }