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

Impossible to add a Geojson layer #367

Open
giboz opened this issue Jun 8, 2020 · 13 comments
Open

Impossible to add a Geojson layer #367

giboz opened this issue Jun 8, 2020 · 13 comments

Comments

@giboz
Copy link

giboz commented Jun 8, 2020

Hello

I've been using your library to try to display a map on a nativescript/angular app and i ran into an issue.

I created my map, no problem there, and then tried to add a Geojson layer on top of that.
I used the addLayer method like described in the documentation and kept running into an error "No map has been loaded".

After multiple checks on my code, i investigated the source code of the lib to find more info about what i did wrong and found that :

The addLayer method calls the addLineLayer because in my case i want to draw lines (https://github.com/Yermo/nativescript-mapbox/blob/master/src/mapbox.android.ts#L3121)

addLineLayers calls the addSource to add the layer source (https://github.com/Yermo/nativescript-mapbox/blob/master/src/mapbox.android.ts#L3262)

As you can see, it calls it with 2 parameters.

this.addSource( sourceId, style.source );

In addSource (https://github.com/Yermo/nativescript-mapbox/blob/master/src/mapbox.android.ts#L2958) we have a 3rd optional parameter called nativeMap.

the first thing the method does is check that parameter and return an error if it's not set.

const theMap = nativeMap; if (!theMap) { reject("No map has been loaded"); return; }

So i have an error. It's logical but unexpected.

Is this an error or am i using a deprecated method to add a layer ?

@giboz
Copy link
Author

giboz commented Jun 8, 2020

After more tests i realized that the "nativeMapView" that is supposed to be the 3rd parameter is actually never defined at all for android. So it's undefined and event by passing it to the addSource it wouldn't work.

Also, in the addSource method, at the end, there is a piece of code :

if (!source) { var ex = "No source to add"; reject(ex); return; } theMap.mapboxMap.addSource(source);

But the variable source is never defined in the method if type === 'geojson'.

If i remove the if that checks for the source to be present, the if that checks for theMap to be present and the last line where it tries to do the addSource on theMap, my geojson is displayed correctly on the map.

So there's no way that addLayer could work with a geojson in it's current form.

I'm guessing i'm not calling the correct method but it's weired that there's code about geojson there that would never work and that it works by removing the strange code.

So i'm confused. I also don't see anything else to achive what i want.

@Yermo
Copy link
Owner

Yermo commented Jun 8, 2020

I have to fix it. My plan is to loop back focus on the backlog of issues and pull requests starting this Friday.

@giboz
Copy link
Author

giboz commented Jun 9, 2020

Nice.

I'll work on something else then and check for updates of the lib later.

Thanks.

@Yermo
Copy link
Owner

Yermo commented Jun 12, 2020

I think I have this working now.

Please checkout branch issue_367_geojsonsource.

Rebuild the plugin and demo-angular app using:

cd src
npm run build.debug
cd ../demo-angular
tns run <platform>

I've added a test-source page to the demo angular app.

Let me know if it works for you.

@giboz
Copy link
Author

giboz commented Jun 14, 2020

Hello,

I built it and tested it on my own app and it works fine.

Thank you for the fix, i'll update my dependencies as soon as it's released.

@trueandperfect
Copy link

how would I install this specific branch of the plugin in my existing project? Not the demo project.

@Yermo
Copy link
Owner

Yermo commented Jul 3, 2020

Checkout the branch.
Build the plugin
Then reference it from your project in package.json

"nativescript-mapbox": "file:../nativescript-mapbox/publish/dist/package",

(Update the path to match your hierarchy obviously)

@mircobabini
Copy link

Hey @Yermo. Will you release a version with this implementation?

@Yermo
Copy link
Owner

Yermo commented Jul 7, 2020

Yes. I plan to do another major pass through the plugin in a few weeks.

@mircobabini
Copy link

mircobabini commented Aug 19, 2020

Hello Yermo. Since I know you are pretty busy but you made a great work finding a path to fix many of the opened issues, I want to give you a help to make tests of the almost-ready updates and try to summarize them all into a working and updated release of this module, with a complete and updated demo sample. Please, get in tour with me to give it a boost 🎉

@Yermo
Copy link
Owner

Yermo commented Aug 19, 2020

@mircobabini that would be awesome. I am almost finished with the project that has occupied all my time, but if you could lend a hand it would be a huge help.

@mircobabini
Copy link

Hey @Yermo just back from holidays. Sure, let me know when you find time to wrap all the most important edits into a new branch or something. Also, let me know if you want to share a roadmap.

@alexonozor
Copy link

Hi @Yermo how do i use this feature on my local app?

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

No branches or pull requests

5 participants