Merge pull request #527 from contentstack/VB-593-v2

fix: Canvas element not selectable after field change

Author: hiteshshetty-dev

src/visualBuilder/utils/__test__/updateFocussedState.test.ts

@@ -6,7 +6,7 @@ import {
 import { VisualBuilder } from "../..";
 import {
     addFocusOverlay,
-    hideFocusOverlay,
+    hideOverlay,
 } from "../../generators/generateOverlay";
 import { mockGetBoundingClientRect } from "../../../__test__/utils";
 import { act } from "@testing-library/preact";
@@ -17,7 +17,7 @@ import { isFieldDisabled } from "../isFieldDisabled";
 
 vi.mock("../../generators/generateOverlay", () => ({
     addFocusOverlay: vi.fn(),
-    hideFocusOverlay: vi.fn(),
+    hideOverlay: vi.fn(),
 }));
 
 vi.mock("../getEntryPermissionsCached", () => ({
@@ -44,10 +44,10 @@ vi.mock("../../utils/fieldSchemaMap", () => {
     };
 });
 
+
 describe("updateFocussedState", () => {
     beforeEach(() => {
-        let previousSelectedEditableDOM: HTMLElement;
-        previousSelectedEditableDOM = document.createElement("div");
+        const previousSelectedEditableDOM = document.createElement("div");
         previousSelectedEditableDOM.setAttribute(
             "data-cslp",
             "content_type_uid.entry_uid.locale.field_path"
@@ -72,7 +72,7 @@ describe("updateFocussedState", () => {
         expect(result).toBeUndefined();
     });
 
-    it("should hide focus overlay if newPreviousSelectedElement is not found", () => {
+    it("should call hideOverlay if newPreviousSelectedElement is not found", () => {
         const resizeObserverMock = {
             disconnect: vi.fn(),
         } as unknown as ResizeObserver;
@@ -93,7 +93,13 @@ describe("updateFocussedState", () => {
             resizeObserver: resizeObserverMock,
         });
 
-        expect(hideFocusOverlay).toHaveBeenCalled();
+        expect(hideOverlay).toHaveBeenCalledWith({
+            visualBuilderOverlayWrapper: overlayWrapperMock,
+            focusedToolbar: focusedToolbarMock,
+            visualBuilderContainer: visualBuilderContainerMock,
+            resizeObserver: resizeObserverMock,
+            noTrigger: true,
+        });
         spyQuerySelector.mockRestore();
     });
 
@@ -241,8 +247,7 @@ describe("updateFocussedState", () => {
 
 describe("updateFocussedStateOnMutation", () => {
     beforeEach(() => {
-        let previousSelectedEditableDOM: HTMLElement;
-        previousSelectedEditableDOM = document.createElement("div");
+        const previousSelectedEditableDOM = document.createElement("div");
         previousSelectedEditableDOM.setAttribute(
             "data-cslp",
             "content_type_uid.entry_uid.locale.field_path"
@@ -261,7 +266,7 @@ describe("updateFocussedStateOnMutation", () => {
         expect(result).toBeUndefined();
     });
 
-    it("should hide focus overlay if newSelectedElement is not found", () => {
+    it("should call hideOverlay if newSelectedElement is not found", () => {
         const resizeObserverMock = {
             disconnect: vi.fn(),
         } as unknown as ResizeObserver;
@@ -278,7 +283,13 @@ describe("updateFocussedStateOnMutation", () => {
             resizeObserverMock
         );
 
-        expect(hideFocusOverlay).toHaveBeenCalled();
+        expect(hideOverlay).toHaveBeenCalledWith({
+            visualBuilderOverlayWrapper: focusOverlayWrapperMock,
+            focusedToolbar: focusedToolbarMock,
+            visualBuilderContainer: visualBuilderContainerMock,
+            resizeObserver: resizeObserverMock,
+            noTrigger: true,
+        });
     });
 
     it("should update focus outline dimensions", () => {

src/visualBuilder/utils/updateFocussedState.ts

@@ -3,7 +3,7 @@ import { extractDetailsFromCslp } from "../../cslp";
 import { getAddInstanceButtons } from "../generators/generateAddInstanceButtons";
 import {
     addFocusOverlay,
-    hideFocusOverlay,
+    hideOverlay,
 } from "../generators/generateOverlay";
 import { hideHoverOutline } from "../listeners/mouseHover";
 import {
@@ -120,7 +120,7 @@ export async function updateFocussedState({
         ) ||
         document.querySelector(`[data-cslp="${previousSelectedElementCslp}"]`);
     if (!newPreviousSelectedElement && resizeObserver) {
-        hideFocusOverlay({
+        hideOverlay({
             visualBuilderOverlayWrapper: overlayWrapper,
             focusedToolbar,
             visualBuilderContainer,
@@ -267,7 +267,7 @@ export function updateFocussedStateOnMutation(
             `[data-cslp-unique-id="${selectedElementCslpUniqueId}"]`
         ) || document.querySelector(`[data-cslp="${selectedElementCslp}"]`);
     if (!newSelectedElement && resizeObserver) {
-        hideFocusOverlay({
+        hideOverlay({
             visualBuilderOverlayWrapper: focusOverlayWrapper,
             focusedToolbar,
             visualBuilderContainer,
@@ -302,6 +302,9 @@ export function updateFocussedStateOnMutation(
         }
     }
 
+    //TODO: This logic for overlay position is already present in generateOverlay as `addFocusOverlay`.
+    // We should refactor this to use the same logic. Refer "VB-593" branch for more details.
+
     /**
      * Update the focus overlays if they exists.
      */
@@ -388,6 +391,8 @@ export function updateFocussedStateOnMutation(
      * Update the focus toolbar if it exists.
      */
 
+    //TODO: This logic for toolbar position is already present in same file as `positionToolbar`.
+    // We should refactor this to use the same logic. Refer "VB-593" branch for more details.
     if (focusedToolbar) {
         const targetElementRightEdgeOffset =
             window.scrollX + window.innerWidth - selectedElementDimension.left;