Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

task/WG-83: Handle Overlapping Point Clouds #181

Merged
merged 9 commits into from
Dec 18, 2023

Conversation

tjgrafft
Copy link
Contributor

Overview:

Putting a point marker with the polygon/point cloud. This would make it be able to be seen if zoomed out and would allow users to select different overlapping point clouds. Adding those point markers to the marker cluster, as well.

PR Status:

  • Ready.
  • Work in Progress.
  • Hold.

Related Jira tickets:

Summary of Changes:

  • Added a custom marker to the top-left corner of point clouds, so that users can click the icon and get the correct modal to pop up in cases where there were overlapping point clouds in the same location.
  • Added the ability to attach feature data to the marker options prop (so that the modals open when the markers are clicked, which is consistent/same behavior as clicking inside the polygon area)
  • Added function to calculate the top-left corner of a layer/feature (this could be refactored or added onto later for other corners)
  • Added point cloud markers to the marker cluster group, so that all assets would be included in clusters.
  • Added conditionals to the featureClickHandler function to check whether clicked object was a marker or layer.

Testing Steps:

  1. Go to localhost:4200 and find a project where there are overlapping point clouds present. (If you need a project, Nathan or I can share one with you)
  2. Go to the overlapping point clouds on the map. Check that the new point-cloud icon is present in the top-left corner of the polygons.
  3. Click the point-cloud icons and make sure the correct modals open for that point-cloud asset.
  4. Zoom out and make sure you see cluster markers and corresponding numbers related to how many assets are in that area--including the overlapping point clouds.
  5. Compare with images below

UI Photos:

New Point Cloud Markers
Point_cloud_icons


Cluster markers with point-cloud assets included
cluster_marker2
cluster_marker1

Notes:

Copy link
Contributor

@taoteg taoteg left a comment

Choose a reason for hiding this comment

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

LGTM! Ran it locally and worked as advertised.

Note: to test against the Staging env, you will need to disable lines 41-66 to match this commit in feature/questionnaire (until that branch gets merged):

da0f40b#diff-7bedcb7bd6c40fef89e5075610ff1cc53e7d0f64a3fb6f06632d56caa966f499

@@ -350,6 +350,15 @@ export class MapComponent implements OnInit, OnDestroy {
let feat: LayerGroup;
if (d.geometry.type === 'Polygon' && d.properties.style) {
feat = L.geoJSON(d, { style: d.properties.style });
} else if (d.featureType() === 'point_cloud') {
feat = L.geoJSON(d, { style: d.properties.style });
this.features.addLayer(feat);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
this.features.addLayer(feat);

This should be deleted.

as later on in the code we add it here

        if (d.geometry.type === 'Point') {
          markers.addLayer(feat);
        } else {
          this.features.addLayer(feat);
        }

Copy link
Collaborator

@nathanfranklin nathanfranklin left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Just note one small change that is required to not add the polygon twice.

@tjgrafft tjgrafft merged commit 6a34d5f into master Dec 18, 2023
5 checks passed
@tjgrafft tjgrafft deleted the task/WG-83-overlapping-point-clouds branch December 18, 2023 22:46
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.

3 participants