fix(composer): selectedDataBinding not able to update selected node (#…

…274)

packages/scene-composer/src/SceneViewer.spec.tsx

@@ -5,20 +5,24 @@ const mockSceneComposerApi = {
   setSelectedSceneNodeRef: jest.fn(),
 };
 
+let onSceneLoadedCb;
 jest.doMock('./components/SceneComposerInternal', () => {
   const original = jest.requireActual('./components/SceneComposerInternal');
   return {
     ...original,
-    SceneComposerInternal: 'SceneComposerInternal',
+    SceneComposerInternal: (props) => {
+      onSceneLoadedCb = props.onSceneLoaded;
+      return mockComponent('SceneComposerInternal')(props);
+    },
     useSceneComposerApi: () => mockSceneComposerApi,
   }
 });
 
 import * as React from 'react';
 import renderer, { act } from 'react-test-renderer';
-import { useStore } from './store';
 import { SceneViewer } from './SceneViewer';
 import { KnownComponentType } from './interfaces';
+import mockComponent from '../__mocks__/mockComponent';
 /* eslint-enable */
 
 describe('SceneViewer', () => {
@@ -52,6 +56,14 @@ describe('SceneViewer', () => {
       container = renderer.create(<SceneViewer sceneLoader={mockSceneLoader} selectedDataBinding={mockLabel} />);
     });
 
+    // not called before scene is loaded
+    expect(mockSceneComposerApi.findSceneNodeRefBy).not.toBeCalled();
+    expect(mockSceneComposerApi.setCameraTarget).not.toBeCalled();
+
+    onSceneLoadedCb();
+    container.update(<SceneViewer sceneLoader={mockSceneLoader} selectedDataBinding={mockLabel} />);
+
+    // called after scene is loaded
     expect(mockSceneComposerApi.findSceneNodeRefBy).toBeCalledTimes(1);
     expect(mockSceneComposerApi.findSceneNodeRefBy).toBeCalledWith(mockLabel, [KnownComponentType.Tag]);
     expect(mockSceneComposerApi.setCameraTarget).toBeCalledTimes(1);
@@ -75,6 +87,9 @@ describe('SceneViewer', () => {
       container = renderer.create(<SceneViewer sceneLoader={mockSceneLoader} selectedDataBinding={mockLabel} />);
     });
 
+    onSceneLoadedCb();
+    container.update(<SceneViewer sceneLoader={mockSceneLoader} selectedDataBinding={mockLabel} />);
+
     expect(mockSceneComposerApi.findSceneNodeRefBy).toBeCalledTimes(1);
     expect(mockSceneComposerApi.setCameraTarget).toBeCalledTimes(0);
     expect(mockSceneComposerApi.setSelectedSceneNodeRef).toBeCalledTimes(1);

packages/scene-composer/src/SceneViewer.tsx

@@ -1,4 +1,4 @@
-import React, { useEffect, useMemo, useRef } from 'react';
+import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react';
 import { v4 as uuid } from 'uuid';
 import { isEqual } from 'lodash';
 import styled from 'styled-components';
@@ -25,11 +25,18 @@ export const SceneViewer: React.FC<SceneViewerProps> = ({ sceneComposerId, confi
   }, [sceneComposerId]);
   const composerApis = useSceneComposerApi(composerId);
   const prevSelectedRef: any = useRef();
+  const [sceneLoaded, setSceneLoaded] = useState(false);
 
   useEffect(() => {
+    // Do not update when scene is not loaded because nodes will be unavailable
+    if (!sceneLoaded) {
+      return;
+    }
+    // Do not update when there is no change
     if (isEqual(prevSelectedRef.current, props.selectedDataBinding)) {
       return;
     }
+
     prevSelectedRef.current = props.selectedDataBinding;
 
     if (props.selectedDataBinding === undefined) {
@@ -44,7 +51,11 @@ export const SceneViewer: React.FC<SceneViewerProps> = ({ sceneComposerId, confi
     } else {
       composerApis.setSelectedSceneNodeRef(undefined);
     }
-  }, [props.selectedDataBinding]);
+  }, [props.selectedDataBinding, sceneLoaded]);
+
+  const onSceneLoaded = useCallback(() => {
+    setSceneLoaded(true);
+  }, [setSceneLoaded]);
 
   return (
     <SceneComposerContainer data-testid={'webgl-root'}>
@@ -54,6 +65,7 @@ export const SceneViewer: React.FC<SceneViewerProps> = ({ sceneComposerId, confi
           ...(config || {}),
           mode: 'Viewing',
         }}
+        onSceneLoaded={onSceneLoaded}
         {...props}
       />
     </SceneComposerContainer>

packages/scene-composer/src/__snapshots__/SceneViewer.spec.tsx.snap

@@ -18,20 +18,12 @@ exports[`SceneViewer should render correctly 1`] = `
   className="c0"
   data-testid="webgl-root"
 >
-  <SceneComposerInternal
-    config={
-      Object {
-        "mode": "Viewing",
-      }
-    }
+  <div
+    config="{\\"mode\\":\\"Viewing\\"}"
+    data-mocked="SceneComposerInternal"
+    onSceneLoaded={[Function]}
     sceneComposerId="123"
-    sceneLoader={
-      Object {
-        "getSceneObject": [MockFunction],
-        "getSceneUri": [Function],
-        "getSceneUrl": [Function],
-      }
-    }
+    sceneLoader="{}"
   />
 </div>
 `;

packages/scene-composer/src/components/StateManager.tsx

@@ -36,6 +36,7 @@ const StateManager: React.FC<SceneComposerInternalProps> = ({
   sceneLoader,
   config,
   onSceneUpdated,
+  onSceneLoaded,
   valueDataBindingProvider,
   showAssetBrowserCallback,
   onWidgetClick,
@@ -55,6 +56,7 @@ const StateManager: React.FC<SceneComposerInternalProps> = ({
     setDataInput,
     setDataBindingTemplate,
     loadScene,
+    sceneLoaded,
     selectedSceneNodeRef,
     setSelectedSceneNodeRef,
     getSceneNodeByRef,
@@ -68,6 +70,7 @@ const StateManager: React.FC<SceneComposerInternalProps> = ({
   const messages = useStore(sceneComposerId)((state) => state.getMessages());
 
   const dataProviderRef = useRef<ProviderWithViewport<TimeSeriesData[]> | undefined>(undefined);
+  const prevSelection = useRef<string | undefined>(undefined);
 
   const { setActiveCameraSettings, setActiveCameraName } = useActiveCamera();
 
@@ -105,6 +108,13 @@ const StateManager: React.FC<SceneComposerInternalProps> = ({
   }, [activeCamera, selectedDataBinding]);
 
   useEffect(() => {
+    // This hook will somehow be triggered twice initially with selectedSceneNodeRef = undefined.
+    // Compare prevSelection with selectedSceneNodeRef to make sure the event is sent only when value
+    // is changed.
+    if (prevSelection.current === selectedSceneNodeRef) return;
+
+    prevSelection.current = selectedSceneNodeRef;
+
     if (onSelectionChanged) {
       const node = getSceneNodeByRef(selectedSceneNodeRef);
       const componentTypes = node?.components.map((component) => component.type) ?? [];
@@ -212,6 +222,16 @@ const StateManager: React.FC<SceneComposerInternalProps> = ({
     }
   }, [sceneContent]);
 
+  useEffect(() => {
+    if (onSceneLoaded && sceneLoaded) {
+      // Delay the event handler to let other components finish loading, otherwise the consumer side will
+      // fail to update scene states
+      setTimeout(() => {
+        onSceneLoaded();
+      }, 1);
+    }
+  }, [sceneLoaded, onSceneLoaded]);
+
   // Subscribe to store update
   useEffect(() => {
     if (onSceneUpdated) {

packages/scene-composer/src/components/three-fiber/EditorCamera.tsx

@@ -37,6 +37,7 @@ export const EditorMainCamera = React.forwardRef<Camera>((_, forwardedRef) => {
   const [cameraTarget, setCameraTarget] = useState<{ target?: FixedCameraTarget; shouldTween?: boolean }>();
   const [controlsRemove, setControlsRemove] = useState(false);
   const [setTween, updateTween] = useTween<TweenValueObject>();
+  const [mounted, setMounted] = useState(false);
 
   const activeCamera = useActiveCamera();
 
@@ -77,15 +78,32 @@ export const EditorMainCamera = React.forwardRef<Camera>((_, forwardedRef) => {
     }
   }, []);
 
-  // translate camera command
   useEffect(() => {
-    log?.verbose('setting camera target by command', cameraCommand);
+    setMounted(true);
+  }, []);
+
+  useEffect(() => {
+    // Don't update camera target before component is mounted because wrong bounding box will be calaulated.
+    if (!mounted) return;
+
+    if (!cameraCommand) {
+      // update the camera position to default
+      const sceneObject = scene.getObjectByName(ROOT_OBJECT_3D_NAME);
+      log?.verbose('setting camera target to root object', cameraCommand);
+
+      setCameraTarget({
+        target: sceneObject && findBestViewingPosition(sceneObject, true, cameraControlsImplRef.current),
+        shouldTween: true,
+      });
+    } else {
+      log?.verbose('setting camera target by command', cameraCommand);
 
-    setCameraTarget({
-      target: getCameraTargetByCommand(cameraCommand),
-      shouldTween: cameraCommand?.mode === 'transition',
-    });
-  }, [cameraCommand]);
+      setCameraTarget({
+        target: getCameraTargetByCommand(cameraCommand),
+        shouldTween: cameraCommand?.mode === 'transition',
+      });
+    }
+  }, [cameraCommand, mounted]);
 
   // execute camera command
   useEffect(() => {
@@ -136,15 +154,6 @@ export const EditorMainCamera = React.forwardRef<Camera>((_, forwardedRef) => {
     };
   }, [transformControls]);
 
-  // on mount, update the camera position
-  useEffect(() => {
-    const sceneObject = scene.getObjectByName(ROOT_OBJECT_3D_NAME);
-    setCameraTarget({
-      target: sceneObject && findBestViewingPosition(sceneObject, true, cameraControlsImplRef.current),
-      shouldTween: true,
-    });
-  }, []);
-
   useFrame(() => updateTween());
 
   useEffect(() => {

packages/scene-composer/src/components/three-fiber/ModelRefComponent/GLTFModelComponent.tsx

@@ -143,7 +143,7 @@ export const GLTFModelComponent: React.FC<GLTFModelProps> = ({
       }
     });
 
-    if (Date.now() - lastPointerMove >= CURSOR_VISIBILITY_TIMEOUT && !addingWidget) {
+    if (Date.now() - lastPointerMove >= CURSOR_VISIBILITY_TIMEOUT && !addingWidget && cursorVisible) {
       setCursorVisible(false);
     }
   });

packages/scene-composer/src/interfaces/sceneComposerInternal.ts

@@ -31,6 +31,7 @@ export type ShowAssetBrowserCallback = (callback: AssetBrowserResultCallback) =>
 
 export interface SceneComposerInternalProps extends SceneViewerPropsShared {
   onSceneUpdated?: OnSceneUpdateCallback;
+  onSceneLoaded?: () => void;
 
   valueDataBindingProvider?: IValueDataBindingProvider;
   showAssetBrowserCallback?: ShowAssetBrowserCallback;

packages/scene-composer/stories/SceneViewer.stories.tsx

@@ -151,7 +151,7 @@ export default {
 } as ComponentMeta<typeof SceneViewer>;
 
 export const Default: ComponentStory<typeof SceneViewer> = (args: SceneViewerProps & { sceneId?: string }) => {
-  const [selected, setSelected] = useState<any>(undefined);
+  const [selected, setSelected] = useState<any>({ entityId: 'room1', componentName: 'temperatureSensor2' });
   const loader = useMemo(() => {
     return args.sceneLoader;
   }, [args.sceneId]);
@@ -163,6 +163,8 @@ export const Default: ComponentStory<typeof SceneViewer> = (args: SceneViewerPro
 
   const onSelectionChanged = useCallback((e: ISelectionChangedEvent) => {
     const dataBindingContext = e.additionalComponentData?.[0].dataBindingContext;
+    console.log('onSelectionChanged', dataBindingContext);
+
     setSelected(
       dataBindingContext
         ? {

packages/scene-composer/tests/StateManager.spec.tsx

@@ -56,6 +56,8 @@ describe('StateManager', () => {
     loadScene: jest.fn(),
     getMessages: jest.fn().mockReturnValue(['messagge']),
     getObject3DBySceneNodeRef,
+    selectedSceneNodeRef: undefined,
+    sceneLoaded: false,
   };
   const mockSceneContent = 'This is test content';
   const mockGetSceneObjectFunction = jest.fn();
@@ -338,4 +340,86 @@ describe('StateManager', () => {
     });
     expect(useActiveCamera().setActiveCameraName).not.toBeCalled();
   });
+
+  it('should call onSceneLoaded', async () => {
+    useStore('default').setState({
+      ...baseState,
+    });
+    const onSceneLoaded = jest.fn();
+
+    let container;
+    await act(async () => {
+      container = renderer.create(
+        <StateManager
+          sceneLoader={mockSceneLoader}
+          config={{ dracoDecoder: true } as any}
+          onSceneLoaded={onSceneLoaded}
+        />,
+      );
+    });
+
+    useStore('default').setState({
+      ...baseState,
+      sceneLoaded: true,
+    });
+
+    container.update(
+      <StateManager
+        sceneLoader={mockSceneLoader}
+        config={{ dracoDecoder: true } as any}
+        onSceneLoaded={onSceneLoaded}
+      />,
+    );
+    await new Promise((resolve) => setTimeout(resolve, 1));
+
+    expect(onSceneLoaded).toBeCalledTimes(1);
+  });
+
+  it('should call onSelectionChanged as expected', async () => {
+    useStore('default').setState({
+      ...baseState,
+    });
+    const onSelectionChanged = jest.fn();
+
+    // not called when selection is not changed
+    let container;
+    await act(async () => {
+      container = renderer.create(
+        <StateManager
+          sceneLoader={mockSceneLoader}
+          config={{ dracoDecoder: true } as any}
+          onSelectionChanged={onSelectionChanged}
+        />,
+      );
+    });
+    expect(onSelectionChanged).not.toBeCalled();
+
+    // called when selection is changed
+    useStore('default').setState({
+      ...baseState,
+      selectedSceneNodeRef: 'abc',
+    });
+    container.update(
+      <StateManager
+        sceneLoader={mockSceneLoader}
+        config={{ dracoDecoder: true } as any}
+        onSelectionChanged={onSelectionChanged}
+      />,
+    );
+    expect(onSelectionChanged).toBeCalledTimes(1);
+
+    // not called when selection is not changed
+    useStore('default').setState({
+      ...baseState,
+      selectedSceneNodeRef: 'abc',
+    });
+    container.update(
+      <StateManager
+        sceneLoader={mockSceneLoader}
+        config={{ dracoDecoder: true } as any}
+        onSelectionChanged={onSelectionChanged}
+      />,
+    );
+    expect(onSelectionChanged).toBeCalledTimes(1);
+  });
 });