Skip to content

DBC22-6776: frontend implementation for grouping camera icons - #1385

Open
bcgov-brwang wants to merge 2 commits into
mainfrom
feature/DBC22-6776
Open

DBC22-6776: frontend implementation for grouping camera icons#1385
bcgov-brwang wants to merge 2 commits into
mainfrom
feature/DBC22-6776

Conversation

@bcgov-brwang

@bcgov-brwang bcgov-brwang commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

📝 Submitter

🔗 JIRA Ticket


✅ Quality Assurance & Requirements

  • Requirements Met: I have confirmed that all acceptance criteria from the JIRA ticket are fulfilled.
  • Tested desktop in local or dev envs
  • Tested mobile in local or dev envs
  • Ran unit tests locally
  • SonarCloud: I have verified that the SonarCloud analysis is clean/passing for this branch.

⚙️ Configuration & Environment

  • New Env Variables: Does this PR require new environment variables? (Yes/No)

    If yes, please list them here and ensure they are added to secret manager, the .env.example. and the Vault by an STA.

🧪 How to Test (if required)

  1. Deploy to dev.
  2. Open the DriveBC map in the DEV environment and enable the Camera layer.
  3. Verify that camera icons are grouped into numbered cluster icons at appropriate zoom levels.
  4. Verify that cluster icons and individual camera icons transition smoothly when zooming in and out.
  5. Verify that individual camera icons continue to function correctly and that existing camera functionality is unaffected.

🔍 Reviewer Checklist

  • Reviewed code for logic and cleanliness
  • Re-tested desktop/mobile in local or dev envs
  • Verified no new console warnings/errors
  • Confirmed that any new env variables are understood/documented

Comment thread src/frontend/src/Components/map/layers/camerasLayer.js Fixed
Comment thread src/frontend/src/Components/map/handlers/hover.js Fixed
Comment thread src/frontend/src/Components/map/handlers/hover.js Fixed
Comment thread src/frontend/src/Components/map/layers/index.js Fixed
@bcgov-brwang
bcgov-brwang requested a review from ray-oxd July 14, 2026 16:30
@bcgov-brwang bcgov-brwang changed the title Feature/dbc22 6776 DBC22-6776: frontend implementation for grouping camera icons Jul 14, 2026
@bcgov-brwang bcgov-brwang added the ready for review The PR is ready for review label Jul 14, 2026
@bcgov-brwang
bcgov-brwang marked this pull request as ready for review July 14, 2026 16:37
Comment thread src/frontend/src/Components/map/handlers/click.js Fixed
@sonarqubecloud

Copy link
Copy Markdown

draft 2

draft 3

draft 4

draft 5

draft 6

DBC22-6776: fixed broken test cases

removed commented out code

DBC22-6776: switch to fixed pixels to cluster icons

DBC22-6776: removed variables not used

DBC22-6776: updated grouped icons styling

DBC22-6776: updated grouped icon text color

DBC22-6776: updated text font in grouped icon

DBC22-6776: fixed broken test case

DBC22-6776: excluded index.js to void unit test canclling during Analysis progress

DBC22-6776: fixed sonar scan alerts

DBC22-6776: fixed sonar scan alerts part 2

DBC22-6776: single click on group icon takes user to next zoom level
@ray-oxd
ray-oxd force-pushed the feature/DBC22-6776 branch from aae7d43 to c056070 Compare July 27, 2026 22:23
@ray-oxd

ray-oxd commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator
  • Please use a proper commit instead of "draft 1". Squash all the draft commits with a new one.
  • Hover state seems a bit odd (but acceptable?); when you zoom in with a group hovered, all the cams broken out from that group will be in hovered state for a split second. When you zoom out, only the group with the previously hovered group included will be hovered, which I think works fine.

updateReferenceFeature,
setLoadingLayers
setLoadingLayers,
50

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

what is this magic number here?

if (size === 1) {
const cam = features[0];
if (cam.get('clicked')) return cameraStyles.active;
if (cam.get('hovered')) return cameraStyles.hover;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

are we missing unread cam stylings here?

@@ -62,7 +121,11 @@ export function updateCamerasLayer(cameras, layer, setLoadingLayers) {
}, {});

for (const feature of layer.getSource().getFeatures()) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This line might cease to work for grouped cameras but I've not tested this.

This may need to be
layer.getSource().getSource();

or
// Cluster source (what layer.getSource() returns now)
const clusterSource = layer.getSource();

// Real cameras — clustered or not, all of them live here
const cameraSource = clusterSource.getSource();

then use cameraSource instead.


if (!clickedFeature.get('type')) {
clickedFeature = features[0].values_?.features[0];
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

what is this repetition here? In addition, this whole block needs more comments on key operations

}
catch {
feature.values_.features[0].setCameraStyle('static');
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

the try/catchs here are workarounds, we should make sure we always have a feature instead (see comment regarding layer source)

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

Labels

ready for review The PR is ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants