r69037 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r69036‎ | r69037 | r69038 >
Date:23:40, 4 July 2010
Author:avar
Status:deferred (Comments)
Tags:
Comment:
Maps: define a openlayers google group

This doesn't work yet because we also need to import the Google Maps
API in addition to OpenLayers, see the example here:

http://openlayers.org/dev/examples/google.html
Modified paths:
  • /trunk/extensions/Maps/Maps_Settings.php (modified) (history)

Diff [purge]

Index: trunk/extensions/Maps/Maps_Settings.php
——@@ -359,6 +359,11 @@
360360
361361 # The difinitions for the layers that should be available for the user.
362362 $egMapsOLAvailableLayers = array(
 363+ 'google-physical' => array( 'OpenLayers.Layer.Google( "Google Physical", {type: G_PHYSICAL_MAP} )', 'google' ),
 364+ 'google-streets' => array( 'OpenLayers.Layer.Google( "Google Streets", {numZoomLevels: 20} )', 'google' ),
 365+ 'google-hybrid' => array( 'OpenLayers.Layer.Google( "Google Hybrid", {type: G_HYBRID_MAP, numZoomLevels: 20} )', 'google' ),
 366+ 'google-satellite' => array( 'OpenLayers.Layer.Google( "Google Satellite", {type: G_SATELLITE_MAP, numZoomLevels: 22} )', 'google' ),
 367+
363368 'bing-normal' => array( 'OpenLayers.Layer.VirtualEarth( "Bing Streets", {type: VEMapStyle.Shaded, "sphericalMercator":true} )', 'bing' ),
364369 'bing-satellite' => array( 'OpenLayers.Layer.VirtualEarth( "Bing Satellite", {type: VEMapStyle.Aerial, "sphericalMercator":true} )', 'bing' ),
365370 'bing-hybrid' => array( 'OpenLayers.Layer.VirtualEarth( "Bing Hybrid", {type: VEMapStyle.Hybrid, "sphericalMercator":true} )', 'bing' ),
——@@ -394,6 +399,7 @@
395400 # Layer group definitions. Group names must be different from layer names, and
396401 # must only contain layers that are present in $egMapsOLAvailableLayers.
397402 $egMapsOLLayerGroups = array(
 403+ 'google' => array( 'google-physical', 'google-streets', 'google-hybrid', 'google-satellite' ),
398404 'yahoo' => array( 'yahoo-normal', 'yahoo-satellite', 'yahoo-hybrid' ),
399405 'bing' => array( 'bing-normal', 'bing-satellite', 'bing-hybrid' ),
400406 'osm' => array( 'osmarender', 'osm-mapnik', 'osm-cyclemap' ),

Follow-up revisions

RevisionCommit summaryAuthorDate
r69102Reverted r69037jeroendedauw17:43, 6 July 2010

Comments

#Comment by Jeroen De Dauw (talk | contribs)   17:45, 6 July 2010

I remembered why I moved this out of the default Maps settings - you need to define a dependency for this to work, and the dependency needs to API key, so the whole thing will break when you don't have the key, even when not using G Maps. Some specific code could be created to handle this, but I want the layer stuff to stay generic, so you can completely remove any layer service editing only the settings file.

#Comment by Jeroen De Dauw (talk | contribs)   17:45, 6 July 2010

So reverted in r69102

#Comment by Ævar Arnfjörð Bjarmason (talk | contribs)   17:59, 6 July 2010

The revert is fine. But the point of being able to use OpenLayers + Google Maps is not to avoid using an API key.

It's being able to use OSM, Yahoo, Bing etc. *and* Google Maps all at the same time. And being able to use the OpenLayers styles / internationalization etc.

But that can be done by editing the config file, and as I understand once Maps moves to Google Maps API v3 you won't need a map key anymore, so this support isn't needed.

I still think it would be better though to have a working preset (which would automatically pull in the Google Maps JavaScript) for completion, instead of everyone having to copy/paste the config for it.

#Comment by Jeroen De Dauw (talk | contribs)   18:05, 6 July 2010

If you see a way of doing this without hard-coding in specific Google Maps support and not breaking stuff for people who do not have the API key that would be great, but I figure it can't be done with the current setup :(

#Comment by Ævar Arnfjörð Bjarmason (talk | contribs)   18:08, 6 July 2010

I didn't see a way to do it other than some hack to globally keep track of what JavaScript we've included, not include the Google JavaScript twice, and make the OpenLayers output also include the Google code if the google layer is requested.

But conceptually this is probably something that should be supported, and not just for Google. There are probably layers you can use with OpenLayers that need some custom JavaScript / configuration.

It's fine that that needs to go in LocalSettings, though.

#Comment by Jeroen De Dauw (talk | contribs)   18:16, 6 July 2010

You can define dependencies, but not have variables in them, like the gmaps key

Status & tagging log