From 436980edf5e9bc7370ba663b6fca87aa1f945e16 Mon Sep 17 00:00:00 2001 From: Arpit Date: Fri, 13 Oct 2023 19:21:21 +0530 Subject: [PATCH] [appdef] fixes - dnd container cloning edge cases (#7820) * fixes: copy/pasting components updating wrong display preferences * fixes: copy/pasting tabs and cloning components inside tabs * fixes: duplication of calendar component bug * if components in subcontainer(children) are selected via selecto along with its parent, children should not be going through duplication * if components in subcontainer(children) are selected via selecto along with its parent, children should not be going through duplication --- frontend/src/Editor/EditorFunc.jsx | 6 +-- frontend/src/Editor/SubContainer.jsx | 2 +- frontend/src/_helpers/appUtils.js | 59 +++++++++++++++++------ frontend/src/_stores/utils.js | 16 +++--- server/src/services/components.service.ts | 25 +++++----- 5 files changed, 68 insertions(+), 40 deletions(-) diff --git a/frontend/src/Editor/EditorFunc.jsx b/frontend/src/Editor/EditorFunc.jsx index 7144a3d34d..4213a9d4c2 100644 --- a/frontend/src/Editor/EditorFunc.jsx +++ b/frontend/src/Editor/EditorFunc.jsx @@ -813,6 +813,8 @@ const EditorComponent = (props) => { const diffPatches = diff(appDefinition, updatedAppDefinition); + console.log('---arpit::---[diffPatches]', { diffPatches }); + const inversePatches = diff(updatedAppDefinition, appDefinition); const shouldUpdate = !_.isEmpty(diffPatches) && !isEqual(appDefinitionDiff, diffPatches); @@ -855,8 +857,6 @@ const EditorComponent = (props) => { isUpdatingEditorStateInProcess: updatingEditorStateInProcess, appDefinition: updatedAppDefinition, }); - - computeComponentState(updatedAppDefinition.pages[currentPageId]?.components); } if (config.ENABLE_MULTIPLAYER_EDITING && !opts?.skipYmapUpdate && opts?.currentSessionId !== currentSessionId) { @@ -920,7 +920,7 @@ const EditorComponent = (props) => { } else if (!isEmpty(editingVersion)) { // param diff ofr table columns needs the complte column data or else the json structure is not correct computeComponentPropertyDiff function handles this const paramDiff = computeComponentPropertyDiff(appDefinitionDiff, appDefinition, appDiffOptions); - const updateDiff = computeAppDiff(paramDiff, currentPageId, appDiffOptions); + const updateDiff = computeAppDiff(paramDiff, currentPageId, appDiffOptions, currentLayout); updateAppVersion(appId, editingVersion?.id, currentPageId, updateDiff, isUserSwitchedVersion) .then(() => { diff --git a/frontend/src/Editor/SubContainer.jsx b/frontend/src/Editor/SubContainer.jsx index 4d787b99dd..06f8d0758d 100644 --- a/frontend/src/Editor/SubContainer.jsx +++ b/frontend/src/Editor/SubContainer.jsx @@ -384,7 +384,7 @@ export const SubContainer = ({ enableReleasedVersionPopupState(); return; } - console.log('---arpit---onDragStop---'); + const canvasWidth = getContainerCanvasWidth(); const nodeBounds = direction.node.getBoundingClientRect(); diff --git a/frontend/src/_helpers/appUtils.js b/frontend/src/_helpers/appUtils.js index d81889df77..77dbf78784 100644 --- a/frontend/src/_helpers/appUtils.js +++ b/frontend/src/_helpers/appUtils.js @@ -1175,10 +1175,24 @@ export function renderTooltip({ props, text }) { ); } +/* +@computeComponentState: (components = {}) => Promise +This change is made to enhance the code readability by optimizing the logic +for computing component state. It replaces the previous try-catch block with +a more efficient approach, precomputing the parent component types and using +conditional checks for better performance and error handling.*/ + export function computeComponentState(components = {}) { let componentState = {}; const currentComponents = getCurrentState().components; + // Precompute parent component types + const parentComponentTypes = {}; + Object.keys(components).forEach((key) => { + const { component } = components[key]; + parentComponentTypes[key] = component.component; + }); + Object.keys(components).forEach((key) => { if (!components[key]) return; @@ -1189,17 +1203,9 @@ export function computeComponentState(components = {}) { const existingValues = currentComponents[existingComponentName]; if (component.parent) { - const parentComponent = components[component.parent]; - let isListView = false, - isForm = false; - try { - isListView = parentComponent.component.component === 'Listview'; - isForm = parentComponent.component.component === 'Form'; - } catch { - console.log('error'); - } + const parentComponentType = parentComponentTypes[component.parent]; - if (!isListView && !isForm) { + if (parentComponentType !== 'Listview' && parentComponentType !== 'Form') { componentState[component.name] = { ...componentMeta.exposedVariables, id: key, @@ -1375,7 +1381,7 @@ const updateNewComponents = (pageId, appDefinition, newComponents, updateAppDefi if (!isCut) { opts.cloningComponent = componentMap; } - + console.log('---arpit::---[updateAppDefinition]', { x: newAppDefinition.pages[pageId].components }); updateAppDefinition(newAppDefinition, opts); }; @@ -1390,11 +1396,27 @@ export const cloneComponents = ( if (selectedComponents.length < 1) return getSelectedText(); const { components: allComponents } = appDefinition.pages[currentPageId]; + + // if parent is selected, then remove the parent from the selected components + const filteredSelectedComponents = selectedComponents.filter((component) => { + const parentComponentId = component.component?.parent; + if (parentComponentId) { + // Check if the parent component is also selected + const isParentSelected = selectedComponents.some((comp) => comp.id === parentComponentId); + + // If the parent is selected, filter out the child component + if (isParentSelected) { + return false; + } + } + return true; + }); + let newDefinition = _.cloneDeep(appDefinition); let newComponents = [], newComponentObj = {}, addedComponentId = new Set(); - for (let selectedComponent of selectedComponents) { + for (let selectedComponent of filteredSelectedComponents) { if (addedComponentId.has(selectedComponent.id)) continue; const component = { component: allComponents[selectedComponent.id]?.component, @@ -1490,7 +1512,7 @@ const updateComponentLayout = (components, parentId, isCut = false) => { }); }); }; - +// const isChildOfTabsOrCalendar = (component, allComponents = [], componentParentId = undefined) => { const parentId = componentParentId ?? component.component?.parent?.split('-').slice(0, -1).join('-'); @@ -1528,10 +1550,14 @@ export const addComponents = ( ...finalComponents, }); - const isParentAlsoCopied = component.component.parent && componentMap[component.component.parent]; + const isParentTabOrCalendar = isChildOfTabsOrCalendar(component, pastedComponents, parentId); + const parentRef = isParentTabOrCalendar + ? component.component.parent.split('-').slice(0, -1).join('-') + : component.component.parent; + const isParentAlsoCopied = parentRef && componentMap[parentRef]; componentMap[component.componentId] = newComponentId; - let isChild = parentId ? parentId : component.component.parent; + let isChild = isParentAlsoCopied ? component.component.parent : parentId; const componentData = JSON.parse(JSON.stringify(component.component)); if (isCloning && parentId && !componentData.parent) { @@ -1563,10 +1589,11 @@ export const addComponents = ( }, layouts: component.layouts, }; + finalComponents[newComponentId] = newComponent; }); - !isCloning && updateComponentLayout(pastedComponents, parentId, isCut); + updateComponentLayout(pastedComponents, parentId, isCut); updateNewComponents(pageId, appDefinition, finalComponents, appDefinitionChanged, componentMap, isCut); !isCloning && toast.success('Component pasted succesfully'); diff --git a/frontend/src/_stores/utils.js b/frontend/src/_stores/utils.js index f7046622d8..62c1ed2d6d 100644 --- a/frontend/src/_stores/utils.js +++ b/frontend/src/_stores/utils.js @@ -46,8 +46,8 @@ const updateType = Object.freeze({ componentDeleted: 'components', }); -export const computeAppDiff = (appDiff, currentPageId, opts) => { - const { updateDiff, type, operation } = updateFor(appDiff, currentPageId, opts); +export const computeAppDiff = (appDiff, currentPageId, opts, currentLayout) => { + const { updateDiff, type, operation } = updateFor(appDiff, currentPageId, opts, currentLayout); return { updateDiff, type, operation }; }; @@ -121,7 +121,7 @@ export const computeComponentPropertyDiff = (appDiff, definition, opts) => { return _diff; }; -const updateFor = (appDiff, currentPageId, opts) => { +const updateFor = (appDiff, currentPageId, opts, currentLayout) => { const updateTypeMappings = [ { updateTypes: ['componentAdded', 'componentDefinitionChanged', 'componentDeleted', 'containerChanges'], @@ -155,7 +155,7 @@ const updateFor = (appDiff, currentPageId, opts) => { const optionsTypes = _.intersection(options, updateTypes); if (optionsTypes.length > 0) { - return processingFunction(appDiff, currentPageId, optionsTypes); + return processingFunction(appDiff, currentPageId, optionsTypes, currentLayout); } } @@ -197,7 +197,7 @@ const computePageUpdate = (appDiff, currentPageId, opts) => { return { updateDiff, type, operation }; }; -const computeComponentDiff = (appDiff, currentPageId, opts) => { +const computeComponentDiff = (appDiff, currentPageId, opts, currentLayout) => { let type; let updateDiff; let operation = 'update'; @@ -220,6 +220,10 @@ const computeComponentDiff = (appDiff, currentPageId, opts) => { const componentMeta = componentTypes.find((comp) => comp.component === component.component.component); + if (!componentMeta) { + return result; + } + const metaDiff = diff(componentMeta, component.component); result[id] = _.defaultsDeep(metaDiff, defaultComponent); @@ -245,7 +249,7 @@ const computeComponentDiff = (appDiff, currentPageId, opts) => { }); } - const currentDisplayPreference = _.keys(appDiff.pages[currentPageId].components[id].layouts)[0]; + const currentDisplayPreference = currentLayout; if (currentDisplayPreference === 'mobile') { result[id].others.showOnMobile = { value: '{{true}}' }; diff --git a/server/src/services/components.service.ts b/server/src/services/components.service.ts index 5f7655c263..1650ffe17d 100644 --- a/server/src/services/components.service.ts +++ b/server/src/services/components.service.ts @@ -131,11 +131,9 @@ export class ComponentsService { async componentLayoutChange(componenstLayoutDiff: object) { return dbTransactionWrap(async (manager: EntityManager) => { for (const componentId in componenstLayoutDiff) { - const { layouts } = componenstLayoutDiff[componentId]; + const doesComponentExist = await manager.findAndCount(Component, { id: componentId }); - const componentLayout = await manager.findOne(Layout, { componentId }); - - if (!componentLayout) { + if (!doesComponentExist[1]) { return { error: { message: `Component with id ${componentId} does not exist`, @@ -143,19 +141,18 @@ export class ComponentsService { }; } + const { layouts } = componenstLayoutDiff[componentId]; + for (const type in layouts) { - const layout = { - type, - ...layouts[type], - }; - const currentLayout = Object.assign({}, componentLayout); + const componentLayout = await manager.findOne(Layout, { componentId, type }); - const newLayout = { - ...currentLayout, - ...layout, - }; + if (componentLayout) { + const layout = { + ...layouts[type], + } as Partial; - await manager.update(Layout, { id: componentLayout.id }, newLayout); + await manager.update(Layout, { id: componentLayout.id }, layout); + } } } });