From a7ce956e1f2f12c10608f630bbe4505dc96141f6 Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Sun, 29 Mar 2020 11:04:38 -0700 Subject: [PATCH] Don't use global selectors in street-level photo services (re: #7445) --- modules/services/mapillary.js | 38 ++++++++--------- modules/services/openstreetcam.js | 61 +++++++++++++-------------- modules/services/streetside.js | 43 +++++++++---------- modules/svg/mapillary_images.js | 10 ++--- modules/svg/mapillary_map_features.js | 6 +-- modules/svg/mapillary_signs.js | 6 +-- modules/svg/openstreetcam_images.js | 10 ++--- modules/svg/streetside.js | 8 ++-- test/spec/services/mapillary.js | 4 +- test/spec/services/openstreetcam.js | 4 +- 10 files changed, 94 insertions(+), 96 deletions(-) diff --git a/modules/services/mapillary.js b/modules/services/mapillary.js index bc800b9c1..7d0c560eb 100644 --- a/modules/services/mapillary.js +++ b/modules/services/mapillary.js @@ -1,6 +1,6 @@ /* global Mapillary:false */ import { dispatch as d3_dispatch } from 'd3-dispatch'; -import { select as d3_select, selectAll as d3_selectAll } from 'd3-selection'; +import { select as d3_select } from 'd3-selection'; import RBush from 'rbush'; @@ -419,8 +419,8 @@ export default { }, - showViewer: function() { - var wrap = d3_select('.photoviewer') + showViewer: function(context) { + var wrap = context.container().select('.photoviewer') .classed('hide', false); var isHidden = wrap.selectAll('.photo-wrapper.mly-wrapper.hide').size(); @@ -459,18 +459,18 @@ export default { context.container().selectAll('.viewfield-group, .sequence, .icon-detected') .classed('currentView', false); - return this.setStyles(null, true); + return this.setStyles(context, null, true); }, parsePagination: parsePagination, - updateViewer: function(imageKey, context) { + updateViewer: function(context, imageKey) { if (!imageKey) return this; if (!_mlyViewer) { - this.initViewer(imageKey, context); + this.initViewer(context, imageKey); } else { _mlyViewer.moveToKey(imageKey) .catch(function(e) { console.error('mly3', e); }); // eslint-disable-line no-console @@ -480,7 +480,7 @@ export default { }, - initViewer: function(imageKey, context) { + initViewer: function(context, imageKey) { var that = this; if (window.Mapillary && imageKey) { var opts = { @@ -537,12 +537,12 @@ export default { // If `node.key` matches the current _mlySelectedImageKey, call `selectImage()` // one more time to update the detections and attribution.. if (node.key === selectedKey) { - that.selectImage(_mlySelectedImageKey, true); + that.selectImage(context, _mlySelectedImageKey, true); } } else { // `nodechanged` initiated from the Mapillary viewer controls.. var loc = node.computedLatLon ? [node.computedLatLon.lon, node.computedLatLon.lat] : [node.latLon.lon, node.latLon.lat]; context.map().centerEase(loc); - that.selectImage(node.key, true); + that.selectImage(context, node.key, true); } } @@ -555,7 +555,7 @@ export default { // Pass in the image key string as `imageKey`. // This allows images to be selected from places that dont have access // to the full image datum (like the street signs layer or the js viewer) - selectImage: function(imageKey, fromViewer) { + selectImage: function(context, imageKey, fromViewer) { _mlySelectedImageKey = imageKey; @@ -563,7 +563,7 @@ export default { // There just might be a delay before user sees detections, captured_at, etc. var d = _mlyCache.images.forImageKey[imageKey]; - var viewer = d3_select('.photoviewer'); + var viewer = context.container().select('.photoviewer'); if (!viewer.empty()) viewer.datum(d); imageKey = (d && d.key) || imageKey; @@ -571,10 +571,10 @@ export default { _mlyClicks.push(imageKey); } - this.setStyles(null, true); + this.setStyles(context, null, true); // if signs signs are shown, highlight the ones that appear in this image - d3_selectAll('.layer-mapillary-signs .icon-detected') + context.container().selectAll('.layer-mapillary-signs .icon-detected') .classed('currentView', function(d) { return d.detections.some(function(detection) { return detection.image_key === imageKey; @@ -602,14 +602,14 @@ export default { // Updates the currently highlighted sequence and selected bubble. // Reset is only necessary when interacting with the viewport because // this implicitly changes the currently selected bubble/sequence - setStyles: function(hovered, reset) { + setStyles: function(context, hovered, reset) { if (reset) { // reset all layers - d3_selectAll('.viewfield-group') + context.container().selectAll('.viewfield-group') .classed('highlighted', false) .classed('hovered', false) .classed('currentView', false); - d3_selectAll('.sequence') + context.container().selectAll('.sequence') .classed('highlighted', false) .classed('currentView', false); } @@ -627,17 +627,17 @@ export default { // highlight sibling viewfields on either the selected or the hovered sequences var highlightedImageKeys = utilArrayUnion(hoveredImageKeys, selectedImageKeys); - d3_selectAll('.layer-mapillary .viewfield-group') + context.container().selectAll('.layer-mapillary .viewfield-group') .classed('highlighted', function(d) { return highlightedImageKeys.indexOf(d.key) !== -1; }) .classed('hovered', function(d) { return d.key === hoveredImageKey; }) .classed('currentView', function(d) { return d.key === selectedImageKey; }); - d3_selectAll('.layer-mapillary .sequence') + context.container().selectAll('.layer-mapillary .sequence') .classed('highlighted', function(d) { return d.properties.key === hoveredSequenceKey; }) .classed('currentView', function(d) { return d.properties.key === selectedSequenceKey; }); // update viewfields if needed - d3_selectAll('.viewfield-group .viewfield') + context.container().selectAll('.viewfield-group .viewfield') .attr('d', viewfieldPath); function viewfieldPath() { diff --git a/modules/services/openstreetcam.js b/modules/services/openstreetcam.js index ee96f9810..a03634078 100644 --- a/modules/services/openstreetcam.js +++ b/modules/services/openstreetcam.js @@ -1,6 +1,6 @@ import { dispatch as d3_dispatch } from 'd3-dispatch'; import { json as d3_json } from 'd3-fetch'; -import { event as d3_event, select as d3_select, selectAll as d3_selectAll } from 'd3-selection'; +import { event as d3_event } from 'd3-selection'; import { zoom as d3_zoom, zoomIdentity as d3_zoomIdentity } from 'd3-zoom'; import RBush from 'rbush'; @@ -18,8 +18,7 @@ var dispatch = d3_dispatch('loadedImages'); var imgZoom = d3_zoom() .extent([[0, 0], [320, 240]]) .translateExtent([[0, 0], [320, 240]]) - .scaleExtent([1, 15]) - .on('zoom', zoomPan); + .scaleExtent([1, 15]); var _oscCache; var _oscSelectedImage; @@ -171,13 +170,6 @@ function searchLimited(limit, projection, rtree) { } -function zoomPan() { - var t = d3_event.transform; - d3_select('.photoviewer .osc-image-wrap') - .call(utilSetTransform, t.x, t.y, t.k); -} - - export default { init: function() { @@ -247,14 +239,14 @@ export default { var that = this; // add osc-wrapper - var wrap = d3_select('.photoviewer').selectAll('.osc-wrapper') + var wrap = context.container().select('.photoviewer').selectAll('.osc-wrapper') .data([0]); var wrapEnter = wrap.enter() .append('div') .attr('class', 'photo-wrapper osc-wrapper') .classed('hide', true) - .call(imgZoom) + .call(imgZoom.on('zoom', zoomPan)) .on('dblclick.zoom', null); wrapEnter @@ -302,6 +294,13 @@ export default { }); + function zoomPan() { + var t = d3_event.transform; + context.container().select('.photoviewer .osc-image-wrap') + .call(utilSetTransform, t.x, t.y, t.k); + } + + function rotate(deg) { return function() { if (!_oscSelectedImage) return; @@ -316,7 +315,7 @@ export default { if (r < -180) r += 360; sequence.rotation = r; - var wrap = d3_select('.photoviewer .osc-wrapper'); + var wrap = context.container().select('.photoviewer .osc-wrapper'); wrap .transition() @@ -344,15 +343,15 @@ export default { context.map().centerEase(nextImage.loc); that - .selectImage(nextImage) - .updateViewer(nextImage); + .selectImage(context, nextImage) + .updateViewer(context, nextImage); }; } }, - showViewer: function() { - var viewer = d3_select('.photoviewer') + showViewer: function(context) { + var viewer = context.container().select('.photoviewer') .classed('hide', false); var isHidden = viewer.selectAll('.photo-wrapper.osc-wrapper.hide').size(); @@ -385,12 +384,12 @@ export default { context.container().selectAll('.viewfield-group, .sequence, .icon-sign') .classed('currentView', false); - return this.setStyles(null, true); + return this.setStyles(context, null, true); }, - updateViewer: function(d) { - var wrap = d3_select('.photoviewer .osc-wrapper'); + updateViewer: function(context, d) { + var wrap = context.container().select('.photoviewer .osc-wrapper'); var imageWrap = wrap.selectAll('.osc-image-wrap'); var attribution = wrap.selectAll('.photo-attribution').html(''); @@ -459,14 +458,14 @@ export default { }, - selectImage: function(d) { + selectImage: function(context, d) { _oscSelectedImage = d; - var viewer = d3_select('.photoviewer'); + var viewer = context.container().select('.photoviewer'); if (!viewer.empty()) viewer.datum(d); - this.setStyles(null, true); + this.setStyles(context, null, true); - d3_selectAll('.icon-sign') + context.container().selectAll('.icon-sign') .classed('currentView', false); return this; @@ -486,14 +485,14 @@ export default { // Updates the currently highlighted sequence and selected bubble. // Reset is only necessary when interacting with the viewport because // this implicitly changes the currently selected bubble/sequence - setStyles: function(hovered, reset) { + setStyles: function(context, hovered, reset) { if (reset) { // reset all layers - d3_selectAll('.viewfield-group') + context.container().selectAll('.viewfield-group') .classed('highlighted', false) .classed('hovered', false) .classed('currentView', false); - d3_selectAll('.sequence') + context.container().selectAll('.sequence') .classed('highlighted', false) .classed('currentView', false); } @@ -503,7 +502,7 @@ export default { var hoveredSequence = hoveredSequenceKey && _oscCache.sequences[hoveredSequenceKey]; var hoveredImageKeys = (hoveredSequence && hoveredSequence.images.map(function (d) { return d.key; })) || []; - var viewer = d3_select('.photoviewer'); + var viewer = context.container().select('.photoviewer'); var selected = viewer.empty() ? undefined : viewer.datum(); var selectedImageKey = selected && selected.key; var selectedSequenceKey = this.getSequenceKeyForImage(selected); @@ -513,17 +512,17 @@ export default { // highlight sibling viewfields on either the selected or the hovered sequences var highlightedImageKeys = utilArrayUnion(hoveredImageKeys, selectedImageKeys); - d3_selectAll('.layer-openstreetcam .viewfield-group') + context.container().selectAll('.layer-openstreetcam .viewfield-group') .classed('highlighted', function(d) { return highlightedImageKeys.indexOf(d.key) !== -1; }) .classed('hovered', function(d) { return d.key === hoveredImageKey; }) .classed('currentView', function(d) { return d.key === selectedImageKey; }); - d3_selectAll('.layer-openstreetcam .sequence') + context.container().selectAll('.layer-openstreetcam .sequence') .classed('highlighted', function(d) { return d.properties.key === hoveredSequenceKey; }) .classed('currentView', function(d) { return d.properties.key === selectedSequenceKey; }); // update viewfields if needed - d3_selectAll('.viewfield-group .viewfield') + context.container().selectAll('.viewfield-group .viewfield') .attr('d', viewfieldPath); function viewfieldPath() { diff --git a/modules/services/streetside.js b/modules/services/streetside.js index 794f3f08b..cb9beff03 100644 --- a/modules/services/streetside.js +++ b/modules/services/streetside.js @@ -3,8 +3,7 @@ import { timer as d3_timer } from 'd3-timer'; import { event as d3_event, - select as d3_select, - selectAll as d3_selectAll + select as d3_select } from 'd3-selection'; import RBush from 'rbush'; @@ -500,7 +499,7 @@ export default { let that = this; // create ms-wrapper, a photo wrapper class - let wrap = d3_select('.photoviewer').selectAll('.ms-wrapper') + let wrap = context.container().select('.photoviewer').selectAll('.ms-wrapper') .data([0]); // inject ms-wrapper into the photoviewer div @@ -567,7 +566,7 @@ export default { function step(stepBy) { return () => { - let viewer = d3_select('.photoviewer'); + let viewer = context.container().select('.photoviewer'); let selected = viewer.empty() ? undefined : viewer.datum(); if (!selected) return; @@ -630,11 +629,11 @@ export default { context.map().centerEase(nextBubble.loc); - that.selectImage(nextBubble) + that.selectImage(context, nextBubble) .then(response => { if (response.status === 'ok') { _sceneOptions.yaw = yaw; - that.showViewer(); + that.showViewer(context); } }); }; @@ -645,7 +644,7 @@ export default { /** * showViewer() */ - showViewer: function(yaw) { + showViewer: function(context, yaw) { if (!_sceneOptions) return; if (yaw !== undefined) { @@ -669,7 +668,7 @@ export default { } } - let wrap = d3_select('.photoviewer') + let wrap = context.container().select('.photoviewer') .classed('hide', false); let isHidden = wrap.selectAll('.photo-wrapper.ms-wrapper.hide').size(); @@ -703,21 +702,21 @@ export default { context.container().selectAll('.viewfield-group, .sequence, .icon-sign') .classed('currentView', false); - return this.setStyles(null, true); + return this.setStyles(context, null, true); }, /** * selectImage(). */ - selectImage: function (d) { + selectImage: function (context, d) { let that = this; - let viewer = d3_select('.photoviewer'); + let viewer = context.container().select('.photoviewer'); if (!viewer.empty()) viewer.datum(d); - this.setStyles(null, true); + this.setStyles(context, null, true); - let wrap = d3_select('.photoviewer .ms-wrapper'); + let wrap = context.container().select('.photoviewer .ms-wrapper'); let attribution = wrap.selectAll('.photo-attribution').html(''); wrap.selectAll('.pnlm-load-box') // display "loading.." @@ -757,11 +756,11 @@ export default { hfov: _pannellumViewer.getHfov() }; - that.selectImage(d) + that.selectImage(context, d) .then(response => { if (response.status === 'ok') { _sceneOptions = Object.assign(_sceneOptions, viewstate); - that.showViewer(); + that.showViewer(context); } }); }); @@ -879,14 +878,14 @@ export default { // Updates the currently highlighted sequence and selected bubble. // Reset is only necessary when interacting with the viewport because // this implicitly changes the currently selected bubble/sequence - setStyles: function (hovered, reset) { + setStyles: function (context, hovered, reset) { if (reset) { // reset all layers - d3_selectAll('.viewfield-group') + context.container().selectAll('.viewfield-group') .classed('highlighted', false) .classed('hovered', false) .classed('currentView', false); - d3_selectAll('.sequence') + context.container().selectAll('.sequence') .classed('highlighted', false) .classed('currentView', false); } @@ -896,7 +895,7 @@ export default { let hoveredSequence = hoveredSequenceKey && _ssCache.sequences[hoveredSequenceKey]; let hoveredBubbleKeys = (hoveredSequence && hoveredSequence.bubbles.map(d => d.key)) || []; - let viewer = d3_select('.photoviewer'); + let viewer = context.container().select('.photoviewer'); let selected = viewer.empty() ? undefined : viewer.datum(); let selectedBubbleKey = selected && selected.key; let selectedSequenceKey = this.getSequenceKeyForBubble(selected); @@ -906,17 +905,17 @@ export default { // highlight sibling viewfields on either the selected or the hovered sequences let highlightedBubbleKeys = utilArrayUnion(hoveredBubbleKeys, selectedBubbleKeys); - d3_selectAll('.layer-streetside-images .viewfield-group') + context.container().selectAll('.layer-streetside-images .viewfield-group') .classed('highlighted', d => highlightedBubbleKeys.indexOf(d.key) !== -1) .classed('hovered', d => d.key === hoveredBubbleKey) .classed('currentView', d => d.key === selectedBubbleKey); - d3_selectAll('.layer-streetside-images .sequence') + context.container().selectAll('.layer-streetside-images .sequence') .classed('highlighted', d => d.properties.key === hoveredSequenceKey) .classed('currentView', d => d.properties.key === selectedSequenceKey); // update viewfields if needed - d3_selectAll('.viewfield-group .viewfield') + context.container().selectAll('.viewfield-group .viewfield') .attr('d', viewfieldPath); function viewfieldPath() { diff --git a/modules/svg/mapillary_images.js b/modules/svg/mapillary_images.js index 23ca14262..83f66bf76 100644 --- a/modules/svg/mapillary_images.js +++ b/modules/svg/mapillary_images.js @@ -89,9 +89,9 @@ export function svgMapillaryImages(projection, context, dispatch) { if (!service) return; service - .selectImage(d.key) - .updateViewer(d.key, context) - .showViewer(); + .selectImage(context, d.key) + .updateViewer(context, d.key) + .showViewer(context); context.map().centerEase(d.loc); } @@ -99,13 +99,13 @@ export function svgMapillaryImages(projection, context, dispatch) { function mouseover(d) { var service = getService(); - if (service) service.setStyles(d); + if (service) service.setStyles(context, d); } function mouseout() { var service = getService(); - if (service) service.setStyles(null); + if (service) service.setStyles(context, null); } diff --git a/modules/svg/mapillary_map_features.js b/modules/svg/mapillary_map_features.js index 3d7a5ab33..df48a192a 100644 --- a/modules/svg/mapillary_map_features.js +++ b/modules/svg/mapillary_map_features.js @@ -72,9 +72,9 @@ export function svgMapillaryMapFeatures(projection, context, dispatch) { }); service - .selectImage(imageKey) - .updateViewer(imageKey, context) - .showViewer(); + .selectImage(context, imageKey) + .updateViewer(context, imageKey) + .showViewer(context); } diff --git a/modules/svg/mapillary_signs.js b/modules/svg/mapillary_signs.js index 7b2c697ac..127566738 100644 --- a/modules/svg/mapillary_signs.js +++ b/modules/svg/mapillary_signs.js @@ -72,9 +72,9 @@ export function svgMapillarySigns(projection, context, dispatch) { }); service - .selectImage(imageKey) - .updateViewer(imageKey, context) - .showViewer(); + .selectImage(context, imageKey) + .updateViewer(context, imageKey) + .showViewer(context); } diff --git a/modules/svg/openstreetcam_images.js b/modules/svg/openstreetcam_images.js index 40df2e7b4..1db4d6e22 100644 --- a/modules/svg/openstreetcam_images.js +++ b/modules/svg/openstreetcam_images.js @@ -74,9 +74,9 @@ export function svgOpenstreetcamImages(projection, context, dispatch) { if (!service) return; service - .selectImage(d) - .updateViewer(d) - .showViewer(); + .selectImage(context, d) + .updateViewer(context, d) + .showViewer(context); context.map().centerEase(d.loc); } @@ -84,13 +84,13 @@ export function svgOpenstreetcamImages(projection, context, dispatch) { function mouseover(d) { var service = getService(); - if (service) service.setStyles(d); + if (service) service.setStyles(context, d); } function mouseout() { var service = getService(); - if (service) service.setStyles(null); + if (service) service.setStyles(context, null); } diff --git a/modules/svg/streetside.js b/modules/svg/streetside.js index 81a0b9cff..023d333e0 100644 --- a/modules/svg/streetside.js +++ b/modules/svg/streetside.js @@ -98,10 +98,10 @@ export function svgStreetside(projection, context, dispatch) { _selectedSequence = d.sequenceKey; service - .selectImage(d) + .selectImage(context, d) .then(response => { if (response.status === 'ok'){ - service.showViewer(_viewerYaw); + service.showViewer(context, _viewerYaw); } }); @@ -114,7 +114,7 @@ export function svgStreetside(projection, context, dispatch) { */ function mouseover(d) { var service = getService(); - if (service) service.setStyles(d); + if (service) service.setStyles(context, d); } /** @@ -122,7 +122,7 @@ export function svgStreetside(projection, context, dispatch) { */ function mouseout() { var service = getService(); - if (service) service.setStyles(null); + if (service) service.setStyles(context, null); } /** diff --git a/test/spec/services/mapillary.js b/test/spec/services/mapillary.js index 8ff8c5fdd..e7faf5d28 100644 --- a/test/spec/services/mapillary.js +++ b/test/spec/services/mapillary.js @@ -45,7 +45,7 @@ describe('iD.serviceMapillary', function() { describe('#reset', function() { it('resets cache and image', function() { mapillary.cache().foo = 'bar'; - mapillary.selectImage({ key: 'baz', loc: [10,0] }); + mapillary.selectImage(context, { key: 'baz', loc: [10,0] }); mapillary.reset(); expect(mapillary.cache()).to.not.have.property('foo'); @@ -354,7 +354,7 @@ describe('iD.serviceMapillary', function() { describe('#selectImage', function() { it('gets and sets the selected image', function() { var d = { key: 'baz', loc: [10,0] }; - mapillary.selectImage(d.key); + mapillary.selectImage(context, d.key); expect(mapillary.getSelectedImageKey()).to.eql(d.key); }); }); diff --git a/test/spec/services/openstreetcam.js b/test/spec/services/openstreetcam.js index e53218fdf..6f01788cf 100644 --- a/test/spec/services/openstreetcam.js +++ b/test/spec/services/openstreetcam.js @@ -42,7 +42,7 @@ describe('iD.serviceOpenstreetcam', function() { describe('#reset', function() { it('resets cache and image', function() { openstreetcam.cache().foo = 'bar'; - openstreetcam.selectImage({key: 'baz'}); + openstreetcam.selectImage(context, {key: 'baz'}); openstreetcam.reset(); expect(openstreetcam.cache()).to.not.have.property('foo'); @@ -260,7 +260,7 @@ describe('iD.serviceOpenstreetcam', function() { describe('#selectedImage', function() { it('sets and gets selected image', function() { - openstreetcam.selectImage('foo'); + openstreetcam.selectImage(context, 'foo'); expect(openstreetcam.getSelectedImage()).to.eql('foo'); }); });