From e7107b96cbba63327a878d6cd1ef197783940363 Mon Sep 17 00:00:00 2001 From: ahocevar Date: Sat, 21 Jan 2012 19:11:08 +0100 Subject: [PATCH 1/3] Fixing PanZoomBar and Panel issues after #164. For PanZoomBar, this fixes the slider behavior. Now the buttonclick listener argument also includes a buttonXY property, and PanZoomPanel does not need an Events instance for the zoombarDiv any more. For Panel, this fixes events for panels outside the map. Just setting the element on the Events instance was no longer enough after e70569b2bb4c573d6b52ca569230db7fca501c18. Events::attachToElement is now used, and it needed to be modified to also work if the Events instance had no element previously. Finally, I renamed the button property of the buttonclick listener argument to buttonElement, to not confuse it with the browser event button property, and added some more tests and documentation. --- lib/OpenLayers/Control/LayerSwitcher.js | 4 +- lib/OpenLayers/Control/OverviewMap.js | 4 +- lib/OpenLayers/Control/PanZoom.js | 2 +- lib/OpenLayers/Control/PanZoomBar.js | 57 ++++++------------------- lib/OpenLayers/Control/Panel.js | 4 +- lib/OpenLayers/Events.js | 21 ++++----- lib/OpenLayers/Events/buttonclick.js | 26 ++++++++--- tests/Control/PanZoom.html | 9 ++-- tests/Control/PanZoomBar.html | 14 +++--- tests/Control/Panel.html | 15 +++++++ tests/Events.html | 10 +++++ 11 files changed, 87 insertions(+), 79 deletions(-) diff --git a/lib/OpenLayers/Control/LayerSwitcher.js b/lib/OpenLayers/Control/LayerSwitcher.js index 552b650155..2b5d3e9bac 100644 --- a/lib/OpenLayers/Control/LayerSwitcher.js +++ b/lib/OpenLayers/Control/LayerSwitcher.js @@ -197,9 +197,9 @@ OpenLayers.Control.LayerSwitcher = * evt - {Event} */ onButtonClick: function(evt) { - if (evt.button === this.minimizeDiv) { + if (evt.buttonElement === this.minimizeDiv) { this.minimizeControl(); - } else if (evt.button === this.maximizeDiv) { + } else if (evt.buttonElement === this.maximizeDiv) { this.maximizeControl(); }; }, diff --git a/lib/OpenLayers/Control/OverviewMap.js b/lib/OpenLayers/Control/OverviewMap.js index d3e19003d9..67bba68886 100644 --- a/lib/OpenLayers/Control/OverviewMap.js +++ b/lib/OpenLayers/Control/OverviewMap.js @@ -352,9 +352,9 @@ OpenLayers.Control.OverviewMap = OpenLayers.Class(OpenLayers.Control, { * evt - {Event} */ onButtonClick: function(evt) { - if (evt.button === this.minimizeDiv) { + if (evt.buttonElement === this.minimizeDiv) { this.minimizeControl(); - } else if (evt.button === this.maximizeDiv) { + } else if (evt.buttonElement === this.maximizeDiv) { this.maximizeControl(); }; }, diff --git a/lib/OpenLayers/Control/PanZoom.js b/lib/OpenLayers/Control/PanZoom.js index c120589b34..1caa5fe784 100644 --- a/lib/OpenLayers/Control/PanZoom.js +++ b/lib/OpenLayers/Control/PanZoom.js @@ -176,7 +176,7 @@ OpenLayers.Control.PanZoom = OpenLayers.Class(OpenLayers.Control, { * evt - {Event} */ onButtonClick: function(evt) { - var btn = evt.button; + var btn = evt.buttonElement; switch (btn.action) { case "panup": this.map.pan(0, -this.getSlideFactor("h")); diff --git a/lib/OpenLayers/Control/PanZoomBar.js b/lib/OpenLayers/Control/PanZoomBar.js index ceb7ccb215..3646e41554 100644 --- a/lib/OpenLayers/Control/PanZoomBar.js +++ b/lib/OpenLayers/Control/PanZoomBar.js @@ -239,18 +239,9 @@ OpenLayers.Control.PanZoomBar = OpenLayers.Class(OpenLayers.Control.PanZoom, { imgLocation); } div.style.cursor = "pointer"; + div.className = "olButton"; this.zoombarDiv = div; - this.divEvents = new OpenLayers.Events(this, div, null, true, - {includeXY: true}); - this.divEvents.on({ - "touchmove": this.passEventToSlider, - "mousedown": this.divClick, - "mousemove": this.passEventToSlider, - "dblclick": this.doubleClick, - "click": this.doubleClick - }); - this.div.appendChild(div); this.startTop = parseInt(div.style.top); @@ -277,15 +268,6 @@ OpenLayers.Control.PanZoomBar = OpenLayers.Class(OpenLayers.Control.PanZoom, { }); this.sliderEvents.destroy(); - this.divEvents.un({ - "touchmove": this.passEventToSlider, - "mousedown": this.divClick, - "mousemove": this.passEventToSlider, - "dblclick": this.doubleClick, - "click": this.doubleClick - }); - this.divEvents.destroy(); - this.div.removeChild(this.zoombarDiv); this.zoombarDiv = null; this.div.removeChild(this.slider); @@ -295,34 +277,19 @@ OpenLayers.Control.PanZoomBar = OpenLayers.Class(OpenLayers.Control.PanZoom, { }, /** - * Method: passEventToSlider - * This function is used to pass events that happen on the div, or the map, - * through to the slider, which then does its moving thing. - * - * Parameters: - * evt - {} - */ - passEventToSlider:function(evt) { - this.sliderEvents.handleBrowserEvent(evt); - }, - - /** - * Method: divClick - * Picks up on clicks directly on the zoombar div - * and sets the zoom level appropriately. + * Method: onButtonClick */ - divClick: function (evt) { - if (!OpenLayers.Event.isLeftClick(evt)) { - return; + onButtonClick: function(evt) { + OpenLayers.Control.PanZoom.prototype.onButtonClick.apply(this, arguments); + if (evt.buttonElement === this.zoombarDiv) { + var levels = evt.buttonXY.y / this.zoomStopHeight; + if(this.forceFixedZoomLevel || !this.map.fractionalZoom) { + levels = Math.floor(levels); + } + var zoom = (this.map.getNumZoomLevels() - 1) - levels; + zoom = Math.min(Math.max(zoom, 0), this.map.getNumZoomLevels() - 1); + this.map.zoomTo(zoom); } - var levels = evt.xy.y / this.zoomStopHeight; - if(this.forceFixedZoomLevel || !this.map.fractionalZoom) { - levels = Math.floor(levels); - } - var zoom = (this.map.getNumZoomLevels() - 1) - levels; - zoom = Math.min(Math.max(zoom, 0), this.map.getNumZoomLevels() - 1); - this.map.zoomTo(zoom); - OpenLayers.Event.stop(evt); }, /* diff --git a/lib/OpenLayers/Control/Panel.js b/lib/OpenLayers/Control/Panel.js index 5dc4db15d7..b04f6c3863 100644 --- a/lib/OpenLayers/Control/Panel.js +++ b/lib/OpenLayers/Control/Panel.js @@ -170,7 +170,7 @@ OpenLayers.Control.Panel = OpenLayers.Class(OpenLayers.Control, { draw: function() { OpenLayers.Control.prototype.draw.apply(this, arguments); if (this.outsideViewport) { - this.events.element = this.div; + this.events.attachToElement(this.div); this.events.register("buttonclick", this, this.onButtonClick); } else { this.map.events.register("buttonclick", this, this.onButtonClick); @@ -322,7 +322,7 @@ OpenLayers.Control.Panel = OpenLayers.Class(OpenLayers.Control, { */ onButtonClick: function (evt) { var controls = this.controls, - button = evt.button || OpenLayers.Event.element(evt); + button = evt.buttonElement; for (var i=controls.length-1; i>=0; --i) { if (controls[i].panel_div === button) { this.activateControl(controls[i]); diff --git a/lib/OpenLayers/Events.js b/lib/OpenLayers/Events.js index 04b90f2bdb..224c46e7e9 100644 --- a/lib/OpenLayers/Events.js +++ b/lib/OpenLayers/Events.js @@ -551,16 +551,6 @@ OpenLayers.Events = OpenLayers.Class({ // if a dom element is specified, add a listeners list // for browser events on the element and register them if (element != null) { - // keep a bound copy of handleBrowserEvent() so that we can - // pass the same function to both Event.observe() and .stopObserving() - this.eventHandler = OpenLayers.Function.bindAsEventListener( - this.handleBrowserEvent, this - ); - - // to be used with observe and stopObserving - this.clearMouseListener = OpenLayers.Function.bind( - this.clearMouseCache, this - ); this.attachToElement(element); } }, @@ -610,6 +600,17 @@ OpenLayers.Events = OpenLayers.Class({ attachToElement: function (element) { if (this.element) { OpenLayers.Event.stopObservingElement(this.element); + } else { + // keep a bound copy of handleBrowserEvent() so that we can + // pass the same function to both Event.observe() and .stopObserving() + this.eventHandler = OpenLayers.Function.bindAsEventListener( + this.handleBrowserEvent, this + ); + + // to be used with observe and stopObserving + this.clearMouseListener = OpenLayers.Function.bind( + this.clearMouseCache, this + ); } this.element = element; for (var i = 0, len = this.BROWSER_EVENTS.length; i < len; i++) { diff --git a/lib/OpenLayers/Events/buttonclick.js b/lib/OpenLayers/Events/buttonclick.js index 6b8e16cdeb..1485714d31 100644 --- a/lib/OpenLayers/Events/buttonclick.js +++ b/lib/OpenLayers/Events/buttonclick.js @@ -15,6 +15,12 @@ * * This event type makes sure that button clicks do not interfere with other * events that are registered on the same . + * + * Event types provided by this extension: + * - *buttonclick* Triggered when a button is clicked. Listeners receive an + * object with a *buttonElement* property referencing the dom element of + * the clicked button, and an *buttonXY* property with the click position + * relative to the button. */ OpenLayers.Events.buttonclick = OpenLayers.Class({ @@ -57,6 +63,11 @@ OpenLayers.Events.buttonclick = OpenLayers.Class({ */ completeRegEx: /^mouseup|touchend$/, + /** + * Property: startEvt + * {Event} The event that started the click sequence + */ + /** * Constructor: OpenLayers.Events.buttonclick * Construct a buttonclick event type. Applications are not supposed to @@ -100,25 +111,30 @@ OpenLayers.Events.buttonclick = OpenLayers.Class({ element = element.parentNode; } if (OpenLayers.Element.hasClass(element, "olButton")) { - if (this._buttonStarted) { + if (this.startEvt) { if (this.completeRegEx.test(evt.type)) { + var pos = OpenLayers.Util.pagePosition(element); this.target.triggerEvent("buttonclick", { - button: element + buttonElement: element, + buttonXY: { + x: this.startEvt.clientX - pos[0], + y: this.startEvt.clientY - pos[1] + } }); } if (this.cancelRegEx.test(evt.type)) { - delete this._buttonStarted; + delete this.startEvt; } OpenLayers.Event.stop(evt); propagate = false; } if (this.startRegEx.test(evt.type)) { - this._buttonStarted = true; + this.startEvt = evt; OpenLayers.Event.stop(evt); propagate = false; } } else { - delete this._buttonStarted; + delete this.startEvt; } } return propagate; diff --git a/tests/Control/PanZoom.html b/tests/Control/PanZoom.html index 2e9a68b388..e99398e7e6 100644 --- a/tests/Control/PanZoom.html +++ b/tests/Control/PanZoom.html @@ -159,7 +159,6 @@ }; var delta, dir; - var evt = {which: 1}; // a fake left click var buttons = control.buttons; map.pan = function(dx, dy){ t.eq([dx,dy],delta,"Panning " + dir + " sets right delta with slideRatio"); @@ -168,25 +167,25 @@ //up var delta = [0, -50]; var dir = "up"; - evt.button = buttons[0]; + var evt = {buttonElement: buttons[0]}; control.onButtonClick.call(control, evt); //left var delta = [-125, 0]; var dir = "left"; - evt.button = buttons[1]; + evt.buttonElement = buttons[1]; control.onButtonClick.call(control, evt); //right var delta = [125, 0]; var dir = "right"; - evt.button = buttons[2]; + evt.buttonElement = buttons[2]; control.onButtonClick.call(control, evt); //down var delta = [0, 50]; var dir = "down"; - evt.button = buttons[3]; + evt.buttonElement = buttons[3]; control.onButtonClick.call(control, evt); map.destroy(); diff --git a/tests/Control/PanZoomBar.html b/tests/Control/PanZoomBar.html index 891527eab6..852c00f4df 100644 --- a/tests/Control/PanZoomBar.html +++ b/tests/Control/PanZoomBar.html @@ -50,7 +50,7 @@ t.eq(control.zoombarDiv, null, "zoombar div nullified.") } - function test_Control_PanZoomBar_divClick (t) { + function test_Control_PanZoomBar_onButtonClick (t) { t.plan(2); map = new OpenLayers.Map('map', {controls:[]}); var layer = new OpenLayers.Layer.WMS("Test Layer", @@ -59,16 +59,16 @@ map.addLayer(layer); control = new OpenLayers.Control.PanZoomBar(); map.addControl(control); - control.divClick({'xy': {'x': 0, 'y': 50}, which: 1}); + control.onButtonClick({'buttonXY': {'x': 0, 'y': 50}, buttonElement: control.zoombarDiv}); t.eq(map.zoom, 11, "zoom is correct on standard map"); map.fractionalZoom = true; - control.divClick({'xy': {'x': 0, 'y': 49}, which: 1}); + control.onButtonClick({'buttonXY': {'x': 0, 'y': 49}, buttonElement: control.zoombarDiv}); t.eq(map.zoom.toFixed(3), '10.545', "zoom is correct on fractional zoom map"); } - function test_Control_PanZoomBar_forceFixedZoomLevel_divClick(t){ + function test_Control_PanZoomBar_forceFixedZoomLevel_onButtonClick(t){ t.plan(1); map = new OpenLayers.Map('map', { controls: [], @@ -84,12 +84,12 @@ }); map.addControl(control); - control.divClick({ - 'xy': { + control.onButtonClick({ + 'buttonXY': { 'x': 0, 'y': 49 }, - which: 1 + buttonElement: control.zoombarDiv }); t.eq(map.zoom, 11, "forceFixedZoomLevel makes sure that after a div click only fixed zoom levels are used even if the map has fractionalZoom"); } diff --git a/tests/Control/Panel.html b/tests/Control/Panel.html index d88a7a2abe..98ccc2c122 100644 --- a/tests/Control/Panel.html +++ b/tests/Control/Panel.html @@ -322,6 +322,21 @@ map.destroy(); } + + function test_buttonclick(t) { + t.plan(4); + var map = new OpenLayers.Map('map'); + var panel1 = new OpenLayers.Control.Panel(); + var div = document.createElement("div"); + var panel2 = new OpenLayers.Control.Panel({div: div}); + map.addControls([panel1, panel2]); + + t.ok(map.events.listeners.buttonclick, "buttonclick event registered on map's Events instance for panel inside map"); + t.ok(!panel1.events.element, "Panel inside map has no element on its Events instance"); + t.ok(panel2.events.listeners.buttonclick, "buttonclick event registered on panel's Events instance if outside map") + t.ok(panel2.events.element === div, "Panel outside map has the panel's div as element on its Events instance"); + + } diff --git a/tests/Events.html b/tests/Events.html index cd3b04d42a..eebda2426c 100644 --- a/tests/Events.html +++ b/tests/Events.html @@ -313,6 +313,16 @@ t.eq(evt.clientX, 1.5, "evt.clientX value is correct"); t.eq(evt.clientY, 1.5, "evt.clientY value is correct"); } + + function test_Events_attachToElement(t) { + t.plan(3); + var events = new OpenLayers.Events({}, null); + var element = document.createElement("div"); + events.attachToElement(element); + t.ok(events.eventHandler, "eventHandler method bound"); + t.ok(events.clearMouseListener, "clearMouseListener method bound"); + t.ok(events.element === element, "element set"); + } function test_Events_destroy (t) { t.plan(2); From ed889412803aafc8fb8cd5d6db607def0c90850c Mon Sep 17 00:00:00 2001 From: ahocevar Date: Sun, 22 Jan 2012 13:03:56 +0100 Subject: [PATCH 2/3] Incorporating review comments from @jorix. --- lib/OpenLayers/Control/PanZoomBar.js | 35 +++++++++++++++++----------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/lib/OpenLayers/Control/PanZoomBar.js b/lib/OpenLayers/Control/PanZoomBar.js index 3646e41554..941a816a6e 100644 --- a/lib/OpenLayers/Control/PanZoomBar.js +++ b/lib/OpenLayers/Control/PanZoomBar.js @@ -47,12 +47,6 @@ OpenLayers.Control.PanZoomBar = OpenLayers.Class(OpenLayers.Control.PanZoom, { */ zoombarDiv: null, - /** - * Property: divEvents - * {} - */ - divEvents: null, - /** * APIProperty: zoomWorldIcon * {Boolean} @@ -212,9 +206,7 @@ OpenLayers.Control.PanZoomBar = OpenLayers.Class(OpenLayers.Control.PanZoom, { "touchend": this.zoomBarUp, "mousedown": this.zoomBarDown, "mousemove": this.zoomBarDrag, - "mouseup": this.zoomBarUp, - "dblclick": this.doubleClick, - "click": this.doubleClick + "mouseup": this.zoomBarUp }); var sz = { @@ -259,15 +251,15 @@ OpenLayers.Control.PanZoomBar = OpenLayers.Class(OpenLayers.Control.PanZoom, { */ _removeZoomBar: function() { this.sliderEvents.un({ + "touchstart": this.zoomBarDown, "touchmove": this.zoomBarDrag, + "touchend": this.zoomBarUp, "mousedown": this.zoomBarDown, "mousemove": this.zoomBarDrag, - "mouseup": this.zoomBarUp, - "dblclick": this.doubleClick, - "click": this.doubleClick + "mouseup": this.zoomBarUp }); this.sliderEvents.destroy(); - + this.div.removeChild(this.zoombarDiv); this.zoombarDiv = null; this.div.removeChild(this.slider); @@ -278,6 +270,9 @@ OpenLayers.Control.PanZoomBar = OpenLayers.Class(OpenLayers.Control.PanZoom, { /** * Method: onButtonClick + * + * Parameters: + * evt - {Event} */ onButtonClick: function(evt) { OpenLayers.Control.PanZoom.prototype.onButtonClick.apply(this, arguments); @@ -292,6 +287,18 @@ OpenLayers.Control.PanZoomBar = OpenLayers.Class(OpenLayers.Control.PanZoom, { } }, + /** + * Method: passEventToSlider + * This function is used to pass events that happen on the div, or the map, + * through to the slider, which then does its moving thing. + * + * Parameters: + * evt - {} + */ + passEventToSlider:function(evt) { + this.sliderEvents.handleBrowserEvent(evt); + }, + /* * Method: zoomBarDown * event listener for clicks on the slider @@ -392,4 +399,4 @@ OpenLayers.Control.PanZoomBar = OpenLayers.Class(OpenLayers.Control.PanZoom, { }, CLASS_NAME: "OpenLayers.Control.PanZoomBar" -}); +}); \ No newline at end of file From 5a1378cdab2f2180499763cbe4bf9a58451fdaba Mon Sep 17 00:00:00 2001 From: ahocevar Date: Sun, 22 Jan 2012 21:19:00 +0100 Subject: [PATCH 3/3] Improved extension example docs and destroy method. --- lib/OpenLayers/Events.js | 8 +++++--- lib/OpenLayers/Events/buttonclick.js | 1 + 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/OpenLayers/Events.js b/lib/OpenLayers/Events.js index 224c46e7e9..907ced8109 100644 --- a/lib/OpenLayers/Events.js +++ b/lib/OpenLayers/Events.js @@ -486,10 +486,12 @@ OpenLayers.Events = OpenLayers.Class({ * this.target.extensions["fooend"] = true; * }, * destroy: function() { - * this.target.unregister("click", this, this.doStuff); + * var target = this.target; + * target.unregister("click", this, this.doStuff); + * delete this.target; * // only required if extension provides more than one event type - * delete this.target.extensions["foostart"]; - * delete this.target.extensions["fooend"]; + * delete target.extensions["foostart"]; + * delete target.extensions["fooend"]; * }, * doStuff: function(evt) { * var propagate = true; diff --git a/lib/OpenLayers/Events/buttonclick.js b/lib/OpenLayers/Events/buttonclick.js index 1485714d31..1fddfb705e 100644 --- a/lib/OpenLayers/Events/buttonclick.js +++ b/lib/OpenLayers/Events/buttonclick.js @@ -94,6 +94,7 @@ OpenLayers.Events.buttonclick = OpenLayers.Class({ for (var i=this.events.length-1; i>=0; --i) { this.target.unregister(this.events[i], this, this.buttonClick); } + delete this.target; }, /**