Skip to content

working on points lines and polygons#3

Open
finkegabriel wants to merge 3 commits into
mainfrom
trail-mode
Open

working on points lines and polygons#3
finkegabriel wants to merge 3 commits into
mainfrom
trail-mode

Conversation

@finkegabriel

@finkegabriel finkegabriel commented Apr 23, 2026

Copy link
Copy Markdown
Owner

This is a WIP.


Note

Medium Risk
Moderate risk because it changes core map editing interactions (click/dblclick/keyboard) and geometry handling across LineString, Point, and Polygon, which could introduce edge-case regressions in selection, vertex indexing, and deletion.

Overview
Extends the trail editor to handle Point and Polygon geometries in addition to LineString, including selection, vertex rendering, and GeoJSON export.

Adds polygon/connection workflows during trail creation: clicking near the first vertex converts an in-progress line into a filled polygon, and clicking near any existing vertex snaps/extends the current line to that vertex.

Introduces new completion and cleanup paths: double-click/p/Enter can finalize a single-vertex trail as a point, Backspace deletion now supports point/polygon semantics, and entering trail mode now clears prior selection/branch state to start fresh.

Reviewed by Cursor Bugbot for commit d05c436. Bugbot is set up for automated code reviews on this repo. Configure here.

@finkegabriel finkegabriel self-assigned this Apr 23, 2026
@netlify

netlify Bot commented Apr 23, 2026

Copy link
Copy Markdown

Deploy Preview for mapspace ready!

Name Link
🔨 Latest commit a87f8d8
🔍 Latest deploy log https://app.netlify.com/projects/mapspace/deploys/69ea7f60cd2bba0008222a31
😎 Deploy Preview https://deploy-preview-3--mapspace.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 6 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit d05c436. Configure here.

Comment thread index.js
const clickedFeature = map.forEachFeatureAtPixel(evt.pixel, f => f);

if (clickedFeature && clickedFeature.getGeometry().getType() === 'LineString') {
if (clickedFeature && (clickedFeature.getGeometry().getType() === 'LineString' || clickedFeature.getGeometry().getType() === 'Polygon' || clickedFeature.getGeometry().getType() === 'Point')) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Point features become invisible with stroke-only styles

High Severity

When any feature is clicked, the singleclick handler applies defaultStyle or selectedStyle to all features. Both styles only define a stroke, with no image property. In OpenLayers, Point geometries require an image style (like CircleStyle) to render — stroke alone has no effect. After clicking on any feature, all Point features in the vector source become invisible. The same issue occurs in the Replace trail handler where defaultStyle is applied to all features.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d05c436. Configure here.

Comment thread index.js
updateVertices();
setGlobalSelected(vertexMap.findIndex(e => e.feature === selectedFeature && e.index === coords.length - 1));
return;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Click handler corrupts Polygon/Point geometry coordinates

High Severity

The "Create trail" context menu action now enters trail-editing mode for Polygon and Point features, but the click handler's coordinate manipulation code (the isNearVertex connection and the branching/appending logic) assumes the geometry is always a LineString. For a Polygon, getCoordinates() returns nested ring arrays, and for a Point, it returns [x, y]. Pushing a coordinate to these structures and calling setCoordinates produces malformed geometries and likely crashes.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d05c436. Configure here.

Comment thread index.js
originalCoords = [...selectedFeature.getGeometry().getCoordinates()[0]];
} else if (geomType === 'Point') {
originalCoords = [selectedFeature.getGeometry().getCoordinates()];
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Deselect restores wrong coordinate format for Polygon/Point

Medium Severity

The originalCoords stored for Polygon (flat ring [A, B, C]) and Point ([[x, y]]) is in a format incompatible with what setCoordinates expects. The Deselect handler calls setCoordinates(originalCoords) directly on the geometry. A Polygon expects [[A, B, C]] (nested), and a Point expects [x, y] (flat). This corrupts the geometry when the user deselects after entering "Create trail" on a Polygon or Point.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d05c436. Configure here.

Comment thread index.js
// Keep as LineString but finish editing
selectedFeature.setStyle(defaultStyle);
}
// For 3+ points, keep as LineString

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Enter key omits style reset for 3+ point trails

Low Severity

The Enter key handler sets the style for 1-point trails (blue circle) and 2-point trails (defaultStyle), but omits any style change for trails with 3 or more points. These features keep the yellow selectedStyle after trail creation mode exits, leaving them visually in a "selected" state even though editing has ended. This is inconsistent with the 2-point behavior.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d05c436. Configure here.

Comment thread index.js
fill: new Fill({ color: 'blue' }),
stroke: new Stroke({ color: 'white', width: 2 }),
}),
}));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Point conversion logic duplicated across four handlers

Low Severity

The logic to convert a single-point LineString to a Point geometry with a blue CircleStyle is identically repeated in the dblclick handler, the p key handler, the Enter key handler, and the Replace trail handler. Extracting this into a shared helper function would reduce maintenance burden and ensure consistent behavior if the style or conversion logic needs to change.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d05c436. Configure here.

Comment thread index.js
document.body.style.cursor = 'auto';
updateVertices();
updateTextarea();
evt.preventDefault(); // Prevent default double-click zoom

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Double-click zoom not actually prevented by preventDefault

Medium Severity

evt.preventDefault() does not prevent OpenLayers' DoubleClickZoom interaction from firing. In OpenLayers 10.x, the zoom interaction checks whether propagation was stopped, not whether default was prevented. The correct call is evt.stopPropagation(). As written, double-clicking to convert a single-point trail to a Point will simultaneously zoom the map in, which is disorienting and contrary to the comment's stated intent.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d05c436. Configure here.

@cursor

cursor Bot commented Apr 23, 2026

Copy link
Copy Markdown

You have used all of your free Bugbot PR reviews.

To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

@cursor

cursor Bot commented Apr 23, 2026

Copy link
Copy Markdown

You have used all of your free Bugbot PR reviews.

To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

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.

1 participant