Skip to content

Add contextmenu for map and markers#202

Draft
jules43 wants to merge 20 commits into
SupraGamesCommunity:mainfrom
jules43:contextmenu
Draft

Add contextmenu for map and markers#202
jules43 wants to merge 20 commits into
SupraGamesCommunity:mainfrom
jules43:contextmenu

Conversation

@jules43
Copy link
Copy Markdown
Collaborator

@jules43 jules43 commented May 15, 2026

Initially main map has 'add map pin' and markers can toggleFound and move player position - which updates the delta altitude values.

I created a fork of leaflet.contextmenu and updated a bunch of stuff, then integrated that into the project.

@jules43 jules43 requested a review from coverprice May 15, 2026 17:17
Comment thread scripts/rendericons.js
Comment on lines 232 to +235
const pinIconSize = size * 0.5; // Size to draw the FA icon on a pin marker
const pinCentreYOffset = pinIconSize * -0.25; // Y offset from centre of icon to centre for a pin
const ptIconSize = size * 0.7; // Size to draw the FA icon on a point marker
const ptCentreYOffset = 0; // Y offset from centre of icon to centre for a pin
const ptCentreYOffset = pinIconSize * 0; // Y offset from centre of icon to centre for a pin
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.

"Size" here is unit-less, which is a bit confusing for me. It seems to be a proportion of the original size, and not a specific size like "10 pixels". So for clarity I'd suggest renaming it to something like pinIconScalingFactor ?

In the line you modified, it's not clear why you're multiplying that scaling factor by another value to get a specific number of ... pixels? pts? It's not clear. But anyway, the code is converting a proportion to concrete units and it's not clear to me what's going on here.

Comment thread scripts/rendericons.js
Comment on lines +274 to 276
if(iconName == 'face-smile' || iconName == 'player.red'){
console.log('playerstart');
}
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.

Nit: Use console.debug instead of console.log, to better communicate the intent.

Comment on lines +131 to +133

DomEvent.disableClickPropagation(this._container);

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 is a customization of a 3rd-party vendored import, and so it's probably wise to earmark this as such (both here and at the top of the file) so that we know to port these forward in case we end up upgrading this library in the future.

Comment on lines +200 to +201
L.DomEvent.disableClickPropagation(container);

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.

Ditto to my comment about library customizations.

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