From 8ec845d5204f6e5e4ba63d2ed489de01555fff68 Mon Sep 17 00:00:00 2001 From: "Street, Arthur (Data61, Eveleigh ATP)" Date: Fri, 4 Aug 2017 12:02:28 +1000 Subject: [PATCH 01/10] Fix so removing layers from the control restores them to both sides --- CHANGELOG.md | 1 + index.js | 112 ++++++++++++++++++++++++++++++--------------------- 2 files changed, 68 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d129ffe..1824f51 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - ADDED: `getPosition()` returns the x coordinate (relative to the map container) of the divider - FIXED: **[BREAKING]** Export factory function on `L.control` not `L.Control` - FIXED: Slider drag was not working on touch devices +- FIXED: Removing layers from the control did not restore them to both sides ## [v1.1.1] - 2015-12-03 diff --git a/index.js b/index.js index 6626047..cc0e02d 100644 --- a/index.js +++ b/index.js @@ -42,13 +42,26 @@ function uncancelMapDrag (e) { // convert arg to an array - returns empty array if arg is undefined function asArray (arg) { - return (arg === 'undefined') ? [] : Array.isArray(arg) ? arg : [arg] + return (arg === undefined) ? [] : Array.isArray(arg) ? arg : [arg] } function noop () { return } +function applyToMissingLayers(map, layers, layersToCheckAgainst, applyFunction) { + // Loops through each layer in layers, and if the layer is on the map but NOT in layersToCheckAgainst, + // calls doFunction(layer). + layers.forEach(function (layer) { + if (layer && map.hasLayer(layer)) { + if (layersToCheckAgainst.indexOf(layer) < 0) { + applyFunction(layer) + } + } + }) + +} + L.Control.SideBySide = L.Control.extend({ options: { thumbSize: 42, @@ -86,7 +99,7 @@ L.Control.SideBySide = L.Control.extend({ range.value = 0.5 range.style.paddingLeft = range.style.paddingRight = this.options.padding + 'px' this._addEvents() - this._updateLayers() + this._updateLayers(this._leftLayers, this._rightLayers) return this }, @@ -94,12 +107,6 @@ L.Control.SideBySide = L.Control.extend({ if (!this._map) { return this } - if (this._leftLayer) { - this._leftLayer.getContainer().style.clip = "" - } - if (this._rightLayer) { - this._rightLayer.getContainer().style.clip = "" - } this._removeEvents() L.DomUtil.remove(this._container) @@ -109,14 +116,12 @@ L.Control.SideBySide = L.Control.extend({ }, setLeftLayers: function (leftLayers) { - this._leftLayers = asArray(leftLayers) - this._updateLayers() + this._updateLayers(asArray(leftLayers), null) return this }, setRightLayers: function (rightLayers) { - this._rightLayers = asArray(rightLayers) - this._updateLayers() + this._updateLayers(null, asArray(rightLayers)) return this }, @@ -131,51 +136,68 @@ L.Control.SideBySide = L.Control.extend({ this.fire('dividermove', {x: dividerX}) var clipLeft = 'rect(' + [nw.y, clipX, se.y, nw.x].join('px,') + 'px)' var clipRight = 'rect(' + [nw.y, se.x, se.y, clipX].join('px,') + 'px)' - if (this._leftLayer) { - this._leftLayer.getContainer().style.clip = clipLeft - } - if (this._rightLayer) { - this._rightLayer.getContainer().style.clip = clipRight - } + this._leftLayers.forEach(function(layer) { + layer.getContainer().style.clip = clipLeft + }) + this._rightLayers.forEach(function(layer) { + layer.getContainer().style.clip = clipRight + }) }, - _updateLayers: function () { - if (!this._map) { + _removeClip: function (layer) { + layer.getContainer().style.clip = '' + }, + + _updateLayers: function (newLeftLayers, newRightLayers) { + var map = this._map + if (!map) { return this } - var prevLeft = this._leftLayer - var prevRight = this._rightLayer - this._leftLayer = this._rightLayer = null - this._leftLayers.forEach(function (layer) { - if (this._map.hasLayer(layer)) { - this._leftLayer = layer - } - }, this) - this._rightLayers.forEach(function (layer) { - if (this._map.hasLayer(layer)) { - this._rightLayer = layer - } - }, this) - if (prevLeft !== this._leftLayer) { - prevLeft && this.fire('leftlayerremove', {layer: prevLeft}) - this._leftLayer && this.fire('leftlayeradd', {layer: this._leftLayer}) + var prevLeftLayers = asArray(this._leftLayers) + var prevRightLayers = asArray(this._rightLayers) + + // If either parameter is not supplied, use the original; this can still lead to events being fired + // because whether the layer is on the map can change. + if (!newLeftLayers) { + newLeftLayers = asArray(this._leftLayers) } - if (prevRight !== this._rightLayer) { - prevRight && this.fire('rightlayerremove', {layer: prevRight}) - this._rightLayer && this.fire('rightlayeradd', {layer: this._rightLayer}) + if (!newRightLayers) { + newRightLayers = asArray(this._rightLayers) } + var that = this + // Add new layers. + applyToMissingLayers(map, newLeftLayers, prevLeftLayers, function(layer) {that.fire('leftlayeradd', {layer: layer})}) + applyToMissingLayers(map, newRightLayers, prevRightLayers, function(layer) {that.fire('rightlayeradd', {layer: layer})}) + // Remove layers which were present, but are no longer. + applyToMissingLayers(map, prevLeftLayers, newLeftLayers, function(layer) {that.fire('leftlayerremove', {layer: layer})}) + applyToMissingLayers(map, prevRightLayers, newRightLayers, function(layer) {that.fire('rightlayerremove', {layer: layer})}) + + // Any layers which have been removed from the control need their clip css removed, so they appear on both sides. + applyToMissingLayers(map, prevLeftLayers.concat(prevRightLayers), newLeftLayers.concat(newRightLayers), that._removeClip) + + // Update our records. + this._leftLayers = newLeftLayers + this._rightLayers = newRightLayers + + // Update the clip css for the layers which are on the left or right. + // Note this uses this._leftLayers and _rightLayers, so we updated them first. this._updateClip() }, + _updateLayersFromEvent: function () { + // If a layer is added or removed from the map, we don't need to pass which layer it is. + this._updateLayers() + }, + _addEvents: function () { var range = this._range var map = this._map if (!map || !range) return map.on('move', this._updateClip, this) - map.on('layeradd layerremove', this._updateLayers, this) + map.on('layeradd layerremove', this._updateLayersFromEvent, this) on(range, getRangeEvent(range), this._updateClip, this) - on(range, L.Browser.touch ? 'touchstart' : 'mousedown', cancelMapDrag, this) - on(range, L.Browser.touch ? 'touchend' : 'mouseup', uncancelMapDrag, this) + on(range, 'mousedown touchstart', cancelMapDrag, this) + on(range, 'mouseup touchend', uncancelMapDrag, this) }, _removeEvents: function () { @@ -183,11 +205,11 @@ L.Control.SideBySide = L.Control.extend({ var map = this._map if (range) { off(range, getRangeEvent(range), this._updateClip, this) - off(range, L.Browser.touch ? 'touchstart' : 'mousedown', cancelMapDrag, this) - off(range, L.Browser.touch ? 'touchend' : 'mouseup', uncancelMapDrag, this) + off(range, 'mousedown touchstart', cancelMapDrag, this) + off(range, 'mouseup touchend', uncancelMapDrag, this) } if (map) { - map.off('layeradd layerremove', this._updateLayers, this) + map.off('layeradd layerremove', this._updateLayersFromEvent, this) map.off('move', this._updateClip, this) } } From 50249fc92e24f3caf1a171426ee377ccf74215c3 Mon Sep 17 00:00:00 2001 From: "Street, Arthur (Data61, Eveleigh ATP)" Date: Fri, 4 Aug 2017 12:06:43 +1000 Subject: [PATCH 02/10] update changelog for multiple layers --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1824f51..89df761 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - FIXED: **[BREAKING]** Export factory function on `L.control` not `L.Control` - FIXED: Slider drag was not working on touch devices - FIXED: Removing layers from the control did not restore them to both sides +- FIXED: Better support for multiple layers ## [v1.1.1] - 2015-12-03 From ce9edd2ec3c1919ff8d89a9ba115ff4180de6d78 Mon Sep 17 00:00:00 2001 From: Arthur Street Date: Fri, 4 Aug 2017 12:16:50 +1000 Subject: [PATCH 03/10] Update index.js --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index cc0e02d..a476158 100644 --- a/index.js +++ b/index.js @@ -51,7 +51,7 @@ function noop () { function applyToMissingLayers(map, layers, layersToCheckAgainst, applyFunction) { // Loops through each layer in layers, and if the layer is on the map but NOT in layersToCheckAgainst, - // calls doFunction(layer). + // calls applyFunction(layer). layers.forEach(function (layer) { if (layer && map.hasLayer(layer)) { if (layersToCheckAgainst.indexOf(layer) < 0) { From 7bfc2e982609924d1219e006c15f37ddab0bd4b4 Mon Sep 17 00:00:00 2001 From: Arthur Street Date: Fri, 4 Aug 2017 12:21:11 +1000 Subject: [PATCH 04/10] Update index.js --- index.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/index.js b/index.js index a476158..7249127 100644 --- a/index.js +++ b/index.js @@ -104,9 +104,11 @@ L.Control.SideBySide = L.Control.extend({ }, remove: function () { + // Remove the side-by-side control. if (!this._map) { return this } + this._updateLayers([], []) this._removeEvents() L.DomUtil.remove(this._container) From c41fe19f9f892b4a95b4a9f90c802ea7a1cdc9e5 Mon Sep 17 00:00:00 2001 From: Arthur Street Date: Fri, 4 Aug 2017 12:57:40 +1000 Subject: [PATCH 05/10] Update CHANGELOG.md --- CHANGELOG.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 89df761..e715b03 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ This project adheres to [Semantic Versioning](http://semver.org/). ## Unreleased +- FIXED: Removing layers from the control did not restore them to both sides +- FIXED: Better support for multiple layers + ## [v2.0.0] - 2015-12-08 - ADDED: Add `setLeftLayers()` and `setRightLayers()` methods @@ -12,8 +15,6 @@ This project adheres to [Semantic Versioning](http://semver.org/). - ADDED: `getPosition()` returns the x coordinate (relative to the map container) of the divider - FIXED: **[BREAKING]** Export factory function on `L.control` not `L.Control` - FIXED: Slider drag was not working on touch devices -- FIXED: Removing layers from the control did not restore them to both sides -- FIXED: Better support for multiple layers ## [v1.1.1] - 2015-12-03 From 194b9e029f13fdc9ef5c5ac9c91fe70433cfde96 Mon Sep 17 00:00:00 2001 From: "Street, Arthur (Data61, Eveleigh ATP)" Date: Fri, 4 Aug 2017 15:23:14 +1000 Subject: [PATCH 06/10] handle missing container --- index.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index 7249127..25904bd 100644 --- a/index.js +++ b/index.js @@ -59,7 +59,12 @@ function applyToMissingLayers(map, layers, layersToCheckAgainst, applyFunction) } } }) +} +function setClip(layer, clip) { + if (layer.getContainer()) { + layer.getContainer().style.clip = clip + } } L.Control.SideBySide = L.Control.extend({ @@ -139,15 +144,15 @@ L.Control.SideBySide = L.Control.extend({ var clipLeft = 'rect(' + [nw.y, clipX, se.y, nw.x].join('px,') + 'px)' var clipRight = 'rect(' + [nw.y, se.x, se.y, clipX].join('px,') + 'px)' this._leftLayers.forEach(function(layer) { - layer.getContainer().style.clip = clipLeft + setClip(layer, clipLeft) }) this._rightLayers.forEach(function(layer) { - layer.getContainer().style.clip = clipRight + setClip(layer, clipRight) }) }, _removeClip: function (layer) { - layer.getContainer().style.clip = '' + setClip(layer, '') }, _updateLayers: function (newLeftLayers, newRightLayers) { From 59cb55297f507f2106ee2be4c1d58412c4a6d417 Mon Sep 17 00:00:00 2001 From: "Street, Arthur (Data61, Eveleigh ATP)" Date: Fri, 4 Aug 2017 15:29:22 +1000 Subject: [PATCH 07/10] revert an inadvertant change --- index.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/index.js b/index.js index 25904bd..0065d77 100644 --- a/index.js +++ b/index.js @@ -203,8 +203,8 @@ L.Control.SideBySide = L.Control.extend({ map.on('move', this._updateClip, this) map.on('layeradd layerremove', this._updateLayersFromEvent, this) on(range, getRangeEvent(range), this._updateClip, this) - on(range, 'mousedown touchstart', cancelMapDrag, this) - on(range, 'mouseup touchend', uncancelMapDrag, this) + on(range, L.Browser.touch ? 'touchstart' : 'mousedown', cancelMapDrag, this) + on(range, L.Browser.touch ? 'touchend' : 'mouseup', uncancelMapDrag, this) }, _removeEvents: function () { @@ -212,8 +212,8 @@ L.Control.SideBySide = L.Control.extend({ var map = this._map if (range) { off(range, getRangeEvent(range), this._updateClip, this) - off(range, 'mousedown touchstart', cancelMapDrag, this) - off(range, 'mouseup touchend', uncancelMapDrag, this) + off(range, L.Browser.touch ? 'touchstart' : 'mousedown', cancelMapDrag, this) + off(range, L.Browser.touch ? 'touchend' : 'mouseup', uncancelMapDrag, this) } if (map) { map.off('layeradd layerremove', this._updateLayersFromEvent, this) From b96b196c41182732cc3b5fa112147e1a1499d29d Mon Sep 17 00:00:00 2001 From: "Street, Arthur (Data61, Eveleigh ATP)" Date: Tue, 8 Aug 2017 11:26:43 +1000 Subject: [PATCH 08/10] Fix initialize and lint errors --- index.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/index.js b/index.js index 0065d77..d45ea75 100644 --- a/index.js +++ b/index.js @@ -49,7 +49,7 @@ function noop () { return } -function applyToMissingLayers(map, layers, layersToCheckAgainst, applyFunction) { +function applyToMissingLayers (map, layers, layersToCheckAgainst, applyFunction) { // Loops through each layer in layers, and if the layer is on the map but NOT in layersToCheckAgainst, // calls applyFunction(layer). layers.forEach(function (layer) { @@ -61,7 +61,7 @@ function applyToMissingLayers(map, layers, layersToCheckAgainst, applyFunction) }) } -function setClip(layer, clip) { +function setClip (layer, clip) { if (layer.getContainer()) { layer.getContainer().style.clip = clip } @@ -74,8 +74,8 @@ L.Control.SideBySide = L.Control.extend({ }, initialize: function (leftLayers, rightLayers, options) { - this.setLeftLayers(leftLayers) - this.setRightLayers(rightLayers) + this._leftLayers = asArray(leftLayers) + this._rightLayers = asArray(rightLayers) L.setOptions(this, options) }, @@ -143,10 +143,10 @@ L.Control.SideBySide = L.Control.extend({ this.fire('dividermove', {x: dividerX}) var clipLeft = 'rect(' + [nw.y, clipX, se.y, nw.x].join('px,') + 'px)' var clipRight = 'rect(' + [nw.y, se.x, se.y, clipX].join('px,') + 'px)' - this._leftLayers.forEach(function(layer) { + this._leftLayers.forEach(function (layer) { setClip(layer, clipLeft) }) - this._rightLayers.forEach(function(layer) { + this._rightLayers.forEach(function (layer) { setClip(layer, clipRight) }) }, @@ -173,11 +173,11 @@ L.Control.SideBySide = L.Control.extend({ } var that = this // Add new layers. - applyToMissingLayers(map, newLeftLayers, prevLeftLayers, function(layer) {that.fire('leftlayeradd', {layer: layer})}) - applyToMissingLayers(map, newRightLayers, prevRightLayers, function(layer) {that.fire('rightlayeradd', {layer: layer})}) + applyToMissingLayers(map, newLeftLayers, prevLeftLayers, function (layer) { that.fire('leftlayeradd', {layer: layer}) }) + applyToMissingLayers(map, newRightLayers, prevRightLayers, function (layer) { that.fire('rightlayeradd', {layer: layer}) }) // Remove layers which were present, but are no longer. - applyToMissingLayers(map, prevLeftLayers, newLeftLayers, function(layer) {that.fire('leftlayerremove', {layer: layer})}) - applyToMissingLayers(map, prevRightLayers, newRightLayers, function(layer) {that.fire('rightlayerremove', {layer: layer})}) + applyToMissingLayers(map, prevLeftLayers, newLeftLayers, function (layer) { that.fire('leftlayerremove', {layer: layer}) }) + applyToMissingLayers(map, prevRightLayers, newRightLayers, function (layer) { that.fire('rightlayerremove', {layer: layer}) }) // Any layers which have been removed from the control need their clip css removed, so they appear on both sides. applyToMissingLayers(map, prevLeftLayers.concat(prevRightLayers), newLeftLayers.concat(newRightLayers), that._removeClip) From 07a4b1b833631203bfae2d9321204d7814c43cb0 Mon Sep 17 00:00:00 2001 From: "Street, Arthur (Data61, Eveleigh ATP)" Date: Tue, 8 Aug 2017 11:36:47 +1000 Subject: [PATCH 09/10] remove underscore from _updateLayers --- index.js | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/index.js b/index.js index d45ea75..ff358db 100644 --- a/index.js +++ b/index.js @@ -104,7 +104,7 @@ L.Control.SideBySide = L.Control.extend({ range.value = 0.5 range.style.paddingLeft = range.style.paddingRight = this.options.padding + 'px' this._addEvents() - this._updateLayers(this._leftLayers, this._rightLayers) + this.updateLayers() return this }, @@ -113,7 +113,7 @@ L.Control.SideBySide = L.Control.extend({ if (!this._map) { return this } - this._updateLayers([], []) + this.updateLayers([], []) this._removeEvents() L.DomUtil.remove(this._container) @@ -123,12 +123,12 @@ L.Control.SideBySide = L.Control.extend({ }, setLeftLayers: function (leftLayers) { - this._updateLayers(asArray(leftLayers), null) + this.updateLayers(asArray(leftLayers), null) return this }, setRightLayers: function (rightLayers) { - this._updateLayers(null, asArray(rightLayers)) + this.updateLayers(null, asArray(rightLayers)) return this }, @@ -155,7 +155,11 @@ L.Control.SideBySide = L.Control.extend({ setClip(layer, '') }, - _updateLayers: function (newLeftLayers, newRightLayers) { + updateLayers: function (newLeftLayers, newRightLayers) { + // Only sets the layers if there is a map. + // Only shows the layers if they are on the map. + // If either parameter is not supplied, maintains the existing layers on that side. + // This can still lead to a change in display if the layers have been added or removed from the map. var map = this._map if (!map) { return this @@ -163,8 +167,6 @@ L.Control.SideBySide = L.Control.extend({ var prevLeftLayers = asArray(this._leftLayers) var prevRightLayers = asArray(this._rightLayers) - // If either parameter is not supplied, use the original; this can still lead to events being fired - // because whether the layer is on the map can change. if (!newLeftLayers) { newLeftLayers = asArray(this._leftLayers) } @@ -193,7 +195,7 @@ L.Control.SideBySide = L.Control.extend({ _updateLayersFromEvent: function () { // If a layer is added or removed from the map, we don't need to pass which layer it is. - this._updateLayers() + this.updateLayers() }, _addEvents: function () { From 129606883a6d80ec86946f1cded307d0f7d03573 Mon Sep 17 00:00:00 2001 From: "Street, Arthur (Data61, Eveleigh ATP)" Date: Tue, 8 Aug 2017 11:45:41 +1000 Subject: [PATCH 10/10] allow non-arrays as parameters to updateLayers --- index.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/index.js b/index.js index ff358db..1d68139 100644 --- a/index.js +++ b/index.js @@ -164,15 +164,18 @@ L.Control.SideBySide = L.Control.extend({ if (!map) { return this } - var prevLeftLayers = asArray(this._leftLayers) - var prevRightLayers = asArray(this._rightLayers) + var prevLeftLayers = this._leftLayers + var prevRightLayers = this._rightLayers if (!newLeftLayers) { - newLeftLayers = asArray(this._leftLayers) + newLeftLayers = prevLeftLayers } if (!newRightLayers) { - newRightLayers = asArray(this._rightLayers) + newRightLayers = prevRightLayers } + newLeftLayers = asArray(newLeftLayers) + newRightLayers = asArray(newRightLayers) + var that = this // Add new layers. applyToMissingLayers(map, newLeftLayers, prevLeftLayers, function (layer) { that.fire('leftlayeradd', {layer: layer}) })