quentinlampin / ngx-openlayers

Angular2+ components for Openlayers 4.x
Mozilla Public License 2.0
137 stars 98 forks source link

bing source #13

Closed FallenRiteMonk closed 7 years ago

FallenRiteMonk commented 7 years ago

Just wanted to ask if the bing source works for you, as I'm trying to do something similar to what you are doing there with the key string. But I always get the error: error_handler.js:53Error: No provider for String! and leaving it away and passing this.key to the super constructor results in an error about not being able to call this before calling the super constructor, which is totally clear. So if it works for you, please let me know what I'm doing wrong so that i can fix it.

Thanks

quentin-ol commented 7 years ago

Hey,

I will look into this later this afternoon (in a few hours). Thanks for spotting the issue and sorry that things are not as polished as it should. Quentin

From: FallenRiteMonk notifications@github.com Reply-To: quentin-ol/angular2-openlayers reply@reply.github.com Date: Tuesday, 6 December 2016 at 11:51 To: quentin-ol/angular2-openlayers angular2-openlayers@noreply.github.com Subject: [quentin-ol/angular2-openlayers] bing source (#13)

Just wanted to ask if the bing source works for you, as I'm trying to do something similar to what you are doing there with the key string. But I always get the error: error_handler.js:53Error: No provider for String! and leaving it away and passing this.key to the super constructor results in an error about not beeing able to call this before calling the super constructor, which is totally clear. So if it works for you, please let me know what I'm doing wrong so that i can fix it.

Thnaks

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/quentin-ol/angular2-openlayers/issues/13, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AOYyYrVIOMN3J22JjLi2Incx_LasPrZAks5rFT4zgaJpZM4LFRTX.


Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration, Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law; they should not be distributed, used or copied without authorisation. If you have received this email in error, please notify the sender and delete this message and its attachments. As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified. Thank you.

quentin-ol commented 7 years ago

Quick update: found the reason why this isn't working. Apart from the silly mistake that I made, i.e. passing the key using dependency injection, the problem stems from the way the BingMap source component is initialised: it's created before its inputs are resolved. It means that the ol.source.BingMaps instance is created without an API key, hence the issue.

I'm working on a rewrite of the source components to tackle this issue: the core idea is that the component itself is a container to the ol instance and that the ol instance is created in the ngOnInit callback.

I will push once I've checked I'm not breaking anything (including PR #15 )

quentin-ol commented 7 years ago

Quick update 2: the dev branch has a working solution for the BingMap source. Here is how to use it:

<aol-map>
  <aol-layer-tile>
    <aol-source-bingmaps [key]="'your-api-key'" [imagerySet]="'Aerial'">
    </aol-source-bingmaps>
  </aol-layer-tile>
</aol-map>

I'm still reviewing the changes before the merge. @FallenRiteMonk, if by change you have time to test the dev branch as well, that'd be great.

FallenRiteMonk commented 7 years ago

@quentin-ol I'd like to test it for you, but could you please give me a bingMap key so I dont have to create one just for this test?

quentin-ol commented 7 years ago

@FallenRiteMonk I know that there is one in the Openlayers examples : https://github.com/openlayers/ol3/blob/master/examples/bing-maps.js

I myself would not use it for any other purpose than testing (one-time) a feature related to OpenLayers as it's the property of the Openlayers project. You might want to ask them specifically about it.

quentin-ol commented 7 years ago

@FallenRiteMonk Just in case you need it :

FallenRiteMonk commented 7 years ago

@quentin-ol LGTM

FallenRiteMonk commented 7 years ago

A quick question: for future contributions do you want me to continue on the dev branch or should I continue like until now on the master branch?

quentin-ol commented 7 years ago

I believe the dev branch should be merged into master. The only reason it exists is to test PRs without breaking everything for those who rely on the lib. If that's ok with everyone, I will do the merge.

FallenRiteMonk commented 7 years ago

I understand that, just thought you might want to keep such a workflow for the future to! Like for example all the contributions are merged into dev and on every release dev is merged into master and taged or something like that.

I don't mind how you then do it, I just thougt I'll ask before doing something and then having to change alot of things due to such a decision!

quentin-ol commented 7 years ago

oh, sorry! Re-reading my comment made me realise I wasn't clear! The plan is to keep a dev branch for testing purposes. I was suggesting that we could merge the two of them now that the PR has been accepted and that its features and bugfixes have been tested. Obviously, next non-trivial changes will land in dev. Thanks again for the PRs, much much appreciated! 👍

FallenRiteMonk commented 7 years ago

Ok sounds good!

No problem, happy to help, and I'm still also helping myself. Just hope to see this soon on npm!