Controls of type 'OpenLayers.Control.Button' should inherit from 'OpenLayers.Control.Button' #414

merged 15 commits into from Aug 17, 2012

3 participants

OpenLayers member

This pull requests suggests that the controls Pan, ZoomIn, ZoomOut and ZoomToMaxExtent should inherit from OpenLayers.Control.Button instead of from OpenLayers.Control. This let's us define and set the type property only once and not separately in all of the subclasses.

Also included in this request are newly added testcases for all of the above changed classes. Previously none were existing for these.

Some of the controls have also been changed to not do anything (including not to throw exceptions) when their respective trigger-method would be called prior the control being added to a map.

All relevant tests pass in Chromium 11, Firefox 11 (both on Linux-Ubuntu) and Internet Explorer 6 and 8 (both on Windows XP).

Please review.

@jorix jorix commented on the diff Apr 17, 2012
@@ -70,26 +62,27 @@ OpenLayers.Control.Pan = OpenLayers.Class(OpenLayers.Control, {
* Method: trigger
trigger: function(){
+ if ( {
jorix Apr 17, 2012

Why this if? (IMHO the Control.Panel can not trigger it before being on the map)

marcjansen Apr 18, 2012 OpenLayers member

I think it doesn't hurt and the cotrols should be usable even when added directly to a map and not via a panel.

marcjansen added some commits May 15, 2012
@marcjansen marcjansen Merge branch 'master' of int…
…o control-inheritance
@marcjansen marcjansen Merge branch 'control-inheritance' of…
…penlayers into control-inheritance

@marcjansen marcjansen merged commit 0c87de4 into openlayers:master Aug 17, 2012
OpenLayers member

As a post-facto review, these changes look good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment