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

Unnecessary Bing Maps warning #1165

Closed
pjcozzi opened this issue Sep 21, 2013 · 16 comments
Closed

Unnecessary Bing Maps warning #1165

pjcozzi opened this issue Sep 21, 2013 · 16 comments
Assignees
Labels
good first issue An opportunity for first time contributors type - bug

Comments

@pjcozzi
Copy link
Contributor

pjcozzi commented Sep 21, 2013

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?

var viewer = new Cesium.Viewer('cesiumContainer', {
    selectedImageryProviderViewModel : new Cesium.ImageryProviderViewModel(
    {
        name : 'Open\u00adStreet\u00adMap',
        iconUrl : Cesium.buildModuleUrl('Widgets/Images/ImageryProviders/openStreetMap.png'),
        tooltip : 'OpenStreetMap (OSM) is a collaborative project to create a free editable map \
of the world.\nhttp://www.openstreetmap.org',
        creationFunction : function() {
            return new Cesium.OpenStreetMapImageryProvider({
                url : 'http://tile.openstreetmap.org/'
            });
        }
    })
});
@kring
Copy link
Member

kring commented Sep 22, 2013

Is this with the geocoding widget merged in? Because that uses the Bing API, too.

@mramato
Copy link
Contributor

mramato commented Sep 22, 2013

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.

@pjcozzi
Copy link
Contributor Author

pjcozzi commented Sep 22, 2013

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.

@emackey
Copy link
Contributor

emackey commented Sep 22, 2013

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.

@pjcozzi
Copy link
Contributor Author

pjcozzi commented Sep 22, 2013

@emackey sounds good. Show me the code and we'll be happy to use it.

@mramato
Copy link
Contributor

mramato commented Sep 22, 2013

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.

var viewer = new Cesium.Viewer('cesiumContainer', {
    animation : false,
    timeline : false
});

var baseLayerPicker = viewer.baseLayerPicker;
var layers = baseLayerPicker.viewModel.imageryProviderViewModels;
for ( var i = 0; i < layers.length; i++) {
    var item = layers[i];
    if (item.name === 'Open\u00adStreet\u00adMap') {
        baseLayerPicker.viewModel.selectedItem = item;
        break;
    }
}

@emackey
Copy link
Contributor

emackey commented Sep 23, 2013

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?

@kring
Copy link
Member

kring commented Sep 23, 2013

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.

@mramato
Copy link
Contributor

mramato commented Sep 23, 2013

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.

@pjcozzi
Copy link
Contributor Author

pjcozzi commented Sep 24, 2013

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.

@pjcozzi pjcozzi added the good first issue An opportunity for first time contributors label Dec 24, 2014
@mramato
Copy link
Contributor

mramato commented Oct 14, 2015

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?

@pjcozzi
Copy link
Contributor Author

pjcozzi commented Oct 14, 2015

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.

@mramato
Copy link
Contributor

mramato commented Sep 18, 2016

Now that we are rotated keys, should we just close this?

@hpinkos
Copy link
Contributor

hpinkos commented Sep 19, 2016

@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

@mramato
Copy link
Contributor

mramato commented Sep 19, 2016

We already have an issue for that in #1262

@hpinkos
Copy link
Contributor

hpinkos commented Sep 19, 2016

Okay, that's fine then

@hpinkos hpinkos closed this as completed Sep 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue An opportunity for first time contributors type - bug
Projects
None yet
Development

No branches or pull requests

5 participants