Skip to content

Conversation

@geertplaisier
Copy link
Collaborator

@geertplaisier geertplaisier commented Nov 21, 2025

HTM-1538 Powered by Pull Request Badge

Copilot finished reviewing on behalf of geertplaisier November 21, 2025 11:42
@github-actions
Copy link

github-actions bot commented Nov 21, 2025

Test Results

  1 files  ± 0  214 suites   - 1   8m 47s ⏱️ +10s
528 tests  -  4  528 ✅  -  4  0 💤 ±0  0 ❌ ±0 
608 runs   - 10  608 ✅  - 10  0 💤 ±0  0 ❌ ±0 

Results for commit 98b4f7a. ± Comparison against base commit 328e55f.

This pull request removes 5 and adds 1 tests. Note that renamed tests count towards both.
ToolbarReducer activates tool ‑ ToolbarReducer activates tool
ToolbarReducer deactivates tool ‑ ToolbarReducer deactivates tool
ToolbarReducer deregister existing tool ‑ ToolbarReducer deregister existing tool
ToolbarReducer re-register existing tool ‑ ToolbarReducer re-register existing tool
ToolbarReducer register tool ‑ ToolbarReducer register tool
Switch3dComponent should render when (measure) tool is enabled ‑ Switch3dComponent should render when (measure) tool is enabled

♻️ This comment has been updated with latest results.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the toolbar state management system by removing the dedicated NgRx toolbar state and instead using the ToolManager for tracking tool activation status. The main architectural change involves adding an owner property to tools to identify which component owns them, and providing new observable streams and methods on MapService to check tool status.

Key Changes:

  • Replaced toolbar NgRx state (actions, selectors, reducer, effects) with ToolManager-based state management
  • Added owner property to ToolConfigModel to track tool ownership by component
  • Introduced getToolStatusChanged$() and someToolsEnabled$() methods on MapService
  • Migrated all toolbar components to use MapService methods instead of NgRx actions/selectors
  • Updated components to use Angular signals where appropriate

Reviewed Changes

