From d54779c971892f90d0d74980f27dc232061f938e Mon Sep 17 00:00:00 2001 From: nereboss Date: Wed, 10 Jan 2024 11:35:18 +0100 Subject: [PATCH 1/7] Add label for the currently selected building #1234 --- .../ui/codeMap/codeMap.mouseEvent.service.ts | 40 +++++++++++++++---- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/visualization/app/codeCharta/ui/codeMap/codeMap.mouseEvent.service.ts b/visualization/app/codeCharta/ui/codeMap/codeMap.mouseEvent.service.ts index 76a9f38d8e..641b18936f 100755 --- a/visualization/app/codeCharta/ui/codeMap/codeMap.mouseEvent.service.ts +++ b/visualization/app/codeCharta/ui/codeMap/codeMap.mouseEvent.service.ts @@ -51,6 +51,7 @@ export class CodeMapMouseEventService implements OnDestroy { private isMoving = false private raycaster = new Raycaster() private temporaryLabelForBuilding = null + private temporaryLabelForSelectedBuilding = null private subscriptions = [ this.store .select(visibleFileStatesSelector) @@ -216,17 +217,14 @@ export class CodeMapMouseEventService implements OnDestroy { const to = this.intersectedBuilding if (from?.id !== to?.id) { - if (this.temporaryLabelForBuilding !== null) { - this.codeMapLabelService.clearTemporaryLabel(this.temporaryLabelForBuilding) - this.temporaryLabelForBuilding = null - } + this.clearTemporaryLabel() this.threeSceneService.resetLabel() this.unhoverBuilding() if (to && !this.isGrabbingOrMoving()) { if (to.node.isLeaf) { const labelForBuilding = - this.threeSceneService.getLabelForHoveredNode(to, labels) ?? this.drawTemporaryLabelFor(to, labels) + this.threeSceneService.getLabelForHoveredNode(to, labels) ?? this.drawTemporaryLabelFor(to) this.threeSceneService.animateLabel(labelForBuilding, this.raycaster, labels) } this.hoverBuilding(to) @@ -236,11 +234,11 @@ export class CodeMapMouseEventService implements OnDestroy { } } - private drawTemporaryLabelFor(codeMapBuilding: CodeMapBuilding, labels: Object3D[]) { + private drawTemporaryLabelFor(codeMapBuilding: CodeMapBuilding) { const enforceLabel = true this.codeMapLabelService.addLeafLabel(codeMapBuilding.node, 0, enforceLabel) - labels = this.threeSceneService.labels?.children + const labels = this.threeSceneService.labels?.children const labelForBuilding = this.threeSceneService.getLabelForHoveredNode(codeMapBuilding, labels) this.temporaryLabelForBuilding = codeMapBuilding.node @@ -355,14 +353,42 @@ export class CodeMapMouseEventService implements OnDestroy { if (!this.hasMouseMovedMoreThanThreePixels(this.mouseOnLastClick)) { if (this.intersectedBuilding) { this.threeSceneService.selectBuilding(this.intersectedBuilding) + this.drawLabelForSelected(this.intersectedBuilding) } else { this.threeSceneService.clearSelection() + this.clearLabelForSelected() } this.threeSceneService.clearConstantHighlight() } this.threeRendererService.render() } + private drawLabelForSelected(codeMapBuilding: CodeMapBuilding) { + if (this.temporaryLabelForSelectedBuilding !== null) { + this.clearTemporaryLabel() + this.codeMapLabelService.clearTemporaryLabel(this.temporaryLabelForSelectedBuilding) + } + if (!codeMapBuilding.node.isLeaf) { + return + } + + this.codeMapLabelService.addLeafLabel(codeMapBuilding.node, 0, true) + + const labels = this.threeSceneService.labels?.children + const labelForBuilding = this.threeSceneService.getLabelForHoveredNode(codeMapBuilding, labels) + this.threeSceneService.animateLabel(labelForBuilding, this.raycaster, labels) + + this.temporaryLabelForSelectedBuilding = codeMapBuilding.node + return labelForBuilding + } + + private clearLabelForSelected() { + if (this.temporaryLabelForSelectedBuilding !== null) { + this.codeMapLabelService.clearTemporaryLabel(this.temporaryLabelForSelectedBuilding) + this.temporaryLabelForSelectedBuilding = null + } + } + private hasMouseMovedMoreThanThreePixels({ x, y }: Coordinates) { return ( Math.abs(this.mouse.x - x) > this.THRESHOLD_FOR_MOUSE_MOVEMENT_TRACKING || From a7a9c1acd85ca8f526f860138fbd2c9a8095df02 Mon Sep 17 00:00:00 2001 From: nereboss Date: Wed, 10 Jan 2024 14:52:25 +0100 Subject: [PATCH 2/7] Fix failing tests and add (still empty) new tests #1234 --- .../codeMap.mouseEvent.service.spec.ts | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/visualization/app/codeCharta/ui/codeMap/codeMap.mouseEvent.service.spec.ts b/visualization/app/codeCharta/ui/codeMap/codeMap.mouseEvent.service.spec.ts index 09e36058bf..d48edaf152 100644 --- a/visualization/app/codeCharta/ui/codeMap/codeMap.mouseEvent.service.spec.ts +++ b/visualization/app/codeCharta/ui/codeMap/codeMap.mouseEvent.service.spec.ts @@ -521,6 +521,9 @@ describe("codeMapMouseEventService", () => { it("should call selectBuilding when no building is selected", () => { threeSceneService.getSelectedBuilding = jest.fn() + threeSceneService.getLabelForHoveredNode = jest.fn() + threeSceneService.animateLabel = jest.fn() + codeMapMouseEventService["intersectedBuilding"] = codeMapBuilding codeMapMouseEventService.onDocumentMouseUp(event) @@ -530,6 +533,9 @@ describe("codeMapMouseEventService", () => { it("should call selectBuilding when a new building is selected", () => { threeSceneService.getSelectedBuilding = jest.fn().mockReturnValue(new CodeMapBuilding(200, null, null, null)) + threeSceneService.getLabelForHoveredNode = jest.fn() + threeSceneService.animateLabel = jest.fn() + codeMapMouseEventService["intersectedBuilding"] = codeMapBuilding codeMapMouseEventService.onDocumentMouseUp(event) @@ -548,6 +554,9 @@ describe("codeMapMouseEventService", () => { }) it("should not call clearSelection, when the mouse has moved less or exact 3 pixels but a building is currently being clicked upon", () => { + threeSceneService.getLabelForHoveredNode = jest.fn() + threeSceneService.animateLabel = jest.fn() + codeMapMouseEventService.onDocumentMouseMove(event) codeMapMouseEventService.onDocumentMouseDown(event) codeMapMouseEventService.onDocumentMouseMove({ clientX: 10, clientY: 17 } as MouseEvent) @@ -817,7 +826,7 @@ describe("codeMapMouseEventService", () => { threeSceneService.getLabelForHoveredNode = jest.fn() codeMapLabelService.addLeafLabel = jest.fn() - codeMapMouseEventService["drawTemporaryLabelFor"](codeMapBuilding, null) + codeMapMouseEventService["drawTemporaryLabelFor"](codeMapBuilding) const nodeHeight = codeMapBuilding.node.height + Math.abs(codeMapBuilding.node.heightDelta ?? 0) expect(threeSceneService.getLabelForHoveredNode).toHaveBeenCalled() @@ -829,10 +838,20 @@ describe("codeMapMouseEventService", () => { threeSceneService.getLabelForHoveredNode = jest.fn() codeMapLabelService.addLeafLabel = jest.fn() - codeMapMouseEventService["drawTemporaryLabelFor"](codeMapBuilding, null) + codeMapMouseEventService["drawTemporaryLabelFor"](codeMapBuilding) expect(threeSceneService.getLabelForHoveredNode).toHaveBeenCalled() expect(codeMapLabelService.addLeafLabel).toHaveBeenCalledWith(codeMapBuilding.node, 0, true) }) }) + + describe("labelForSelectedBuilding", () => { + it("should create a label when clicking on a building", () => {}) + + it("should remove the label when the building is unselected", () => {}) + + it("should remove the old and create the new label when selected building is changed", () => {}) + + it("should keep the label when clicking on the already selected building", () => {}) + }) }) From 454a11ae1cabe4b7d9942384434154d8373c1885 Mon Sep 17 00:00:00 2001 From: nereboss Date: Wed, 10 Jan 2024 14:59:56 +0100 Subject: [PATCH 3/7] Update Changelog #1234 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c69dfe9c18..979058d73f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/) - Clarify sonar user token question [#3445](https://github.com/MaibornWolff/codecharta/pull/3445) - Changed the `--user` flag to `--user-token` in SonarImporter [#3445](https://github.com/MaibornWolff/codecharta/pull/3445) - Changed the interactive dialog of `modify` to prompt user for single action to perform [#3448](https://github.com/MaibornWolff/codecharta/pull/3448) +- Selected buildings now keep their label untul they are unselected [#3465](https://github.com/MaibornWolff/codecharta/pull/3465) ### Fixed 🐞 From 589cc0e230e8982786129d7019ca95b1e18fc3a6 Mon Sep 17 00:00:00 2001 From: nereboss Date: Thu, 11 Jan 2024 14:33:25 +0100 Subject: [PATCH 4/7] Add test cases #1234 --- .../codeMap.mouseEvent.service.spec.ts | 46 +++++++++++++++++-- 1 file changed, 42 insertions(+), 4 deletions(-) diff --git a/visualization/app/codeCharta/ui/codeMap/codeMap.mouseEvent.service.spec.ts b/visualization/app/codeCharta/ui/codeMap/codeMap.mouseEvent.service.spec.ts index d48edaf152..ebdb9ede16 100644 --- a/visualization/app/codeCharta/ui/codeMap/codeMap.mouseEvent.service.spec.ts +++ b/visualization/app/codeCharta/ui/codeMap/codeMap.mouseEvent.service.spec.ts @@ -846,12 +846,50 @@ describe("codeMapMouseEventService", () => { }) describe("labelForSelectedBuilding", () => { - it("should create a label when clicking on a building", () => {}) + it("should create a label when selecting a building", () => { + threeSceneService.getLabelForHoveredNode = jest.fn() + threeSceneService.animateLabel = jest.fn() + codeMapLabelService.addLeafLabel = jest.fn() - it("should remove the label when the building is unselected", () => {}) + codeMapMouseEventService["drawLabelForSelected"](codeMapBuilding) + + expect(threeSceneService.getLabelForHoveredNode).toHaveBeenCalled() + expect(codeMapLabelService.addLeafLabel).toHaveBeenCalledWith(codeMapBuilding.node, 0, true) + expect(codeMapMouseEventService["temporaryLabelForSelectedBuilding"]).toEqual(codeMapBuilding.node) + }) + + it("should remove the label when a previously selected building is unselected", () => { + threeSceneService.getLabelForHoveredNode = jest.fn() + threeSceneService.animateLabel = jest.fn() + codeMapLabelService.clearTemporaryLabel = jest.fn() + codeMapMouseEventService["drawLabelForSelected"](codeMapBuilding) - it("should remove the old and create the new label when selected building is changed", () => {}) + codeMapMouseEventService["clearLabelForSelected"]() - it("should keep the label when clicking on the already selected building", () => {}) + expect(codeMapLabelService.clearTemporaryLabel).toHaveBeenCalledWith(codeMapBuilding.node) + expect(codeMapMouseEventService["temporaryLabelForSelectedBuilding"]).toBeNull() + }) + + it("should remove the old and create the new label when selected building is changed", () => { + //TODO we need two buildings for this, check if we have two in the mocks + }) + + it("should keep the label when clicking on the already selected building", () => { + threeSceneService.getLabelForHoveredNode = jest.fn() + threeSceneService.animateLabel = jest.fn() + codeMapMouseEventService["clearTemporaryLabel"] = jest.fn() + codeMapMouseEventService["drawLabelForSelected"] = jest.fn() + + codeMapMouseEventService["drawLabelForSelected"](codeMapBuilding) + const referenceLabel = codeMapMouseEventService["temporaryLabelForSelectedBuilding"] + codeMapMouseEventService["intersectedBuilding"] = codeMapBuilding + + codeMapMouseEventService["onLeftClick"]() + + expect(codeMapMouseEventService["drawLabelForSelected"]).toHaveBeenCalledWith(codeMapBuilding) + expect(codeMapMouseEventService["drawLabelForSelected"]).toHaveBeenCalledTimes(2) + expect(codeMapMouseEventService["clearTemporaryLabel"]).not.toHaveBeenCalled() + expect(codeMapMouseEventService["temporaryLabelForSelectedBuilding"]).toEqual(referenceLabel) + }) }) }) From 3d14938e5fb2f0f811c9636d6da32931545917d9 Mon Sep 17 00:00:00 2001 From: nereboss Date: Thu, 11 Jan 2024 15:14:00 +0100 Subject: [PATCH 5/7] Update test cases #1234 --- .../codeMap.mouseEvent.service.spec.ts | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/visualization/app/codeCharta/ui/codeMap/codeMap.mouseEvent.service.spec.ts b/visualization/app/codeCharta/ui/codeMap/codeMap.mouseEvent.service.spec.ts index ebdb9ede16..e53b1a154d 100644 --- a/visualization/app/codeCharta/ui/codeMap/codeMap.mouseEvent.service.spec.ts +++ b/visualization/app/codeCharta/ui/codeMap/codeMap.mouseEvent.service.spec.ts @@ -5,7 +5,6 @@ import { ThreeSceneService } from "./threeViewer/threeSceneService" import { ThreeRendererService } from "./threeViewer/threeRenderer.service" import { ViewCubeMouseEventsService } from "../viewCube/viewCube.mouseEvents.service" import { CodeMapBuilding } from "./rendering/codeMapBuilding" -import { CODE_MAP_BUILDING, CONSTANT_HIGHLIGHT, TEST_FILE_WITH_PATHS, TEST_NODES } from "../../util/dataMocks" import { BlacklistItem, CcState, CodeMapNode, Node } from "../../codeCharta.model" import { NodeDecorator } from "../../util/nodeDecorator" import { klona } from "klona" @@ -19,6 +18,7 @@ import { setRightClickedNodeData } from "../../state/store/appStatus/rightClicke import { State, Store } from "@ngrx/store" import { MockStore, provideMockStore } from "@ngrx/store/testing" import { defaultState } from "../../state/store/state.manager" +import { CODE_MAP_BUILDING, CODE_MAP_BUILDING_TS_NODE, CONSTANT_HIGHLIGHT, TEST_FILE_WITH_PATHS, TEST_NODES } from "../../util/dataMocks" jest.mock("../../state/selectors/accumulatedData/idToNode.selector", () => ({ idToNodeSelector: jest.fn() @@ -871,7 +871,25 @@ describe("codeMapMouseEventService", () => { }) it("should remove the old and create the new label when selected building is changed", () => { - //TODO we need two buildings for this, check if we have two in the mocks + const oldSelection = codeMapBuilding + const newSelection = CODE_MAP_BUILDING_TS_NODE + + threeSceneService.getLabelForHoveredNode = jest.fn() + threeSceneService.animateLabel = jest.fn() + codeMapLabelService.clearTemporaryLabel = jest.fn() + codeMapLabelService.addLeafLabel = jest.fn() + + codeMapMouseEventService["drawLabelForSelected"](codeMapBuilding) + + codeMapMouseEventService["intersectedBuilding"] = newSelection + codeMapMouseEventService["onLeftClick"]() + + expect(codeMapMouseEventService["temporaryLabelForSelectedBuilding"]).not.toEqual(oldSelection.node) + expect(codeMapMouseEventService["temporaryLabelForSelectedBuilding"]).toEqual(newSelection.node) + expect(codeMapLabelService.clearTemporaryLabel).toHaveBeenCalledWith(codeMapBuilding.node) + expect(codeMapLabelService.addLeafLabel).toHaveBeenCalledWith(oldSelection.node, 0, true) + expect(codeMapLabelService.addLeafLabel).toHaveBeenCalledWith(newSelection.node, 0, true) + expect(codeMapLabelService.addLeafLabel).toHaveBeenCalledTimes(2) }) it("should keep the label when clicking on the already selected building", () => { @@ -882,8 +900,8 @@ describe("codeMapMouseEventService", () => { codeMapMouseEventService["drawLabelForSelected"](codeMapBuilding) const referenceLabel = codeMapMouseEventService["temporaryLabelForSelectedBuilding"] - codeMapMouseEventService["intersectedBuilding"] = codeMapBuilding + codeMapMouseEventService["intersectedBuilding"] = codeMapBuilding codeMapMouseEventService["onLeftClick"]() expect(codeMapMouseEventService["drawLabelForSelected"]).toHaveBeenCalledWith(codeMapBuilding) From 143b75f0902a68e09533a82525cfc8034ee000fe Mon Sep 17 00:00:00 2001 From: nereboss Date: Thu, 11 Jan 2024 15:14:47 +0100 Subject: [PATCH 6/7] Fix type in changelog #1234 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 979058d73f..789b3d5e77 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/) - Clarify sonar user token question [#3445](https://github.com/MaibornWolff/codecharta/pull/3445) - Changed the `--user` flag to `--user-token` in SonarImporter [#3445](https://github.com/MaibornWolff/codecharta/pull/3445) - Changed the interactive dialog of `modify` to prompt user for single action to perform [#3448](https://github.com/MaibornWolff/codecharta/pull/3448) -- Selected buildings now keep their label untul they are unselected [#3465](https://github.com/MaibornWolff/codecharta/pull/3465) +- Selected buildings now keep their label until they are unselected [#3465](https://github.com/MaibornWolff/codecharta/pull/3465) ### Fixed 🐞 From f478667e3eece1964fe2fb9456f372d932f62061 Mon Sep 17 00:00:00 2001 From: nereboss Date: Fri, 12 Jan 2024 12:45:01 +0100 Subject: [PATCH 7/7] Fix bug with label-line #1234 when clicking on a building and rotating the map after (while the cursor is outside of the map), the line from the building to the label was not drawn to the correct label location --- .../app/codeCharta/ui/codeMap/codeMap.mouseEvent.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/visualization/app/codeCharta/ui/codeMap/codeMap.mouseEvent.service.ts b/visualization/app/codeCharta/ui/codeMap/codeMap.mouseEvent.service.ts index 641b18936f..25b76a16ad 100755 --- a/visualization/app/codeCharta/ui/codeMap/codeMap.mouseEvent.service.ts +++ b/visualization/app/codeCharta/ui/codeMap/codeMap.mouseEvent.service.ts @@ -364,8 +364,8 @@ export class CodeMapMouseEventService implements OnDestroy { } private drawLabelForSelected(codeMapBuilding: CodeMapBuilding) { + this.clearTemporaryLabel() if (this.temporaryLabelForSelectedBuilding !== null) { - this.clearTemporaryLabel() this.codeMapLabelService.clearTemporaryLabel(this.temporaryLabelForSelectedBuilding) } if (!codeMapBuilding.node.isLeaf) {