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

Unable to addSource/addLayer in Android #356

Open
fhissink opened this issue Apr 29, 2020 · 16 comments
Open

Unable to addSource/addLayer in Android #356

fhissink opened this issue Apr 29, 2020 · 16 comments

Comments

@fhissink
Copy link

Hi,

First of all thanks a lot for creating and maintaining this awesome NativeScript plugin.

I've been running into an issue and I was wondering if there maybe is a workaround for this.
I'm trying to add a layer with some in memory features to the map. I've been using addSource and addLayer for this. However the moment I call addSource I get an error No map has been loaded. The same goes for addLayer (for example by calling the addTestCircle in the demo-angular app).

Some digging in the code shows that this is because the nativeMapView is undefined. And I also saw that you've added a comment that this should be fixed:
* @todo FIXME: this.nativeMapView is unused and never actually set to anything.

My question is, is there a workaround for this? Or are there any plans to fix this in the near future? If not could you give me some pointers on how to fix this (if possible)? I could then try to fix it and make a PR for this.

Thanks!

@Yermo
Copy link
Owner

Yermo commented Apr 29, 2020

Are you using the NPM module or have you cloned the repository?

@fhissink
Copy link
Author

I tried both, but both have the same result

Yermo added a commit that referenced this issue Apr 29, 2020
@Yermo
Copy link
Owner

Yermo commented Apr 29, 2020

please checkout branch issue_356 which I just pushed, test it, and let me know if it works for you.

@Yermo
Copy link
Owner

Yermo commented Apr 29, 2020

(And thank you very much for the report.)

@fhissink
Copy link
Author

Yes, that works perfectly, thanks for the quick fix!:)

I did see that removeSource still has a reference to mapboxMap theMap.mapboxMap.removeSource(id);

Another question, is it correct that the addSource now only supports a single Feature? And not a FeatureCollection?

@Yermo
Copy link
Owner

Yermo commented Apr 30, 2020

@fhissink Thanks for testing that.

I'll have to fix removeSource(). Good catch.

I haven't tested FeatureCollections but looking at the code it does look like it's going to need a little work.

https://docs.mapbox.com/android/api/mapbox-java/libjava-geojson/3.0.1/com/mapbox/geojson/package-summary.html

@fhissink
Copy link
Author

fhissink commented Apr 30, 2020

I've been fiddling around a bit and got it working. I can commit these changes to a new branch so you can have a look? Only thing is that I don't have any Apple devices so I'm not able to test it on iOS (therefor I also didn't implement it there).

In the meantime I was also a bit busy with data-driven styling. I'd like to have that for the CirceLayer, I used this sample as a start:
https://docs.mapbox.com/android/maps/examples/style-circles-categorically/
and then mainly this part:
circleColor( match(get("ethnicity"), rgb(0, 0, 0), stop("white", rgb(251, 176, 59)), stop("Black", rgb(34, 59, 83)), stop("Hispanic", rgb(229, 94, 94)), stop("Asian", rgb(59, 178, 208)), stop("Other", rgb(204, 204, 204)))));

I managed to create all the expressions (get, rgb, stops, etc.) but when calling match I got an error:
Error: java.lang.Exception: Failed resolving method match on class com.mapbox.mapboxsdk.style.expressions.Expression

probably the call signature is incorrect which looks like this now:
const match = com.mapbox.mapboxsdk.style.expressions.Expression.match( getExpression, defaultColorExpression, ...stops );

but according to the documentation it should work: https://docs.mapbox.com/android/api/map-sdk/8.6.0/com/mapbox/mapboxsdk/style/expressions/Expression.html#match-com.mapbox.mapboxsdk.style.expressions.Expression-com.mapbox.mapboxsdk.style.expressions.Expression-com.mapbox.mapboxsdk.style.expressions.Expression.Stop...-

Does this ring any bells with you?

@Yermo
Copy link
Owner

Yermo commented Apr 30, 2020

Fell free to submit a pull request and I'll take a look at it.

Getting the layers to work in the limited capacity they do now was a huge hurdle. It's just not intuitive at all. What I want to do with the layers is see if there's a way to parse the raw style specification directly instead of parsing it myself and building it up as I'm doing at the moment. It looks like there are some methods to do that which would hopefully mean we can sidestep building the expression in Java/ObjC and instead just use the Gl JS style spec directly. That would avoid a whole host of issues. It's going to be a while yet before I jump into that work. (I desperately need it for my project so I'm hoping I can get it to work.)

@fhissink
Copy link
Author

fhissink commented May 1, 2020

I managed to get the expressions working with json as an input:

if (style.paint['circle-color']) {
            if (Array.isArray(style.paint['circle-color'])) {
              const jsonArray = new com.google.gson.JsonParser().parse(JSON.stringify(style.paint['circle-color'])).getAsJsonArray();
              const expression = com.mapbox.mapboxsdk.style.expressions.Expression.Converter.convert(jsonArray);
              circleProperties.push(com.mapbox.mapboxsdk.style.layers.PropertyFactory.circleColor(expression));
            } else {
              circleProperties.push(com.mapbox.mapboxsdk.style.layers.PropertyFactory.circleColor(style.paint['circle-color']));
            }
          }

then I can use expressions formatted like this:

'circle-color': [
	'match',
	['get', 'status'],
	'visited',
	'#238E6B', // green
	'visiting',
	'#2A5A8B', // blue
	'#CCC' // default
]

Which results in circles with different colors on the map.
Right now I've only got it working for the circle-color, but I guess it would work for the rest as well.

@Yermo
Copy link
Owner

Yermo commented May 1, 2020

ok, that's awesome.

@fhissink
Copy link
Author

fhissink commented May 4, 2020

I've created a draft PR. I'm not completely there yet but I was wondering if you could already have a look and see if you think this would work or if you foresee any problems by the changes I've made.

There's some extra info in the PR itself. Sorry for the whitespace changes :/
You can check the changes here without the whitespaces:
https://github.com/Yermo/nativescript-mapbox/pull/359/commits/0e1a83ff1cee194c50f8757bb1c4f4f9b2f18b3b

@zghotlawala
Copy link

zghotlawala commented Jun 3, 2020

Hey,
Was wondering how soon you can merged the issue_356 changes to master? As we are not able to draw circles and lines using master branch

@Yermo
Copy link
Owner

Yermo commented Jun 3, 2020

I apologize. I've been busy with other projects and hope to focus on this soon. Before it can be included, the iOS side needs to be updated to match. That's where most of the work is. If someone wants to tackle that it would be of great help and would speed up getting the patch accepted.

@mircobabini
Copy link

I apologize. I've been busy with other projects and hope to focus on this soon. Before it can be included, the iOS side needs to be updated to match. That's where most of the work is. If someone wants to tackle that it would be of great help and would speed up getting the patch accepted.

Forget for a minute the changes from @fhissink and focus on the real issue here: on android nothing can be drawn on the map. You already fixed it with that commit.

Please, release that one.

@vananden
Copy link

We also need to show an overlay on our map to show areas where pollution is so it can be monitored by a multinational organization. We are unable to do this with either a vector or a bitmap at the moment. This only needs to work on Android for us at the moment.

We would gladly support the resolution in any way we can. This feature is supposed to go live this week.

@andipahlevy
Copy link

plz help i get an error.
npm ERR! Could not install from "github.com:Yermo\nativescript-mapbox.git" as it does not contain a package.json file.

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

6 participants