From 0d5bd8b2771be815ccd631c179aba0fcca528812 Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Wed, 11 Dec 2019 17:15:36 -0500 Subject: [PATCH] Store selected mapillary image by its key rather than its object Properly sort selected mapillary signs and features above unselected ones --- modules/services/mapillary.js | 37 ++++++++++++--------------- modules/svg/mapillary_images.js | 2 +- modules/svg/mapillary_map_features.js | 27 +++++++++++-------- modules/svg/mapillary_signs.js | 27 +++++++++++-------- test/spec/services/mapillary.js | 6 ++--- 5 files changed, 55 insertions(+), 44 deletions(-) diff --git a/modules/services/mapillary.js b/modules/services/mapillary.js index d162130db..5e4fee806 100644 --- a/modules/services/mapillary.js +++ b/modules/services/mapillary.js @@ -45,7 +45,7 @@ var dispatch = d3_dispatch('loadedImages', 'loadedSigns', 'loadedMapFeatures', ' var _mlyFallback = false; var _mlyCache; var _mlyClicks; -var _mlySelectedImage; +var _mlySelectedImageKey; var _mlyViewer; @@ -299,7 +299,7 @@ export default { sequences: { inflight: {}, loaded: {}, nextPage: {}, nextURL: {}, rtree: new RBush(), forImageKey: {}, lineString: {} } }; - _mlySelectedImage = null; + _mlySelectedImageKey = null; _mlyClicks = []; }, @@ -441,7 +441,7 @@ export default { hideViewer: function() { - _mlySelectedImage = null; + _mlySelectedImageKey = null; if (!_mlyFallback && _mlyViewer) { _mlyViewer.getComponent('sequence').stop(); @@ -529,19 +529,19 @@ export default { var clicks = _mlyClicks; var index = clicks.indexOf(node.key); - var selectedKey = _mlySelectedImage && _mlySelectedImage.key; + var selectedKey = _mlySelectedImageKey; if (index > -1) { // `nodechanged` initiated from clicking on a marker.. clicks.splice(index, 1); // remove the click - // If `node.key` matches the current _mlySelectedImage, call `selectImage()` + // If `node.key` matches the current _mlySelectedImageKey, call `selectImage()` // one more time to update the detections and attribution.. if (node.key === selectedKey) { - that.selectImage(_mlySelectedImage, node.key, true); + that.selectImage(_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(undefined, node.key, true); + that.selectImage(node.key, true); } } @@ -551,20 +551,17 @@ export default { }, - // Pass the image datum itself in `d` or the `imageKey` string. + // 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(d, imageKey, fromViewer) { - if (!d && imageKey) { - // If the user clicked on something that's not an image marker, we - // might get in here.. Cache lookup can fail, e.g. if the user - // clicked a streetsign, but images are loading slowly asynchronously. - // We'll try to carry on anyway if there is no datum. There just - // might be a delay before user sees detections, captured_at, etc. - d = _mlyCache.images.forImageKey[imageKey]; - } + selectImage: function(imageKey, fromViewer) { + + _mlySelectedImageKey = imageKey; + + // Note the datum could be missing, but we'll try to carry on anyway. + // There just might be a delay before user sees detections, captured_at, etc. + var d = _mlyCache.images.forImageKey[imageKey]; - _mlySelectedImage = d; var viewer = d3_select('#photoviewer'); if (!viewer.empty()) viewer.datum(d); @@ -591,8 +588,8 @@ export default { }, - getSelectedImage: function() { - return _mlySelectedImage; + getSelectedImageKey: function() { + return _mlySelectedImageKey; }, diff --git a/modules/svg/mapillary_images.js b/modules/svg/mapillary_images.js index 38b048a67..a7f3b7932 100644 --- a/modules/svg/mapillary_images.js +++ b/modules/svg/mapillary_images.js @@ -89,7 +89,7 @@ export function svgMapillaryImages(projection, context, dispatch) { if (!service) return; service - .selectImage(d) + .selectImage(d.key) .updateViewer(d.key, context) .showViewer(); diff --git a/modules/svg/mapillary_map_features.js b/modules/svg/mapillary_map_features.js index 4554ee910..3d7a5ab33 100644 --- a/modules/svg/mapillary_map_features.js +++ b/modules/svg/mapillary_map_features.js @@ -60,8 +60,7 @@ export function svgMapillaryMapFeatures(projection, context, dispatch) { context.map().centerEase(d.loc); - var selected = service.getSelectedImage(); - var selectedImageKey = selected && selected.key; + var selectedImageKey = service.getSelectedImageKey(); var imageKey; // Pick one of the images the map feature was detected in, @@ -73,7 +72,7 @@ export function svgMapillaryMapFeatures(projection, context, dispatch) { }); service - .selectImage(null, imageKey) + .selectImage(imageKey) .updateViewer(imageKey, context) .showViewer(); } @@ -82,8 +81,7 @@ export function svgMapillaryMapFeatures(projection, context, dispatch) { function update() { var service = getService(); var data = (service ? service.mapFeatures(projection) : []); - var selected = service && service.getSelectedImage(); - var selectedImageKey = selected && selected.key; + var selectedImageKey = service && service.getSelectedImageKey(); var transform = svgPointTransform(projection); var mapFeatures = layer.selectAll('.icon-map-feature') @@ -130,16 +128,25 @@ export function svgMapillaryMapFeatures(projection, context, dispatch) { // update mapFeatures .merge(enter) - .sort(function(a, b) { - return (a === selected) ? 1 - : (b === selected) ? -1 - : b.loc[1] - a.loc[1]; // sort Y - }) .attr('transform', transform) .classed('currentView', function(d) { return d.detections.some(function(detection) { return detection.image_key === selectedImageKey; }); + }) + .sort(function(a, b) { + var aSelected = a.detections.some(function(detection) { + return detection.image_key === selectedImageKey; + }); + var bSelected = b.detections.some(function(detection) { + return detection.image_key === selectedImageKey; + }); + if (aSelected === bSelected) { + return b.loc[1] - a.loc[1]; // sort Y + } else if (aSelected) { + return 1; + } + return -1; }); } diff --git a/modules/svg/mapillary_signs.js b/modules/svg/mapillary_signs.js index e12a6c1dc..7b2c697ac 100644 --- a/modules/svg/mapillary_signs.js +++ b/modules/svg/mapillary_signs.js @@ -60,8 +60,7 @@ export function svgMapillarySigns(projection, context, dispatch) { context.map().centerEase(d.loc); - var selected = service.getSelectedImage(); - var selectedImageKey = selected && selected.key; + var selectedImageKey = service.getSelectedImageKey(); var imageKey; // Pick one of the images the sign was detected in, @@ -73,7 +72,7 @@ export function svgMapillarySigns(projection, context, dispatch) { }); service - .selectImage(null, imageKey) + .selectImage(imageKey) .updateViewer(imageKey, context) .showViewer(); } @@ -82,8 +81,7 @@ export function svgMapillarySigns(projection, context, dispatch) { function update() { var service = getService(); var data = (service ? service.signs(projection) : []); - var selected = service && service.getSelectedImage(); - var selectedImageKey = selected && selected.key; + var selectedImageKey = service.getSelectedImageKey(); var transform = svgPointTransform(projection); var signs = layer.selectAll('.icon-sign') @@ -117,16 +115,25 @@ export function svgMapillarySigns(projection, context, dispatch) { // update signs .merge(enter) - .sort(function(a, b) { - return (a === selected) ? 1 - : (b === selected) ? -1 - : b.loc[1] - a.loc[1]; // sort Y - }) .attr('transform', transform) .classed('currentView', function(d) { return d.detections.some(function(detection) { return detection.image_key === selectedImageKey; }); + }) + .sort(function(a, b) { + var aSelected = a.detections.some(function(detection) { + return detection.image_key === selectedImageKey; + }); + var bSelected = b.detections.some(function(detection) { + return detection.image_key === selectedImageKey; + }); + if (aSelected === bSelected) { + return b.loc[1] - a.loc[1]; // sort Y + } else if (aSelected) { + return 1; + } + return -1; }); } diff --git a/test/spec/services/mapillary.js b/test/spec/services/mapillary.js index dcb6bcf8d..e5ab0c3e7 100644 --- a/test/spec/services/mapillary.js +++ b/test/spec/services/mapillary.js @@ -49,7 +49,7 @@ describe('iD.serviceMapillary', function() { mapillary.reset(); expect(mapillary.cache()).to.not.have.property('foo'); - expect(mapillary.getSelectedImage()).to.be.null; + expect(mapillary.getSelectedImageKey()).to.be.null; }); }); @@ -354,8 +354,8 @@ 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); - expect(mapillary.getSelectedImage()).to.eql(d); + mapillary.selectImage(d.key); + expect(mapillary.getSelectedImageKey()).to.eql(d.key); }); });