diff --git a/changelogs/unreleased/5997-improve-useeffects.yml b/changelogs/unreleased/5997-improve-useeffects.yml new file mode 100644 index 000000000..43ffdc090 --- /dev/null +++ b/changelogs/unreleased/5997-improve-useeffects.yml @@ -0,0 +1,6 @@ +description: Improve behavior of useEffects in the Instance Composer when related inventories are updated +issue-nr: 5997 +change-type: patch +destination-branches: [master] +sections: + minor-improvement: "{{description}}" diff --git a/src/UI/Components/Diagram/Canvas.tsx b/src/UI/Components/Diagram/Canvas.tsx index d1a48ff48..771c791cc 100644 --- a/src/UI/Components/Diagram/Canvas.tsx +++ b/src/UI/Components/Diagram/Canvas.tsx @@ -8,6 +8,7 @@ import { DictModal, RightSidebar } from "./components"; import { Validation } from "./components/Validation"; import { createConnectionRules, createStencilState } from "./helpers"; import { diagramInit } from "./init"; +import { ActionEnum } from "./interfaces"; import { StencilSidebar } from "./stencil"; import { CanvasWrapper } from "./styles"; import { ZoomHandlerService } from "./zoomHandler"; @@ -38,6 +39,7 @@ export const Canvas: React.FC = ({ editable }) => { setServiceOrderItems, diagramHandlers, setDiagramHandlers, + setCellToEdit, } = useContext(CanvasContext); const Canvas = useRef(null); const LeftSidebar = useRef(null); @@ -54,7 +56,8 @@ export const Canvas: React.FC = ({ editable }) => { // create the diagram & set diagram handlers and the scroller only when service models and main service is defined and the stencil state is ready useEffect(() => { - if (!isStencilStateReady) { + //if diagram handlers are already set or the stencil state is not ready, return early to avoid re-creating the diagram or to creating it too early + if (!isStencilStateReady || diagramHandlers) { return; } @@ -71,20 +74,19 @@ export const Canvas: React.FC = ({ editable }) => { setDiagramHandlers(actions); - return () => { - setStencilState(createStencilState(mainService)); - setIsStencilStateReady(false); - actions.removeCanvas(); - }; // eslint-disable-next-line react-hooks/exhaustive-deps }, [mainService, serviceModels, isStencilStateReady]); /** - * create the left-sidebar only if the left-sidebar ref, the scroller, the related inventories by inter-service relations, the main service and service models are defined - * It's done in separate useEffect to enable eventual re-renders of the sidebar independently of the diagram, e.g. when the related inventories by inter-service relations are loaded + * create the stencil sidebar and zoom handler only when the scroller, related inventories and main service are defined and the ref for sidebar and zoomHandler is there */ useEffect(() => { - if (!LeftSidebar.current || !scroller || !relatedInventoriesQuery.data) { + if ( + !LeftSidebar.current || + !ZoomHandler.current || + !scroller || + !relatedInventoriesQuery.data + ) { return; } @@ -95,10 +97,14 @@ export const Canvas: React.FC = ({ editable }) => { mainService, serviceModels, ); + const zoomHandler = new ZoomHandlerService(ZoomHandler.current, scroller); setLeftSidebar(leftSidebar); - return () => leftSidebar.remove(); + return () => { + leftSidebar.remove(); + zoomHandler.remove(); + }; }, [scroller, relatedInventoriesQuery.data, mainService, serviceModels]); /** @@ -109,42 +115,41 @@ export const Canvas: React.FC = ({ editable }) => { if (!leftSidebar || !diagramHandlers || !isStencilStateReady) { return; } - const newInstances = new Map(); - const cells = diagramHandlers.addInstance(serviceModels, instance); - - cells.forEach((cell) => { - newInstances.set(cell.id, { - instance_id: cell.id, - service_entity: cell.get("entityName"), - config: {}, - action: instance ? null : "create", - attributes: cell.get("instanceAttributes"), - embeddedTo: cell.get("embeddedTo"), - relatedTo: cell.get("relatedTo"), + const newInstances = new Map(); + const copiedGraph = diagramHandlers.saveAndClearCanvas(); + + if (copiedGraph.cells.length > 0) { + diagramHandlers.loadState(copiedGraph); + } else { + const cells = diagramHandlers.addInstance(serviceModels, instance); + + cells.forEach((cell) => { + newInstances.set(cell.id, { + instance_id: cell.id, + service_entity: cell.get("entityName"), + config: {}, + action: instance ? null : ActionEnum.CREATE, + attributes: cell.get("instanceAttributes"), + embeddedTo: cell.get("embeddedTo"), + relatedTo: cell.get("relatedTo"), + }); }); - }); - setServiceOrderItems(newInstances); + setServiceOrderItems(newInstances); + } return () => { - setStencilState(createStencilState(mainService)); - setIsStencilStateReady(false); + setCellToEdit(null); }; // eslint-disable-next-line react-hooks/exhaustive-deps - }, [diagramHandlers, isStencilStateReady, leftSidebar]); - - /** - * add the zoom handler only when the scroller is defined as it's needed to create the zoom handler - */ - useEffect(() => { - if (!ZoomHandler.current || !scroller) { - return; - } - const zoomHandler = new ZoomHandlerService(ZoomHandler.current, scroller); - - return () => zoomHandler.remove(); - }, [scroller]); + }, [ + diagramHandlers, + isStencilStateReady, + leftSidebar, + serviceModels, + instance, + ]); return ( diff --git a/src/UI/Components/Diagram/Context/EventWrapper.tsx b/src/UI/Components/Diagram/Context/EventWrapper.tsx index db2a620e4..e198d89d2 100644 --- a/src/UI/Components/Diagram/Context/EventWrapper.tsx +++ b/src/UI/Components/Diagram/Context/EventWrapper.tsx @@ -6,6 +6,7 @@ import { RelationCounterForCell, } from "../interfaces"; import { ServiceEntityBlock } from "../shapes"; +import { toggleDisabledStencil } from "../stencil/helpers"; import { CanvasContext } from "./Context"; /** @@ -144,25 +145,12 @@ export const EventWrapper: React.FC = ({ break; } - const elements = [ - { selector: `.body_${name}`, className: "stencil_accent-disabled" }, - { selector: `.bodyTwo_${name}`, className: "stencil_body-disabled" }, - { selector: `.text_${name}`, className: "stencil_text-disabled" }, - ]; - const shouldDisable = stencil.max !== null && stencil.max !== undefined && stencil.current >= stencil.max; - // As in the docstrings mentioned, If the current count of the instances created from given stencil is more than or equal to the max count, disable the stencil of given embedded entity - elements.forEach(({ selector, className }) => { - const element = document.querySelector(selector); - - if (element) { - element.classList.toggle(className, shouldDisable); - } - }); + toggleDisabledStencil(name, shouldDisable); return stencilStateCopy; }); diff --git a/src/UI/Components/Diagram/actions.ts b/src/UI/Components/Diagram/actions.ts index 19c033c4d..b2431b14d 100644 --- a/src/UI/Components/Diagram/actions.ts +++ b/src/UI/Components/Diagram/actions.ts @@ -23,6 +23,7 @@ import { relationId, } from "./interfaces"; import { Link, ServiceEntityBlock } from "./shapes"; +import { toggleDisabledStencil } from "./stencil/helpers"; /** * Function that creates, appends and returns created Entity @@ -215,29 +216,7 @@ export function appendInstance( isBlockedFromEditing, ); - //disable Inventory Stencil for inter-service relation instance - const elements = [ - { - selector: `.body_${appendedInstances[0].get("stencilName")}`, - className: "stencil_accent-disabled", - }, - { - selector: `.bodyTwo_${appendedInstances[0].get("stencilName")}`, - className: "stencil_body-disabled", - }, - { - selector: `.text_${appendedInstances[0].get("stencilName")}`, - className: "stencil_text-disabled", - }, - ]; - - elements.forEach(({ selector, className }) => { - const element = document.querySelector(selector); - - if (element) { - element.classList.add(className); - } - }); + toggleDisabledStencil(appendedInstances[0].get("stencilName"), true); } else { //If cell is already in the graph, we need to check if it got in its inter-service relations the one with id that corresponds with created instanceAsTable let isConnected = false; diff --git a/src/UI/Components/Diagram/components/RightSidebar.tsx b/src/UI/Components/Diagram/components/RightSidebar.tsx index ea72bb7b5..42a606ff8 100644 --- a/src/UI/Components/Diagram/components/RightSidebar.tsx +++ b/src/UI/Components/Diagram/components/RightSidebar.tsx @@ -13,6 +13,7 @@ import { words } from "@/UI/words"; import { CanvasContext, InstanceComposerContext } from "../Context/Context"; import { updateServiceOrderItems } from "../helpers"; import { ActionEnum, EventActionEnum } from "../interfaces"; +import { toggleDisabledStencil } from "../stencil/helpers"; import { EntityForm } from "./EntityForm"; interface Props { @@ -104,29 +105,7 @@ export const RightSidebar: React.FC = ({ editable }) => { const stencilName = model.get("stencilName"); if (stencilName) { - //enable Inventory Stencil element for inter-service relation instance - const elements = [ - { - selector: `.body_${stencilName}`, - className: "stencil_accent-disabled", - }, - { - selector: `.bodyTwo_${stencilName}`, - className: "stencil_body-disabled", - }, - { - selector: `.text_${stencilName}`, - className: "stencil_text-disabled", - }, - ]; - - elements.forEach(({ selector, className }) => { - const element = document.querySelector(selector); - - if (element) { - element.classList.remove(className); - } - }); + toggleDisabledStencil(stencilName, false); } }; diff --git a/src/UI/Components/Diagram/init.ts b/src/UI/Components/Diagram/init.ts index 4e3da60f7..44edd2a9c 100644 --- a/src/UI/Components/Diagram/init.ts +++ b/src/UI/Components/Diagram/init.ts @@ -20,6 +20,7 @@ import { } from "./interfaces"; import { ComposerPaper } from "./paper"; import { ServiceEntityBlock } from "./shapes"; +import { toggleDisabledStencil } from "./stencil/helpers"; /** * Initializes the diagram. @@ -114,9 +115,38 @@ export function diagramInit( paper.unfreeze(); return { - removeCanvas: () => { - scroller.remove(); - paper.remove(); + /* eslint-disable-next-line @typescript-eslint/no-explicit-any */ + loadState: (graphJSON: any) => { + graph.fromJSON(graphJSON); + graph.getCells().forEach((cell) => { + if (cell.get("type") !== "Link") { + const copy = graphJSON.cells.find((c) => c.id === cell.id); + const stencilName = cell.get("stencilName"); + + if (cell.get("isEmbedded")) { + document.dispatchEvent( + new CustomEvent("updateStencil", { + detail: { + name: cell.get("entityName"), + }, + }), + ); + } + + if (stencilName) { + toggleDisabledStencil(stencilName, true); + } + cell.set("items", copy.items); // converted cells lacks "items" attribute + } + }); + }, + + saveAndClearCanvas: () => { + const copy = graph.toJSON(); + + graph.getCells().forEach((cell) => cell.remove()); + + return copy; }, addInstance: ( @@ -181,12 +211,21 @@ export function diagramInit( export interface DiagramHandlers { /** - * Removes the canvas. + * Saves the canvas state to JSON format and then clear the canvas. + * + * This function is responsible for cleaning up the canvas, and it's used when stencil/sidebar is updated to keep them aligned with each other + */ + /* eslint-disable-next-line @typescript-eslint/no-explicit-any */ + saveAndClearCanvas: () => any; //graph.toJSON() return object of Any type, the type is different from dia.Graph so I couldn't use it there and left as explicit any + + /** + * it loads the state of the canvas from the provided JSON object. * - * This function is responsible for cleaning up the canvas when it is no longer needed. - * removes the scroller and paper elements. + * @param {any} graphJSON - The JSON object representing the state of the canvas. graph.toJSON() return object of Any type, the type is different from dia.Graph so I couldn't use it there and left as explicit any + * @returns {void} */ - removeCanvas: () => void; + /* eslint-disable-next-line @typescript-eslint/no-explicit-any */ + loadState: (graphJSON: any) => void; /** * Adds an instance to the canvas. diff --git a/src/UI/Components/Diagram/stencil/helpers.test.ts b/src/UI/Components/Diagram/stencil/helpers.test.ts index 81b788e5e..cf9f77963 100644 --- a/src/UI/Components/Diagram/stencil/helpers.test.ts +++ b/src/UI/Components/Diagram/stencil/helpers.test.ts @@ -1,5 +1,7 @@ +import { screen } from "@testing-library/react"; import { containerModel, testApiInstanceModel } from "../Mocks"; import { + toggleDisabledStencil, createStencilElement, transformEmbeddedToStencilElements, } from "./helpers"; @@ -10,6 +12,7 @@ describe("createStencilElement", () => { "default", containerModel.embedded_entities[0], { + id: "123", attrOne: "test_value", attrTwo: "other_test_value", }, @@ -29,36 +32,39 @@ describe("createStencilElement", () => { ).toStrictEqual({ attrOne: "test_value", attrTwo: "other_test_value", + id: "123", }); - expect(embeddedElementWithModel.attributes.attrs?.body).toStrictEqual({ - width: 7, - height: 40, - x: 233, - d: "M 0 0 H calc(w) V calc(h) H 0 Z", - strokeWidth: 2, - fill: "var(--pf-v5-global--palette--blue-400)", - stroke: "none", - class: "body_default", - }); - expect(embeddedElementWithModel.attributes.attrs?.bodyTwo).toStrictEqual({ - width: 240, - height: 40, - fill: "var(--pf-v5-global--palette--white)", - stroke: "none", - class: "bodyTwo_default", - }); - expect(embeddedElementWithModel.attributes.attrs?.label).toStrictEqual({ - x: "10", - textAnchor: "start", - fontFamily: "sans-serif", - fontSize: 12, - text: "default", - refX: undefined, - class: "text_default", - y: "calc(h/2)", - textVerticalAnchor: "middle", - fill: "#333333", + + expect(embeddedElementWithModel.attributes.attrs?.body?.fill).toEqual( + "var(--pf-v5-global--palette--blue-400)", + ); + expect(embeddedElementWithModel.attributes.attrs?.body?.class).toEqual( + "body_default", + ); + + expect(embeddedElementWithModel.attributes.attrs?.bodyTwo?.fill).toEqual( + "var(--pf-v5-global--palette--white)", + ); + expect(embeddedElementWithModel.attributes.attrs?.bodyTwo?.class).toEqual( + "bodyTwo_default", + ); + + expect(embeddedElementWithModel.attributes.attrs?.label?.class).toEqual( + "text_default", + ); + + //check difference with body fill for non-embedded + const nonEmbedded = createStencilElement("nonEmbedded", containerModel, { + attrOne: "test_value", + attrTwo: "other_test_value", }); + + expect(nonEmbedded.attributes.attrs?.body?.fill).toEqual( + "var(--pf-v5-global--palette--purple-500)", + ); + expect(nonEmbedded.attributes.attrs?.body?.class).toEqual( + "body_nonEmbedded", + ); }); }); @@ -105,3 +111,144 @@ describe("transformEmbeddedToStencilElements", () => { expect(result[1].attributes.instanceAttributes).toStrictEqual({}); }); }); + +describe("toggleDisabledStencil", () => { + const setup = (name: string, disabled: boolean) => { + const elementsInfo = [ + [`body_${name}`, "stencil_accent-disabled"], + [`bodyTwo_${name}`, "stencil_body-disabled"], + [`text_${name}`, "stencil_text-disabled"], + ]; + + const elements = elementsInfo.map((element, index) => { + const div = document.createElement("div"); + + div.classList.add(element[0]); + div.setAttribute("data-testid", `${name}-${index}`); + + if (disabled) { + div.classList.add(element[1]); + } + + return div; + }); + + document.body.append(...elements); + }; + + beforeEach(() => { + document.body.innerHTML = ""; + }); + + it("without force set it toggles the stencil elements", () => { + setup("test", false); + + expect(screen.getByTestId("test-0")).not.toHaveClass( + "stencil_accent-disabled", + ); + expect(screen.getByTestId("test-1")).not.toHaveClass( + "stencil_body-disabled", + ); + expect(screen.getByTestId("test-2")).not.toHaveClass( + "stencil_text-disabled", + ); + + toggleDisabledStencil("test"); + + expect(screen.getByTestId("test-0")).toHaveClass("stencil_accent-disabled"); + expect(screen.getByTestId("test-1")).toHaveClass("stencil_body-disabled"); + expect(screen.getByTestId("test-2")).toHaveClass("stencil_text-disabled"); + + toggleDisabledStencil("test"); + + expect(screen.getByTestId("test-0")).not.toHaveClass( + "stencil_accent-disabled", + ); + expect(screen.getByTestId("test-1")).not.toHaveClass( + "stencil_body-disabled", + ); + expect(screen.getByTestId("test-2")).not.toHaveClass( + "stencil_text-disabled", + ); + }); + + it("with force set to false it disables the stencil elements", () => { + setup("test", false); + + expect(screen.getByTestId("test-0")).not.toHaveClass( + "stencil_accent-disabled", + ); + expect(screen.getByTestId("test-1")).not.toHaveClass( + "stencil_body-disabled", + ); + expect(screen.getByTestId("test-2")).not.toHaveClass( + "stencil_text-disabled", + ); + + toggleDisabledStencil("test", true); + + expect(screen.getByTestId("test-0")).toHaveClass("stencil_accent-disabled"); + expect(screen.getByTestId("test-1")).toHaveClass("stencil_body-disabled"); + expect(screen.getByTestId("test-2")).toHaveClass("stencil_text-disabled"); + }); + + it("with force set to true it enables the stencil elements", () => { + setup("test", true); + + expect(screen.getByTestId("test-0")).toHaveClass("stencil_accent-disabled"); + expect(screen.getByTestId("test-1")).toHaveClass("stencil_body-disabled"); + expect(screen.getByTestId("test-2")).toHaveClass("stencil_text-disabled"); + + toggleDisabledStencil("test", false); + + expect(screen.getByTestId("test-0")).not.toHaveClass( + "stencil_accent-disabled", + ); + expect(screen.getByTestId("test-1")).not.toHaveClass( + "stencil_body-disabled", + ); + expect(screen.getByTestId("test-2")).not.toHaveClass( + "stencil_text-disabled", + ); + }); + + it("with force set to false if we try to enable the enabled stencil elements nothing happens", () => { + setup("test", false); + + expect(screen.getByTestId("test-0")).not.toHaveClass( + "stencil_accent-disabled", + ); + expect(screen.getByTestId("test-1")).not.toHaveClass( + "stencil_body-disabled", + ); + expect(screen.getByTestId("test-2")).not.toHaveClass( + "stencil_text-disabled", + ); + + toggleDisabledStencil("test", false); + + expect(screen.getByTestId("test-0")).not.toHaveClass( + "stencil_accent-disabled", + ); + expect(screen.getByTestId("test-1")).not.toHaveClass( + "stencil_body-disabled", + ); + expect(screen.getByTestId("test-2")).not.toHaveClass( + "stencil_text-disabled", + ); + }); + + it("with force set to true if we try to disable the disabled stencil elements nothing happens", () => { + setup("test", true); + + expect(screen.getByTestId("test-0")).toHaveClass("stencil_accent-disabled"); + expect(screen.getByTestId("test-1")).toHaveClass("stencil_body-disabled"); + expect(screen.getByTestId("test-2")).toHaveClass("stencil_text-disabled"); + + toggleDisabledStencil("test", true); + + expect(screen.getByTestId("test-0")).toHaveClass("stencil_accent-disabled"); + expect(screen.getByTestId("test-1")).toHaveClass("stencil_body-disabled"); + expect(screen.getByTestId("test-2")).toHaveClass("stencil_text-disabled"); + }); +}); diff --git a/src/UI/Components/Diagram/stencil/helpers.ts b/src/UI/Components/Diagram/stencil/helpers.ts index 14ec42d3b..9cbda01b5 100644 --- a/src/UI/Components/Diagram/stencil/helpers.ts +++ b/src/UI/Components/Diagram/stencil/helpers.ts @@ -107,3 +107,42 @@ export const createStencilElement = ( ], }); }; + +/** + * Changes the availability of stencil elements by toggling their CSS classes. + * + * This function enables or disables stencil elements for an inter-service relation instance by toggling specific CSS classes. + * + * @param {string} stencilName - The name of the stencil. + * @param {boolean} force - if force is true, adds the disabled className . If force is false, removes disabled className + * + * @returns {void} + */ +export const toggleDisabledStencil = ( + stencilName: string, + force?: boolean, +): void => { + //disable Inventory Stencil for inter-service relation instance + const elements = [ + { + selector: `.body_${stencilName}`, + className: "stencil_accent-disabled", + }, + { + selector: `.bodyTwo_${stencilName}`, + className: "stencil_body-disabled", + }, + { + selector: `.text_${stencilName}`, + className: "stencil_text-disabled", + }, + ]; + + elements.forEach(({ selector, className }) => { + const element = document.querySelector(selector); + + if (element) { + element.classList.toggle(className, force); + } + }); +}; diff --git a/src/UI/Components/Diagram/stencil/inventoryStencil.ts b/src/UI/Components/Diagram/stencil/inventoryStencil.ts index 72126f4cb..b6b984ec2 100644 --- a/src/UI/Components/Diagram/stencil/inventoryStencil.ts +++ b/src/UI/Components/Diagram/stencil/inventoryStencil.ts @@ -3,7 +3,7 @@ import { global_palette_white } from "@patternfly/react-tokens"; import { ServiceModel } from "@/Core"; import { Inventories } from "@/Data/Managers/V2/GETTERS/GetInventoryList"; import { createComposerEntity } from "../actions"; -import { createStencilElement } from "./helpers"; +import { toggleDisabledStencil, createStencilElement } from "./helpers"; const GRID_SIZE = 8; const PADDING_S = GRID_SIZE; @@ -120,28 +120,11 @@ export class InventoryStencilTab { this.stencil.freeze(); //freeze by default as this tab is not active on init this.stencil.on("element:drop", (elementView) => { - const elements = [ - { - selector: `.body_${elementView.model.get("stencilName")}`, - className: "stencil_accent-disabled", - }, - { - selector: `.bodyTwo_${elementView.model.get("stencilName")}`, - className: "stencil_body-disabled", - }, - { - selector: `.text_${elementView.model.get("stencilName")}`, - className: "stencil_text-disabled", - }, - ]; + const stencilName = elementView.model.get("stencilName"); - elements.forEach(({ selector, className }) => { - const element = document.querySelector(selector); - - if (element) { - element.classList.add(className); - } - }); + if (stencilName) { + toggleDisabledStencil(stencilName, true); + } }); } }