-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Unnecessary Bing Maps warning #1165
Comments
Is this with the geocoding widget merged in? Because that uses the Bing API, too. |
I'm pretty sure @kring is correct. I took a quick look at the code to make sure we weren't constructing a Bing provider and throwing it away, but it doesn't look like it. |
You guys are right. I'm OK with just closing this or making the console warning more detailed since it is not obvious that the geocoder uses the Bing API. I knew it did and still didn't make the connection. |
Off-topic, but the sample code shown isn't ideal: It constructs a new ViewModel to be the "selected" one, rather than using a reference to the one that was constructed with the default list. When I ran your Philly app last night, I saw the globe and thought "That's a cool map" and went to the imagery dropdown to see which map was selected. None of the maps in the drop down were selected. This code should pick out one from the default list rather than constructing a new one. And perhaps the viewer or its documentation should steer developers in that direction. |
@emackey sounds good. Show me the code and we'll be happy to use it. |
The code @emackey was talking about is something like the below. I tried this in the app and it worked fine. I was aware of this issue when we originally talked about this, but hackathon code has entirely different standards.
|
Back on-topic, can/should the geocoding widget not warn about the key until a geocode request is issued? Or until someone clicks the widget? |
That's easy enough to do. But whether it's a good idea... I'm not sure. It would increase the chances that someone deploys an app with a Bing dependency without realizing it. |
I agree with @kring if anything I wish the warning was even more in your face (like a small credit on the display with full details in the console). We need to make it absolutely clear to people that they need their own key. |
I agree that the geocoder should warn. I think we just need to add more details to the warning so users know where the Bing key is used. |
Given our recent Bing map keys woes, we should probably create a much more "in-your-face" message for when the API key is set to the default, perhaps by overlaying an HTML element or ViewportQuad in the top center of the screen? |
Yes, we might also even ask that they get a key before downloading Cesium like Leaflet (or default to something low-res that we host). Let's hold for now until we think more deeply. |
Now that we are rotated keys, should we just close this? |
@mramato if you want to close this, we should have a different issue for making the geocoder work with other APIs. That's really what this issue is about |
We already have an issue for that in #1262 |
Okay, that's fine then |
A Bing Maps key warning is generated when we construct the viewer widget with the following code. Perhaps this needs to be done a different way?
The text was updated successfully, but these errors were encountered: