Skip to content

Commit 64f74c1

Browse files
authored
fix(react-components): preserve the camera state in URL that includes position, the target as the pivot point and the camera rotation (#5116)
* add direction optional when setting the camera state that would be coming from the url param mostly It separates the concerns of flexible camera that uses the target as the custom marker * refactor the camera state type definition and keep only the required and the direction in the state updates * update camera state control unit test to support direction as well * clean up * linter * cleanup assertion * remove unnecessary clone for direction in the default camera manager * revert viewer changes to keep it only in the viewer pr * bump react-components version to 0.77.3 and update viewer version to 4.25.1 to ship in the related changes * replace direction with the current rotation value * remove old mock with direction * add a comment for the camera state parameter type definition * update comment * refactor: using Pick instead of the manual type's incremental * update devdependency for reveal and use the identity when rotations are undefined * update lock file with reveal 4.25.1 * lint fix
1 parent 4fee725 commit 64f74c1

5 files changed

Lines changed: 70 additions & 26 deletions

File tree

react-components/package.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@cognite/reveal-react-components",
3-
"version": "0.77.2",
3+
"version": "0.77.3",
44
"exports": {
55
".": {
66
"import": "./dist/index.js",
@@ -37,7 +37,7 @@
3737
"sort-keys": "cdf-i18n-utils-cli sort-local-keys --namespace reveal-react-components --path ./src/common/i18n"
3838
},
3939
"peerDependencies": {
40-
"@cognite/reveal": "4.25.0",
40+
"@cognite/reveal": "4.25.1",
4141
"@cognite/sdk": "^10.0.0",
4242
"react": ">=18",
4343
"react-dom": ">=18",
@@ -57,7 +57,7 @@
5757
"devDependencies": {
5858
"@cognite/cdf-i18n-utils": "^0.7.5",
5959
"@cognite/cdf-utilities": "^3.6.0",
60-
"@cognite/reveal": "^4.24.0",
60+
"@cognite/reveal": "4.25.1",
6161
"@cognite/sdk": "^10.0.0",
6262
"@playwright/test": "1.49.0",
6363
"@storybook/addon-essentials": "8.4.5",

react-components/src/components/RevealCanvas/hooks/useCameraStateControl.test.ts

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { describe, expect, test, vi, beforeEach, beforeAll, afterAll } from 'vit
55

66
import { renderHook } from '@testing-library/react';
77

8-
import { Vector3 } from 'three';
8+
import { Quaternion, Vector3 } from 'three';
99
import { cameraManagerGlobalCameraEvents } from '#test-utils/fixtures/cameraManager';
1010
import { viewerMock } from '#test-utils/fixtures/viewer';
1111
import { useCameraStateControl, type CameraStateParameters } from './useCameraStateControl';
@@ -59,7 +59,14 @@ describe(useCameraStateControl.name, () => {
5959

6060
const { rerender } = renderHook(
6161
({ position }: { position: Vector3 }) => {
62-
useCameraStateControl({ position: position.clone(), target: new Vector3(1, 1, 1) }, setter);
62+
useCameraStateControl(
63+
{
64+
position: position.clone(),
65+
target: new Vector3(1, 1, 1),
66+
rotation: new Quaternion(0, 0, 0, 1)
67+
},
68+
setter
69+
);
6370
},
6471
{ initialProps: { position: new Vector3(0, 0, 0) } }
6572
);
@@ -75,28 +82,50 @@ describe(useCameraStateControl.name, () => {
7582
});
7683
});
7784

78-
test('provided setter is called after updating camera state internally', () => {
85+
test('provided setter is called after updating camera state internally for each parameter', () => {
7986
const setter = vi.fn<(cameraState?: CameraStateParameters) => void>();
8087

8188
const { rerender } = renderHook(() => {
8289
useCameraStateControl(
83-
{ position: new Vector3(0, 0, 0), target: new Vector3(1, 1, 1) },
90+
{
91+
position: new Vector3(0, 0, 0),
92+
target: new Vector3(1, 1, 1),
93+
rotation: new Quaternion(0, 0, 0, 1)
94+
},
8495
setter
8596
);
8697
});
8798

8899
vi.runAllTimers();
89100

101+
// position
90102
viewerMock.cameraManager.setCameraState({
91103
position: new Vector3(1, 0, 0),
92-
target: new Vector3(1, 1, 1)
104+
target: new Vector3(1, 1, 1),
105+
rotation: new Quaternion(0, 0, 0, 1)
93106
});
94107

95-
vi.runAllTimers();
108+
rerender();
109+
110+
// target
111+
viewerMock.cameraManager.setCameraState({
112+
position: new Vector3(0, 0, 0),
113+
target: new Vector3(0, 0, 1),
114+
rotation: new Quaternion(0, 0, 0, 1)
115+
});
116+
117+
rerender();
118+
119+
// rotation
120+
viewerMock.cameraManager.setCameraState({
121+
position: new Vector3(0, 0, 0),
122+
target: new Vector3(1, 1, 1),
123+
rotation: new Quaternion(0.1, 0.01, 0, 0.995)
124+
});
96125

97126
rerender();
98127
vi.runAllTimers();
99128

100-
expect(setter).toHaveBeenCalled();
129+
expect(setter).toHaveBeenCalledTimes(3);
101130
});
102131
});

react-components/src/components/RevealCanvas/hooks/useCameraStateControl.ts

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,11 @@
55
import { type MutableRefObject, useEffect, useRef } from 'react';
66
import { useReveal } from '../ViewerContext';
77
import { type CameraState } from '@cognite/reveal';
8+
import { Quaternion } from 'three';
89

9-
export type CameraStateParameters = Omit<Required<CameraState>, 'rotation'>;
10+
// Camera state parameters type with position and target as requires but rotation as optional
11+
export type CameraStateParameters = Omit<Required<CameraState>, 'rotation'> &
12+
Pick<CameraState, 'rotation'>;
1013

1114
export const useCameraStateControl = (
1215
externalCameraState?: CameraStateParameters,
@@ -17,7 +20,8 @@ export const useCameraStateControl = (
1720
? undefined
1821
: {
1922
position: externalCameraState.position.clone(),
20-
target: externalCameraState.target.clone()
23+
target: externalCameraState.target.clone(),
24+
rotation: externalCameraState.rotation?.clone()
2125
}
2226
);
2327

@@ -63,7 +67,8 @@ const useSetExternalCameraStateOnCameraMove = (
6367

6468
lastSetExternalState.current = {
6569
position: currentCameraManagerState.position.clone(),
66-
target: currentCameraManagerState.target.clone()
70+
target: currentCameraManagerState.target.clone(),
71+
rotation: currentCameraManagerState.rotation.clone()
6772
};
6873

6974
setCameraState?.(currentCameraManagerState);
@@ -89,10 +94,19 @@ function isCameraStatesEqual(
8994
}
9095

9196
const epsilon = 0.001;
92-
const { position: previousPosition, target: previousTarget } = previous;
93-
const { position: currentPosition, target: currentTarget } = current;
94-
return (
95-
previousPosition.distanceToSquared(currentPosition) <= epsilon &&
96-
previousTarget.distanceToSquared(currentTarget) <= epsilon
97-
);
97+
const {
98+
position: previousPosition,
99+
target: previousTarget,
100+
rotation: previousRotation
101+
} = previous;
102+
const { position: currentPosition, target: currentTarget, rotation: currentRotation } = current;
103+
104+
const isPositionStateEqual = previousPosition.distanceToSquared(currentPosition) <= epsilon;
105+
const isTargetStateEqual = previousTarget.distanceToSquared(currentTarget) <= epsilon;
106+
107+
const nonNullCurrentRotation = currentRotation ?? new Quaternion().identity();
108+
const nonNullPreviousRotation = previousRotation ?? new Quaternion().identity();
109+
const isRotationStateEqual = nonNullPreviousRotation.angleTo(nonNullCurrentRotation) <= epsilon;
110+
111+
return isPositionStateEqual && isTargetStateEqual && isRotationStateEqual;
98112
}

react-components/tests/tests-utilities/fixtures/cameraManager.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,10 @@ export const cameraManagerMock = new Mock<CameraManager>()
2626
)
2727
)
2828
.setup((p) => p.setCameraState)
29-
.returns(({ position, target }) => {
29+
.returns(({ position, target, rotation }) => {
3030
cameraManagerGlobalCurrentCameraState.position = position;
3131
cameraManagerGlobalCurrentCameraState.target = target;
32+
cameraManagerGlobalCurrentCameraState.rotation = rotation;
3233
setTimeout(() => {
3334
cameraManagerGlobalCameraEvents.cameraStop.forEach((callback) => {
3435
callback(position!, target!);

react-components/yarn.lock

Lines changed: 6 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)