From 289c490217cb6b12daa077e585ebd347b7fde67a Mon Sep 17 00:00:00 2001 From: ashit-rath <88306433+ashit-rath@users.noreply.github.com> Date: Mon, 13 Sep 2021 18:05:05 +0530 Subject: [PATCH] fix: allow multiple draggable popper to co-exist (#7323) * fix: allow multiple draggable popper to co-exist * added custom data-cy for popper drag handler to fix failing cypress tests --- .../Draggable_PropertyPane_Widgets_spec.js | 18 +++++++++--------- .../Draggable_PropertyPane_spec.js | 18 +++++++++--------- app/client/src/pages/Editor/Popper.tsx | 9 ++++++++- .../src/pages/Editor/PropertyPane/index.tsx | 1 + app/client/src/pages/Editor/utils.ts | 8 ++++++++ 5 files changed, 35 insertions(+), 19 deletions(-) diff --git a/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/PropertyPane/Draggable_PropertyPane_Widgets_spec.js b/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/PropertyPane/Draggable_PropertyPane_Widgets_spec.js index 006fcf5f17..58a61d16b1 100644 --- a/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/PropertyPane/Draggable_PropertyPane_Widgets_spec.js +++ b/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/PropertyPane/Draggable_PropertyPane_Widgets_spec.js @@ -26,14 +26,14 @@ describe("Widget property pane draggable", function() { it("Property pane initial position same untill it is dragged", function() { for (const testWidget of widgetList) { cy.openPropertyPane(testWidget); - cy.get("#popper-draghandler").then((oldPorpPane) => { + cy.get("[data-cy=t--property-pane-drag-handle]").then((oldPorpPane) => { const oldPropPanePosition = oldPorpPane[0].getBoundingClientRect(); cy.get(commonlocators.collapsesection) .first() .click(); cy.get(commonlocators.editPropCrossButton).click({ force: true }); cy.openPropertyPane(testWidget); - cy.get("#popper-draghandler").then((newPropPane) => { + cy.get("[data-cy=t--property-pane-drag-handle]").then((newPropPane) => { const newPropPanePosition = newPropPane[0].getBoundingClientRect(); cy.get(commonlocators.editPropCrossButton).click({ force: true }); expect(oldPropPanePosition.top).to.be.equal(newPropPanePosition.top); @@ -47,15 +47,15 @@ describe("Widget property pane draggable", function() { it("Property pane position should stay same after dragging down", () => { for (const testWidget of widgetList) { cy.openPropertyPane(testWidget); - cy.get("#popper-draghandler") + cy.get("[data-cy=t--property-pane-drag-handle]") .trigger("mousedown", { which: 1 }) .trigger("mousemove", { clientX: 400, clientY: 500 }) .trigger("mouseup", { force: true }); - cy.get("#popper-draghandler").then((oldPorpPane) => { + cy.get("[data-cy=t--property-pane-drag-handle]").then((oldPorpPane) => { const oldPropPanePosition = oldPorpPane[0].getBoundingClientRect(); cy.get(commonlocators.editPropCrossButton).click({ force: true }); cy.openPropertyPane("containerwidget"); - cy.get("#popper-draghandler").then((newPropPane) => { + cy.get("[data-cy=t--property-pane-drag-handle]").then((newPropPane) => { const newPropPanePosition = newPropPane[0].getBoundingClientRect(); cy.get(commonlocators.editPropCrossButton).click({ force: true }); expect(oldPropPanePosition.top).to.be.equal(newPropPanePosition.top); @@ -70,20 +70,20 @@ describe("Widget property pane draggable", function() { it("Property pane should come back into view if forced to drop out of view", () => { for (const testWidget of widgetList) { cy.openPropertyPane(testWidget); - cy.get("#popper-draghandler") + cy.get("[data-cy=t--property-pane-drag-handle]") .trigger("mousedown", { which: 1 }) .trigger("mousemove", { clientX: -10, clientY: -20 }) .trigger("mouseup", { force: true }); - cy.get("#popper-draghandler").then((porpPane) => { + cy.get("[data-cy=t--property-pane-drag-handle]").then((porpPane) => { const propPanePosition = porpPane[0].getBoundingClientRect(); expect(propPanePosition.top).to.be.greaterThan(0); expect(propPanePosition.left).to.be.gte(0); }); - cy.get("#popper-draghandler") + cy.get("[data-cy=t--property-pane-drag-handle]") .trigger("mousedown", { which: 1 }) .trigger("mousemove", { clientX: 1600, clientY: 800 }) .trigger("mouseup", { force: true }); - cy.get("#popper-draghandler").then((porpPane) => { + cy.get("[data-cy=t--property-pane-drag-handle]").then((porpPane) => { const propPanePosition = porpPane[0].getBoundingClientRect(); cy.get(commonlocators.editPropCrossButton).click({ force: true }); expect(propPanePosition.top).to.be.lessThan( diff --git a/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/PropertyPane/Draggable_PropertyPane_spec.js b/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/PropertyPane/Draggable_PropertyPane_spec.js index fb7863e4de..315d92a0ba 100644 --- a/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/PropertyPane/Draggable_PropertyPane_spec.js +++ b/app/client/cypress/integration/Smoke_TestSuite/ClientSideTests/PropertyPane/Draggable_PropertyPane_spec.js @@ -8,14 +8,14 @@ describe("Table Widget property pane feature validation", function() { it("Property pane initial position same untill it is dragged", function() { cy.openPropertyPane("tablewidget"); - cy.get("#popper-draghandler").then((oldPorpPane) => { + cy.get("[data-cy=t--property-pane-drag-handle]").then((oldPorpPane) => { const oldPropPanePosition = oldPorpPane[0].getBoundingClientRect(); cy.get(commonlocators.collapsesection) .first() .click(); cy.get(commonlocators.editPropCrossButton).click({ force: true }); cy.openPropertyPane("tablewidget"); - cy.get("#popper-draghandler").then((newPropPane) => { + cy.get("[data-cy=t--property-pane-drag-handle]").then((newPropPane) => { const newPropPanePosition = newPropPane[0].getBoundingClientRect(); cy.get(commonlocators.editPropCrossButton).click({ force: true }); expect(oldPropPanePosition.top).to.be.equal(newPropPanePosition.top); @@ -26,15 +26,15 @@ describe("Table Widget property pane feature validation", function() { it("Property pane position should stay same after dragging down", () => { cy.openPropertyPane("tablewidget"); - cy.get("#popper-draghandler") + cy.get("[data-cy=t--property-pane-drag-handle]") .trigger("mousedown", { which: 1 }) .trigger("mousemove", { clientX: 400, clientY: 500 }) .trigger("mouseup", { force: true }); - cy.get("#popper-draghandler").then((oldPorpPane) => { + cy.get("[data-cy=t--property-pane-drag-handle]").then((oldPorpPane) => { const oldPropPanePosition = oldPorpPane[0].getBoundingClientRect(); cy.get(commonlocators.editPropCrossButton).click({ force: true }); cy.openPropertyPane("containerwidget"); - cy.get("#popper-draghandler").then((newPropPane) => { + cy.get("[data-cy=t--property-pane-drag-handle]").then((newPropPane) => { const newPropPanePosition = newPropPane[0].getBoundingClientRect(); cy.get(commonlocators.editPropCrossButton).click({ force: true }); expect(oldPropPanePosition.top).to.be.equal(newPropPanePosition.top); @@ -45,20 +45,20 @@ describe("Table Widget property pane feature validation", function() { it("Property pane should come back into view if forced to drop out of view", () => { cy.openPropertyPane("tablewidget"); - cy.get("#popper-draghandler") + cy.get("[data-cy=t--property-pane-drag-handle]") .trigger("mousedown", { which: 1 }) .trigger("mousemove", { clientX: -10, clientY: -20 }) .trigger("mouseup", { force: true }); - cy.get("#popper-draghandler").then((porpPane) => { + cy.get("[data-cy=t--property-pane-drag-handle]").then((porpPane) => { const propPanePosition = porpPane[0].getBoundingClientRect(); expect(propPanePosition.top).to.be.greaterThan(0); expect(propPanePosition.left).to.be.gte(0); }); - cy.get("#popper-draghandler") + cy.get("[data-cy=t--property-pane-drag-handle]") .trigger("mousedown", { which: 1 }) .trigger("mousemove", { clientX: 1600, clientY: 800 }) .trigger("mouseup", { force: true }); - cy.get("#popper-draghandler").then((porpPane) => { + cy.get("[data-cy=t--property-pane-drag-handle]").then((porpPane) => { const propPanePosition = porpPane[0].getBoundingClientRect(); cy.get(commonlocators.editPropCrossButton).click({ force: true }); expect(propPanePosition.top).to.be.lessThan( diff --git a/app/client/src/pages/Editor/Popper.tsx b/app/client/src/pages/Editor/Popper.tsx index 8db06c5af5..3814851b83 100644 --- a/app/client/src/pages/Editor/Popper.tsx +++ b/app/client/src/pages/Editor/Popper.tsx @@ -6,6 +6,7 @@ import { AppState } from "reducers"; import { getThemeDetails, ThemeMode } from "selectors/themeSelectors"; import styled, { ThemeProvider } from "styled-components"; import { noop } from "utils/AppsmithUtils"; +import { generateReactKey } from "utils/generators"; // import { PopperDragHandle } from "./PropertyPane/PropertyPaneConnections"; import { draggableElement } from "./utils"; @@ -26,6 +27,7 @@ export type PopperProps = { modifiers?: Partial; isDraggable?: boolean; disablePopperEvents?: boolean; + cypressSelectorDragHandle?: string; position?: { top: number; left: number; @@ -65,6 +67,9 @@ export function PopperDragHandle() { /* eslint-disable react/display-name */ export default (props: PopperProps) => { const contentRef = useRef(null); + const popperIdRef = useRef(generateReactKey()); + const popperId = popperIdRef.current; + const { isDraggable = false, disablePopperEvents = false, @@ -73,6 +78,7 @@ export default (props: PopperProps) => { onPositionChange = noop, themeMode = props.themeMode || ThemeMode.LIGHT, renderDragBlockPositions, + cypressSelectorDragHandle, } = props; // Meomoizing to avoid rerender of draggable icon. // What is the cost of memoizing? @@ -132,7 +138,7 @@ export default (props: PopperProps) => { if (isDraggable) { disablePopperEvents && _popper.disableEventListeners(); draggableElement( - "popper", + `${popperId}-popper`, _popper.popper, onPositionChange, position, @@ -145,6 +151,7 @@ export default (props: PopperProps) => { ), + cypressSelectorDragHandle, ); } diff --git a/app/client/src/pages/Editor/PropertyPane/index.tsx b/app/client/src/pages/Editor/PropertyPane/index.tsx index 33df784bc8..166ac290d9 100644 --- a/app/client/src/pages/Editor/PropertyPane/index.tsx +++ b/app/client/src/pages/Editor/PropertyPane/index.tsx @@ -251,6 +251,7 @@ class PropertyPane extends Component { )[0]; return ( JSX.Element, + cypressSelectorDragHandle?: string, ) => { let newXPos = 0, newYPos = 0, @@ -132,6 +133,7 @@ export const draggableElement = ( element, dragHandle, renderDragBlockPositions, + cypressSelectorDragHandle, ); } if (initPostion) { @@ -155,6 +157,7 @@ const createDragHandler = ( zIndex?: string; position?: string; }, + cypressSelectorDragHandle?: string, ) => { const oldDragHandler = document.getElementById(`${id}-draghandler`); const dragElement = document.createElement("div"); @@ -163,6 +166,11 @@ const createDragHandler = ( dragElement.style.left = renderDragBlockPositions?.left ?? "135px"; dragElement.style.top = renderDragBlockPositions?.top ?? "0px"; dragElement.style.zIndex = renderDragBlockPositions?.zIndex ?? "3"; + + if (cypressSelectorDragHandle) { + dragElement.setAttribute("data-cy", cypressSelectorDragHandle); + } + oldDragHandler ? el.replaceChild(dragElement, oldDragHandler) : el.appendChild(dragElement);