Copilot reviewed 47 out of 49 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
projects/map/src/lib/openlayers-map/open-layers-tool-manager.ts Enhanced to store tool owner information and emit status changes via observable; added debug logging capabilities
projects/map/src/lib/models/tools/tool-config.model.ts Added required owner property to track which component owns a tool
projects/map/src/lib/models/tools-status.model.ts New model defining tool status structure with owner information
projects/map/src/lib/models/tool-manager.model.ts Updated interface to make toolId optional and renamed method to getToolStatusChanged$
projects/map/src/lib/map-service/map.service.ts Added new methods for tool status management: getToolStatusChanged$, someToolsEnabled$, enableTool, disableTool
projects/core/src/lib/test-helpers/map-service.mock.spec.ts Added createMapServiceMockWithDrawingTools helper and updated mock to include new MapService methods
projects/core/src/lib/map/services/drawing.service.ts Updated to use new getToolStatusChanged$ method and added owner to tool configurations
projects/core/src/lib/map/index.ts Removed export of deleted MapDrawingButtonsComponent
projects/core/src/lib/map/application-map.module.ts Removed MapDrawingButtonsComponent from module declarations
projects/core/src/lib/components/toolbar/toolbar.module.ts Removed toolbar state imports (StoreModule, EffectsModule, reducer, effects)
projects/core/src/lib/components/toolbar/switch3d/switch3d.component.ts Migrated from toolbar selectors to use MapService.someToolsEnabled$
projects/core/src/lib/components/toolbar/switch3d/switch3d.component.spec.ts Updated test to remove toolbar selector dependencies
projects/core/src/lib/components/toolbar/streetview/streetview.component.ts Migrated to MapService and Angular signals; removed toolbar state dependencies
projects/core/src/lib/components/toolbar/streetview/streetview.component.spec.ts Updated test to remove toolbar state mock setup
projects/core/src/lib/components/toolbar/streetview/streetview.component.html Updated to use signal-based template syntax
projects/core/src/lib/components/toolbar/state/* Deleted all toolbar state files (state, selectors, reducer, effects, actions)
projects/core/src/lib/components/toolbar/models/* Deleted ToolbarToolModel and ToolbarComponentEnum
projects/core/src/lib/components/toolbar/measure/measure.component.ts Migrated to MapService and signals; removed toolbar state dependencies
projects/core/src/lib/components/toolbar/measure/measure.component.spec.ts Updated test to remove toolbar selector dependencies
projects/core/src/lib/components/toolbar/measure/measure.component.html Updated to use signal-based template syntax
projects/core/src/lib/components/toolbar/coordinate-link-window/coordinate-link-window.component.ts Migrated to MapService with signals
projects/core/src/lib/components/toolbar/coordinate-link-window/coordinate-link-window.component.spec.ts Updated test to remove toolbar state dependencies
projects/core/src/lib/components/toolbar/coordinate-link-window/coordinate-link-window.component.html Updated to use signal-based template syntax
projects/core/src/lib/components/toolbar/clicked-coordinates/clicked-coordinates.component.ts Migrated to MapService and signals
projects/core/src/lib/components/toolbar/clicked-coordinates/clicked-coordinates.component.spec.ts Updated test to remove toolbar state dependencies
projects/core/src/lib/components/toolbar/clicked-coordinates/clicked-coordinates.component.html Updated to use signal-based template syntax
projects/core/src/lib/components/toolbar/scale-bar/scale-bar.component.ts Added owner property to tool configuration
projects/core/src/lib/components/toolbar/mouse-coordinates/mouse-coordinates.component.ts Added owner property to tool configuration
projects/core/src/lib/components/filter/spatial-filter-form-draw-geometries/map-drawing-buttons/* Moved MapDrawingButtonsComponent to filter module with updated imports
projects/core/src/lib/components/filter/filter-component.module.ts Added MapDrawingButtonsComponent to declarations
projects/core/src/lib/components/feature-info/feature-info/feature-info.component.ts Updated to use MapService and added tool activation logic based on dialog visibility
projects/core/src/lib/components/feature-info/feature-info/feature-info.component.spec.ts Updated test to remove toolbar state dependencies
projects/core/src/lib/components/feature-info/feature-info-dialog/feature-info-dialog.component.ts Added distinctUntilChanged import
projects/core/src/lib/components/edit/state/edit.effects.ts Removed toolbar-related effect
projects/core/src/lib/components/edit/services/edit-map-tool.service.ts Removed toolbar state dependencies and added owner property to tools
projects/core/src/lib/components/edit/edit/edit.component.ts Added subscription to tool status and removed toolbar action dispatches
projects/core/src/lib/components/edit/edit-dialog/edit-dialog.component.ts Removed toolbar action dispatches
projects/core/src/lib/components/drawing/drawing/drawing.component.spec.ts Updated to use new test helper for drawing tools

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 46 out of 48 changed files in this pull request and generated 4 comments.

@codecov
Copy link

codecov bot commented Nov 21, 2025

❌ 2 Tests Failed:

Tests completed Failed Passed Skipped
529 2 527 0
View the top 2 failed test(s) by shortest run time
Switch3dComponent should render
Stack Traces | 0.244s run time
TestingLibraryElementError: Unable to find a label with the text of: Switch to 3D

Ignored nodes: comments, script, style
<body>
  <div
    id="root0"
    ng-version="20.1.4"
  />
</body>
    at Object.getElementError (.../tailormap-viewer/tailormap-viewer/node_modules/@.../dom/dist/config.js:37:19)
    at getAllByLabelText (.../tailormap-viewer/tailormap-viewer/node_modules/@.../dist/queries/label-text.js:111:38)
    at .../tailormap-viewer/tailormap-viewer/node_modules/@.../dom/dist/query-helpers.js:52:17
    at .../tailormap-viewer/tailormap-viewer/node_modules/@.../dom/dist/query-helpers.js:95:19
    at .../toolbar/switch3d/switch3d.component.spec.ts:40:19
    at processTicksAndRejections (node:internal/process/task_queues:105:5)
Switch3dComponent toggle between 2D and 3D
Stack Traces | 1.04s run time
Error: Unable to find role="button"

Ignored nodes: comments, script, style
<body>
  <div
    id="root2"
    ng-version="20.1.4"
  />
</body>
    at waitForWrapper (.../tailormap-viewer/tailormap-viewer/node_modules/@.../dom/dist/wait-for.js:163:27)
    at waitFor$1 (.../tailormap-viewer/tailormap-viewer/node_modules/@.../angular/fesm2022/testing-library-angular.mjs:398:18)
    at Generator.next (<anonymous>)
    at .../tailormap-viewer/tailormap-viewer/node_modules/@.../angular/fesm2022/testing-library-angular.mjs:71:61
    at new ZoneAwarePromise (.../tailormap-viewer/tailormap-viewer/node_modules/zone.js/bundles/zone.umd.js:2580:29)
    at __async (.../tailormap-viewer/tailormap-viewer/node_modules/@.../angular/fesm2022/testing-library-angular.mjs:55:10)
    at waitForWrapper (.../tailormap-viewer/tailormap-viewer/node_modules/@.../angular/fesm2022/testing-library-angular.mjs:390:65)
    at .../tailormap-viewer/tailormap-viewer/node_modules/@.../angular/fesm2022/testing-library-angular.mjs:473:30
    at Generator.next (<anonymous>)
    at .../tailormap-viewer/tailormap-viewer/node_modules/@.../angular/fesm2022/testing-library-angular.mjs:71:61
    at new ZoneAwarePromise (.../tailormap-viewer/tailormap-viewer/node_modules/zone.js/bundles/zone.umd.js:2580:29)
    at __async (.../tailormap-viewer/tailormap-viewer/node_modules/@.../angular/fesm2022/testing-library-angular.mjs:55:10)
    at Object.newQueries.<computed> [as findByRole] (.../tailormap-viewer/tailormap-viewer/node_modules/@.../angular/fesm2022/testing-library-angular.mjs:470:58)
    at .../toolbar/switch3d/switch3d.component.spec.ts:50:40
    at processTicksAndRejections (node:internal/process/task_queues:105:5)

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants