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

Added opt-in Functionality #504

Merged
merged 2 commits into from Oct 5, 2019
Merged

Added opt-in Functionality #504

merged 2 commits into from Oct 5, 2019

Conversation

codeofsumit
Copy link
Contributor

@codeofsumit codeofsumit commented Oct 4, 2019

This PR adds a possibility to initiate leaflet-geoman with an optIn option. Closes #439, Closes #191 .

@codeofsumit codeofsumit merged commit c87218f into develop Oct 5, 2019
@limfinity
Copy link

limfinity commented Oct 10, 2019

Hey @codeofsumit many thanks for tackling this issue so fast! 💯

I tried it out and looked at the changes and would have some suggestions. While trying it out I saw, that when you do not pass any optIn value, the pm object is missing on existing layers.

Also when I add the optIn value I would have expected that layers created with geoman itself would have the pmIgnore flag set by default to false. Is there a way I could do that myself? Because when I create a marker in this way for example, it can't even create them because it is trying to call enable on the undefined pm object. Which makes my application crash.

In general the whole wording and combination of optIn and pmIgnore was hard to understand and makes the code much more verbose than I thought.

@codeofsumit
Copy link
Contributor Author

@limfinity you're right. Do you mind opening up a new issue explaining this again (or just copy paste)? I will tackle it asap

codeofsumit added a commit that referenced this pull request Oct 15, 2019
* removed console.log statement. Fixes #498

* Switch to Leaflet-Geoman (#502) (patch)

* changed names, links and filenames

* keep download stats for leaflet.pm

* corrected es6 import statement

* updated badges

* changed all package name references

* changed npm org name to geoman-io

* updated npm badge to new org

* corrected file names in CDN

* 2.2.1

* fixed install instructions

* Added migration instructions

* minor heading change

* fixed travis badge

* Upgraded to Webpack 4 & fix security issue (#500) (patch)

* upgraded webpack 4 and first fixes
* switched to MiniCssExtractPlugin

* Handles snapping for added shapes during draw. Fixes #477 (patch)

* Finishing a Rectangle now respects snapped marker positions. Fixes #448 (patch)

* imrpoved description to test on a map

* Added opt-in Functionality (#504) (minor)

* added opt-in functionality

* corrected optIn value

* Style changes during edit mode are now properly preserved (#506). Fixes #470 (patch)

* trying to reproduce the error

* fixed color caching problem

* cleaned up tests

* renamed test

* 2.3.0

* Create FUNDING.yml

* Update FUNDING.yml

* Update FUNDING.yml

* Update FUNDING.yml
@codeofsumit codeofsumit deleted the optIn-option branch November 7, 2020 19:38
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.

Opt-in functionality Is it possible to set pmIgnore=true by default?
2 participants