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

Allow duplicate marker type on a portal #321

Merged
merged 6 commits into from
Jan 14, 2022
Merged

Allow duplicate marker type on a portal #321

merged 6 commits into from
Jan 14, 2022

Conversation

bgillock
Copy link
Contributor

Fixes #319 by removing checks for existing marker of same type in type select list and when adding a marker.

@le-jeu
Copy link
Member

le-jeu commented Jan 13, 2022

Other related parts:

const markers = operation.getPortalMarkers(portal);
for (const k of WasabeeMarker.markerTypes) {
const o = L.DomUtil.create("option", null, this._type);
o.value = k;
o.textContent = wX(k);
if (markers.has(k) && k != this.options.marker.type) o.disabled = true;
}
this._type.value = this.options.marker.type;

This function is bound to disappear

getPortalMarkers(portal: WasabeePortal) {
const markers = new Map<string, WasabeeMarker>();
if (!portal) return markers;
for (const m of this.markers) {
if (m.portalId == portal.id) {
markers.set(m.type, m);
}
}
return markers;
}

But that needs work in the applyChanges method

@bgillock
Copy link
Contributor Author

We can keep getPortalMarkers if we use the m.ID as the key instead of the type. Then make the appropriate changes in applyChanges.

@le-jeu
Copy link
Member

le-jeu commented Jan 13, 2022

that won't be really useful for the applyChanges function, getPortalMakers is there for two reasons:

  • support old OP (will be removed), since those OP have only one marker type per portal, it's fine to use current code
  • avoid marker edit leading to duplicate markers with same type, this can be removed safely (summary.edition.duplicate)

@bgillock
Copy link
Contributor Author

With your help, I think I addressed all of the places where a unique marker type was assumed and enforced.

@le-jeu
Copy link
Member

le-jeu commented Jan 14, 2022

No, if you change getPortalMakers you need to udate every uses. But this function can be keep as it was, since it's valid where it's used in applyChanges (1.).
It's just that the mechanism that removes duplicates marker due to marker edit can be removed (2.)

@le-jeu le-jeu added this to the 0.21 milestone Jan 14, 2022
@le-jeu le-jeu added the enhancement New feature or request label Jan 14, 2022
@le-jeu le-jeu merged commit 13f82e6 into wasabee-project:dev Jan 14, 2022
@bgillock bgillock deleted the DupMarkerTypes branch January 17, 2022 18:31
@le-jeu le-jeu mentioned this pull request Mar 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants