Revert the zooming change on focus and restore old functionality (#3793)

* fix: revert the zooming change and restore old functionality * Refactor codeMapRenderService to use sortVisibleNodesByHeightDescending * fix: revert rest of changes to make tests green again * Refactor codeMap threeViewer controls and metricChooser nodeselection service tests * chore: refactor codeMap threeViewer controls and metricChooser nodeselection service tests * Revert focus behaviour to old one for stability * fix changelog --------- Co-authored-by: IhsenBouallegue <IhsenBouallegue@gmail.com>
MaibornWolff · Oct 25, 2024 · 0d29fce · 0d29fce
1 parent 5137912
commit 0d29fce
Show file tree

Hide file tree

Showing 21 changed files with 53 additions and 493 deletions.
diff --git a/visualization/CHANGELOG.md b/visualization/CHANGELOG.md
@@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/)
 - Add experimental feature: show buildings with an area metric value of 0 [#3789](https://github.com/MaibornWolff/codecharta/pull/3789)
 - Add further functionality for nested data to show popup when comparing files with different complexity metrics [#3791](https://github.com/MaibornWolff/codecharta/pull/3791)
 
+### Changed
+
+- Revert focus behaviour to old one for stability [#3793](https://github.com/MaibornWolff/codecharta/pull/3793)
+
 ### Fixed 🐞
 
 - Fix that default sample files are not removed when a new file is loaded by the user [#3768](https://github.com/MaibornWolff/codecharta/pull/3768)

diff --git a/visualization/app/app.module.ts b/visualization/app/app.module.ts
@@ -30,7 +30,6 @@ import { ChangelogDialogModule } from "./codeCharta/ui/dialogs/changelogDialog/c
 import { dialogs } from "./codeCharta/ui/dialogs/dialogs"
 import { BlacklistSearchPatternEffect } from "./codeCharta/ui/ribbonBar/searchPanel/searchBar/blacklistSearchPattern.effect"
 import { MaterialModule } from "./material/material.module"
-import { FocusEffects } from "./codeCharta/state/effects/focusNodes/focus.effect"
 import { IncompatibleMapsDialogModule } from "./codeCharta/ui/filePanel/filePanelDeltaSelector/incompatibleMapsDialog/incompatibleMapsDialog.module"
 
 @NgModule({
@@ -55,8 +54,7 @@ import { IncompatibleMapsDialogModule } from "./codeCharta/ui/filePanel/filePane
             SetLoadingIndicatorEffect,
             SaveCcStateEffect,
             UpdateQueryParametersEffect,
-            UpdateMapColorsEffect,
-            FocusEffects
+            UpdateMapColorsEffect
         ]),
         MaterialModule,
         FormsModule,

diff --git a/visualization/app/codeCharta/services/loadInitialFile/loadInitialFile.service.spec.ts b/visualization/app/codeCharta/services/loadInitialFile/loadInitialFile.service.spec.ts
@@ -372,7 +372,7 @@ describe("LoadInitialFileService", () => {
 
             expect(loadFileService.loadFiles).toHaveBeenCalledWith(mockedNameDataPairs)
             expect(mockedDialog.open).not.toHaveBeenCalled()
-            expect(dispatchSpy).toHaveBeenCalledTimes(countDifferences(mockedState.dynamicSettings, defaultDynamicSettings) - 1)
+            expect(dispatchSpy).toHaveBeenCalledTimes(countDifferences(mockedState.dynamicSettings, defaultDynamicSettings))
         })
 
         it("should set all differing  except sortingOrderAscending and isSearchPanelPinned", async () => {

diff --git a/visualization/app/codeCharta/services/loadInitialFile/loadInitialFile.service.ts b/visualization/app/codeCharta/services/loadInitialFile/loadInitialFile.service.ts
@@ -31,6 +31,7 @@ import { setColorMode } from "../../../../app/codeCharta/state/store/dynamicSett
 import { setColorRange } from "../../../../app/codeCharta/state/store/dynamicSettings/colorRange/colorRange.actions"
 import { setDistributionMetric } from "../../../../app/codeCharta/state/store/dynamicSettings/distributionMetric/distributionMetric.actions"
 import { setEdgeMetric } from "../../../../app/codeCharta/state/store/dynamicSettings/edgeMetric/edgeMetric.actions"
+import { setAllFocusedNodes } from "../../../../app/codeCharta/state/store/dynamicSettings/focusedNodePath/focusedNodePath.actions"
 import { setHeightMetric } from "../../../../app/codeCharta/state/store/dynamicSettings/heightMetric/heightMetric.actions"
 import { setMargin } from "../../../../app/codeCharta/state/store/dynamicSettings/margin/margin.actions"
 import { setSearchPattern } from "../../../../app/codeCharta/state/store/dynamicSettings/searchPattern/searchPattern.actions"
@@ -301,7 +302,7 @@ export class LoadInitialFileService {
                 this.store.dispatch(setDistributionMetric({ value }))
                 break
             case "focusedNodePath":
-                //ignore setting focused nodes
+                this.store.dispatch(setAllFocusedNodes({ value }))
                 break
             case "searchPattern":
                 this.store.dispatch(setSearchPattern({ value }))

diff --git a/...alization/app/codeCharta/state/effects/autoFitCodeMapChange/autoFitCodeMap.effect.spec.ts b/...alization/app/codeCharta/state/effects/autoFitCodeMapChange/autoFitCodeMap.effect.spec.ts
@@ -71,6 +71,13 @@ describe("autoFitCodeMapOnFileSelectionChangeEffect", () => {
         expect(mockedAutoFitTo).not.toHaveBeenCalled()
     })
 
+    it("should auto fit map when focused node paths has changed", () => {
+        store.overrideSelector(focusedNodePathSelector, [])
+        store.refreshState()
+        mockedRenderCodeMap$.next(undefined)
+        expect(mockedAutoFitTo).toHaveBeenCalledTimes(1)
+    })
+
     it("should auto fit map when layout algorithm has changed", () => {
         store.overrideSelector(layoutAlgorithmSelector, LayoutAlgorithm.TreeMapStreet)
         store.refreshState()

diff --git a/visualization/app/codeCharta/state/effects/autoFitCodeMapChange/autoFitCodeMap.effect.ts b/visualization/app/codeCharta/state/effects/autoFitCodeMapChange/autoFitCodeMap.effect.ts
@@ -7,6 +7,7 @@ import { ThreeMapControlsService } from "../../../ui/codeMap/threeViewer/threeMa
 import { visibleFileStatesSelector } from "../../selectors/visibleFileStates/visibleFileStates.selector"
 import { layoutAlgorithmSelector } from "../../store/appSettings/layoutAlgorithm/layoutAlgorithm.selector"
 import { resetCameraIfNewFileIsLoadedSelector } from "../../store/appSettings/resetCameraIfNewFileIsLoaded/resetCameraIfNewFileIsLoaded.selector"
+import { focusedNodePathSelector } from "../../store/dynamicSettings/focusedNodePath/focusedNodePath.selector"
 import { RenderCodeMapEffect } from "../renderCodeMapEffect/renderCodeMap.effect"
 
 @Injectable()
@@ -20,7 +21,11 @@ export class AutoFitCodeMapEffect {
 
     autoFitTo$ = createEffect(
         () =>
-            combineLatest([this.store.select(visibleFileStatesSelector), this.store.select(layoutAlgorithmSelector)]).pipe(
+            combineLatest([
+                this.store.select(visibleFileStatesSelector),
+                this.store.select(focusedNodePathSelector),
+                this.store.select(layoutAlgorithmSelector)
+            ]).pipe(
                 skip(1), // initial map load is already fitted
                 withLatestFrom(this.store.select(resetCameraIfNewFileIsLoadedSelector)),
                 filter(([, resetCameraIfNewFileIsLoaded]) => resetCameraIfNewFileIsLoaded),

diff --git a/visualization/app/codeCharta/state/effects/focusNodes/focus.effect.spec.ts b/visualization/app/codeCharta/state/effects/focusNodes/focus.effect.spec.ts
diff --git a/visualization/app/codeCharta/state/effects/focusNodes/focus.effect.ts b/visualization/app/codeCharta/state/effects/focusNodes/focus.effect.ts
diff --git a/visualization/app/codeCharta/state/effects/renderCodeMapEffect/actionsRequiringRerender.ts b/visualization/app/codeCharta/state/effects/renderCodeMapEffect/actionsRequiringRerender.ts
@@ -19,6 +19,12 @@ import { setColorMetric } from "../../store/dynamicSettings/colorMetric/colorMet
 import { setColorMode } from "../../store/dynamicSettings/colorMode/colorMode.actions"
 import { setColorRange } from "../../store/dynamicSettings/colorRange/colorRange.actions"
 import { setEdgeMetric } from "../../store/dynamicSettings/edgeMetric/edgeMetric.actions"
+import {
+    setAllFocusedNodes,
+    focusNode,
+    unfocusAllNodes,
+    unfocusNode
+} from "../../store/dynamicSettings/focusedNodePath/focusedNodePath.actions"
 import { setHeightMetric } from "../../store/dynamicSettings/heightMetric/heightMetric.actions"
 import { setMargin } from "../../store/dynamicSettings/margin/margin.actions"
 import { setSearchPattern } from "../../store/dynamicSettings/searchPattern/searchPattern.actions"
@@ -49,6 +55,10 @@ export const actionsRequiringRerender = [
     setColorRange,
     setMargin,
     setSearchPattern,
+    setAllFocusedNodes,
+    focusNode,
+    unfocusAllNodes,
+    unfocusNode,
     setHeightMetric,
     setAreaMetric,
     setColorMetric,

diff --git a/...pp/codeCharta/state/store/dynamicSettings/focusedNodePath/focusedNodePath.reducer.spec.ts b/...pp/codeCharta/state/store/dynamicSettings/focusedNodePath/focusedNodePath.reducer.spec.ts
@@ -9,6 +9,12 @@ describe("focusedNodePath", () => {
             expect(result).toEqual(["some/path/*.ts"])
         })
 
+        it("should add focusedNodePath", () => {
+            const result = focusedNodePath(["some/path/*.ts"], focusNode({ value: "foo.ts" }))
+
+            expect(result).toEqual(["foo.ts", "some/path/*.ts"])
+        })
+
         it("should not allow to focus root folder", () => {
             const result = focusedNodePath([], focusNode({ value: "/root" }))
 

diff --git a/...ion/app/codeCharta/state/store/dynamicSettings/focusedNodePath/focusedNodePath.reducer.ts b/...ion/app/codeCharta/state/store/dynamicSettings/focusedNodePath/focusedNodePath.reducer.ts
@@ -1,21 +1,12 @@
 import { focusNode, setAllFocusedNodes, unfocusAllNodes, unfocusNode } from "./focusedNodePath.actions"
 import { fileRoot } from "../../../../services/loadFile/fileRoot"
 import { createReducer, on } from "@ngrx/store"
-import { isChildPath } from "../../../../util/isChildPath"
 
 export const defaultFocusedNodePath: string[] = []
 export const focusedNodePath = createReducer(
     defaultFocusedNodePath,
     on(setAllFocusedNodes, (_state, action) => [...action.value]),
     on(unfocusAllNodes, () => []),
-    on(focusNode, (state, action) => {
-        if (action.value === fileRoot.rootPath) {
-            return state
-        }
-        if (isChildPath(action.value, state[0])) {
-            return [action.value, ...state]
-        }
-        return [action.value]
-    }),
+    on(focusNode, (state, action) => (action.value === fileRoot.rootPath ? state : [action.value, ...state])),
     on(unfocusNode, state => state.slice(1))
 )
diff --git a/visualization/app/codeCharta/ui/codeMap/codeMap.render.service.spec.ts b/visualization/app/codeCharta/ui/codeMap/codeMap.render.service.spec.ts
@@ -234,7 +234,7 @@ describe("codeMapRenderService", () => {
         })
     })
 
-    describe("sortNodes", () => {
+    describe("sortVisibleNodesByHeightDescending", () => {
         it("should return nodes with length of 0 and set min length if experimental features are enabled", () => {
             const newState = {
                 ...state.getValue(),
@@ -246,7 +246,7 @@ describe("codeMapRenderService", () => {
             store.dispatch(setState({ value: newState }))
 
             const nodes = klona(TEST_NODES)
-            const sortedNodes: Node[] = codeMapRenderService["sortNodes"](nodes)
+            const sortedNodes: Node[] = codeMapRenderService.sortVisibleNodesByHeightDescending(nodes)
 
             const updatedNode = klona(TEST_NODE_LEAF_0_LENGTH)
             updatedNode.length = 2
@@ -265,7 +265,7 @@ describe("codeMapRenderService", () => {
             store.dispatch(setState({ value: newState }))
 
             const nodes = klona(TEST_NODES)
-            const sortedNodes: Node[] = codeMapRenderService["sortNodes"](nodes)
+            const sortedNodes: Node[] = codeMapRenderService.sortVisibleNodesByHeightDescending(nodes)
 
             const result: Node[] = [TEST_NODE_ROOT, TEST_NODE_LEAF]
             expect(sortedNodes).toEqual(result)

diff --git a/visualization/app/codeCharta/ui/codeMap/codeMap.render.service.ts b/visualization/app/codeCharta/ui/codeMap/codeMap.render.service.ts
@@ -52,13 +52,13 @@ export class CodeMapRenderService implements OnDestroy {
 
     render(map: CodeMapNode) {
         const nodes = this.getNodes(map)
-        const sortedNodes = this.sortNodes(nodes)
-        this.unflattenedNodes = sortedNodes.filter(({ flat }) => !flat)
+        const visibleSortedNodes = this.sortVisibleNodesByHeightDescending(nodes)
+        this.unflattenedNodes = visibleSortedNodes.filter(({ flat }) => !flat)
 
-        this.setNewMapMesh(nodes, sortedNodes)
+        this.setNewMapMesh(nodes, visibleSortedNodes)
         this.getNodesMatchingColorSelector(this.unflattenedNodes)
         this.setLabels(this.unflattenedNodes)
-        this.setArrows(sortedNodes)
+        this.setArrows(visibleSortedNodes)
     }
 
     private setNewMapMesh(allMeshNodes, visibleSortedNodes) {
@@ -95,13 +95,13 @@ export class CodeMapRenderService implements OnDestroy {
         }
     }
 
-    sortNodes(nodes: Node[]) {
+    sortVisibleNodesByHeightDescending(nodes: Node[]) {
         const experimentalFeaturesEnabled = this.state.getValue().appSettings.experimentalFeaturesEnabled
         if (experimentalFeaturesEnabled) {
             this.setMinBuildingLength(nodes)
-            return nodes.filter(node => node.width > 0).sort((a, b) => b.height - a.height)
+            return nodes.filter(node => node.visible && node.width > 0).sort((a, b) => b.height - a.height)
         }
-        return nodes.filter(node => node.length > 0 && node.width > 0).sort((a, b) => b.height - a.height)
+        return nodes.filter(node => node.visible && node.length > 0 && node.width > 0).sort((a, b) => b.height - a.height)
     }
 
     private setMinBuildingLength(nodes: Node[]) {