-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Better geo streaming #324
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Better geo streaming #324
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -125,99 +125,101 @@ plotScatterGeo.plot = function(geo, scattergeoData) { | |
|
||
gScatterGeoTraces.exit().remove(); | ||
|
||
// TODO add hover - how? | ||
gScatterGeoTraces | ||
.each(function(trace) { | ||
if(!subTypes.hasLines(trace)) return; | ||
// TODO find a way to order the inner nodes on update | ||
gScatterGeoTraces.selectAll('*').remove(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have a few ideas that would allow us to remove this dirty purge call. See: for some examples. But I want to benchmark these patterns before implementing them in the code. So I opted for the easy route in this PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will we have a performance hit removing these on each call to Edit: Didn't refresh for too long... |
||
|
||
d3.select(this) | ||
.append('path') | ||
.datum(makeLineGeoJSON(trace)) | ||
.attr('class', 'js-line'); | ||
}); | ||
gScatterGeoTraces.each(function(trace) { | ||
var s = d3.select(this); | ||
|
||
gScatterGeoTraces.append('g') | ||
.attr('class', 'points') | ||
.each(function(trace) { | ||
var s = d3.select(this), | ||
showMarkers = subTypes.hasMarkers(trace), | ||
showText = subTypes.hasText(trace); | ||
|
||
if((!showMarkers && !showText)) return; | ||
|
||
var cdi = plotScatterGeo.calcGeoJSON(trace, geo.topojson), | ||
cleanHoverLabelsFunc = makeCleanHoverLabelsFunc(geo, trace), | ||
eventDataFunc = makeEventDataFunc(trace); | ||
|
||
var hoverinfo = trace.hoverinfo, | ||
hasNameLabel = ( | ||
hoverinfo === 'all' || | ||
hoverinfo.indexOf('name') !== -1 | ||
); | ||
|
||
function handleMouseOver(pt, ptIndex) { | ||
if(!geo.showHover) return; | ||
|
||
var xy = geo.projection([pt.lon, pt.lat]); | ||
cleanHoverLabelsFunc(pt); | ||
|
||
Fx.loneHover({ | ||
x: xy[0], | ||
y: xy[1], | ||
name: hasNameLabel ? trace.name : undefined, | ||
text: pt.textLabel, | ||
color: pt.mc || (trace.marker || {}).color | ||
}, { | ||
container: geo.hoverContainer.node() | ||
}); | ||
|
||
geo.graphDiv.emit('plotly_hover', eventDataFunc(pt, ptIndex)); | ||
} | ||
|
||
function handleClick(pt, ptIndex) { | ||
geo.graphDiv.emit('plotly_click', eventDataFunc(pt, ptIndex)); | ||
} | ||
|
||
if(showMarkers) { | ||
s.selectAll('path.point') | ||
.data(cdi) | ||
.enter().append('path') | ||
.attr('class', 'point') | ||
.on('mouseover', handleMouseOver) | ||
.on('click', handleClick) | ||
.on('mouseout', function() { | ||
Fx.loneUnhover(geo.hoverContainer); | ||
}) | ||
.on('mousedown', function() { | ||
// to simulate the 'zoomon' event | ||
Fx.loneUnhover(geo.hoverContainer); | ||
}) | ||
.on('mouseup', handleMouseOver); // ~ 'zoomend' | ||
} | ||
|
||
if(showText) { | ||
s.selectAll('g') | ||
.data(cdi) | ||
.enter().append('g') | ||
.append('text'); | ||
} | ||
}); | ||
if(!subTypes.hasLines(trace)) return; | ||
|
||
s.selectAll('path.js-line') | ||
.data([makeLineGeoJSON(trace)]) | ||
.enter().append('path') | ||
.classed('js-line', true); | ||
|
||
// TODO add hover - how? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this not handled on line 188? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Line 188 handles hover for markers, not lines unfortunately. That will come when someone has the guts to refactor |
||
}); | ||
|
||
gScatterGeoTraces.each(function(trace) { | ||
var s = d3.select(this), | ||
showMarkers = subTypes.hasMarkers(trace), | ||
showText = subTypes.hasText(trace); | ||
|
||
if(!showMarkers && !showText) return; | ||
|
||
var cdi = plotScatterGeo.calcGeoJSON(trace, geo.topojson), | ||
cleanHoverLabelsFunc = makeCleanHoverLabelsFunc(geo, trace), | ||
eventDataFunc = makeEventDataFunc(trace); | ||
|
||
var hoverinfo = trace.hoverinfo, | ||
hasNameLabel = ( | ||
hoverinfo === 'all' || | ||
hoverinfo.indexOf('name') !== -1 | ||
); | ||
|
||
function handleMouseOver(pt, ptIndex) { | ||
if(!geo.showHover) return; | ||
|
||
var xy = geo.projection([pt.lon, pt.lat]); | ||
cleanHoverLabelsFunc(pt); | ||
|
||
Fx.loneHover({ | ||
x: xy[0], | ||
y: xy[1], | ||
name: hasNameLabel ? trace.name : undefined, | ||
text: pt.textLabel, | ||
color: pt.mc || (trace.marker || {}).color | ||
}, { | ||
container: geo.hoverContainer.node() | ||
}); | ||
|
||
geo.graphDiv.emit('plotly_hover', eventDataFunc(pt, ptIndex)); | ||
} | ||
|
||
function handleClick(pt, ptIndex) { | ||
geo.graphDiv.emit('plotly_click', eventDataFunc(pt, ptIndex)); | ||
} | ||
|
||
if(showMarkers) { | ||
s.selectAll('path.point').data(cdi) | ||
.enter().append('path') | ||
.classed('point', true) | ||
.on('mouseover', handleMouseOver) | ||
.on('click', handleClick) | ||
.on('mouseout', function() { | ||
Fx.loneUnhover(geo.hoverContainer); | ||
}) | ||
.on('mousedown', function() { | ||
// to simulate the 'zoomon' event | ||
Fx.loneUnhover(geo.hoverContainer); | ||
}) | ||
.on('mouseup', handleMouseOver); // ~ 'zoomend' | ||
} | ||
|
||
if(showText) { | ||
s.selectAll('g').data(cdi) | ||
.enter().append('g') | ||
.append('text'); | ||
} | ||
}); | ||
|
||
plotScatterGeo.style(geo); | ||
}; | ||
|
||
plotScatterGeo.style = function(geo) { | ||
var selection = geo.framework.selectAll('g.trace.scattergeo'); | ||
|
||
selection.style('opacity', function(trace) { return trace.opacity; }); | ||
selection.style('opacity', function(trace) { | ||
return trace.opacity; | ||
}); | ||
|
||
selection.selectAll('g.points') | ||
.each(function(trace) { | ||
d3.select(this).selectAll('path.point') | ||
.call(Drawing.pointStyle, trace); | ||
d3.select(this).selectAll('text') | ||
.call(Drawing.textPointStyle, trace); | ||
}); | ||
selection.each(function(trace) { | ||
d3.select(this).selectAll('path.point') | ||
.call(Drawing.pointStyle, trace); | ||
d3.select(this).selectAll('text') | ||
.call(Drawing.textPointStyle, trace); | ||
}); | ||
|
||
// GeoJSON calc data is incompatible with Drawing.lineGroupStyle | ||
selection.selectAll('path.js-line') | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
var d3 = require('d3'); | ||
|
||
var Plotly = require('@lib/index'); | ||
var Lib = require('@src/lib'); | ||
|
||
var createGraphDiv = require('../assets/create_graph_div'); | ||
var destroyGraphDiv = require('../assets/destroy_graph_div'); | ||
|
@@ -38,7 +39,10 @@ describe('Test geo interactions', function() { | |
|
||
beforeEach(function(done) { | ||
gd = createGraphDiv(); | ||
Plotly.plot(gd, mock.data, mock.layout).then(done); | ||
|
||
var mockCopy = Lib.extendDeep({}, mock); | ||
|
||
Plotly.plot(gd, mockCopy.data, mockCopy.layout).then(done); | ||
}); | ||
|
||
describe('scattergeo hover labels', function() { | ||
|
@@ -261,5 +265,221 @@ describe('Test geo interactions', function() { | |
}); | ||
}); | ||
|
||
describe('streaming calls', function() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @bpostlethwaite can you review ⬇️ ? |
||
var INTERVAL = 10; | ||
|
||
var N_MARKERS_AT_START = Math.min( | ||
mock.data[0].lat.length, | ||
mock.data[0].lon.length | ||
); | ||
|
||
var N_LOCATIONS_AT_START = mock.data[1].locations.length; | ||
|
||
var lonQueue = [45, -45, 12, 20], | ||
latQueue = [-75, 80, 5, 10], | ||
textQueue = ['c', 'd', 'e', 'f'], | ||
locationsQueue = ['AUS', 'FRA', 'DEU', 'MEX'], | ||
zQueue = [100, 20, 30, 12]; | ||
|
||
beforeEach(function(done) { | ||
var update = { | ||
mode: 'lines+markers+text', | ||
text: [['a', 'b']], | ||
'marker.size': 10 | ||
}; | ||
|
||
Plotly.restyle(gd, update, [0]).then(done); | ||
}); | ||
|
||
function countScatterGeoLines() { | ||
return d3.selectAll('g.trace.scattergeo') | ||
.selectAll('path.js-line') | ||
.size(); | ||
} | ||
|
||
function countScatterGeoMarkers() { | ||
return d3.selectAll('g.trace.scattergeo') | ||
.selectAll('path.point') | ||
.size(); | ||
} | ||
|
||
function countScatterGeoTextGroups() { | ||
return d3.selectAll('g.trace.scattergeo') | ||
.selectAll('g') | ||
.size(); | ||
} | ||
|
||
function countScatterGeoTextNodes() { | ||
return d3.selectAll('g.trace.scattergeo') | ||
.selectAll('g') | ||
.select('text') | ||
.size(); | ||
} | ||
|
||
function checkScatterGeoOrder() { | ||
var order = ['js-path', 'point', null]; | ||
var nodes = d3.selectAll('g.trace.scattergeo'); | ||
|
||
nodes.each(function() { | ||
var list = []; | ||
|
||
d3.select(this).selectAll('*').each(function() { | ||
var className = d3.select(this).attr('class'); | ||
list.push(className); | ||
}); | ||
|
||
var listSorted = list.slice().sort(function(a, b) { | ||
return order.indexOf(a) - order.indexOf(b); | ||
}); | ||
|
||
expect(list).toEqual(listSorted); | ||
}); | ||
} | ||
|
||
function countChoroplethPaths() { | ||
return d3.selectAll('g.trace.choropleth') | ||
.selectAll('path.choroplethlocation') | ||
.size(); | ||
} | ||
|
||
it('should be able to add line/marker/text nodes', function(done) { | ||
var i = 0; | ||
|
||
var interval = setInterval(function() { | ||
expect(countTraces('scattergeo')).toBe(1); | ||
expect(countTraces('choropleth')).toBe(1); | ||
expect(countScatterGeoLines()).toBe(1); | ||
expect(countScatterGeoMarkers()).toBe(N_MARKERS_AT_START + i); | ||
expect(countScatterGeoTextGroups()).toBe(N_MARKERS_AT_START + i); | ||
expect(countScatterGeoTextNodes()).toBe(N_MARKERS_AT_START + i); | ||
checkScatterGeoOrder(); | ||
|
||
var trace = gd.data[0]; | ||
trace.lon.push(lonQueue[i]); | ||
trace.lat.push(latQueue[i]); | ||
trace.text.push(textQueue[i]); | ||
|
||
if(i === lonQueue.length - 1) { | ||
clearInterval(interval); | ||
done(); | ||
} | ||
|
||
Plotly.plot(gd); | ||
i++; | ||
}, INTERVAL); | ||
}); | ||
|
||
it('should be able to shift line/marker/text nodes', function(done) { | ||
var i = 0; | ||
|
||
var interval = setInterval(function() { | ||
expect(countTraces('scattergeo')).toBe(1); | ||
expect(countTraces('choropleth')).toBe(1); | ||
expect(countScatterGeoLines()).toBe(1); | ||
expect(countScatterGeoMarkers()).toBe(N_MARKERS_AT_START); | ||
expect(countScatterGeoTextGroups()).toBe(N_MARKERS_AT_START); | ||
expect(countScatterGeoTextNodes()).toBe(N_MARKERS_AT_START); | ||
checkScatterGeoOrder(); | ||
|
||
var trace = gd.data[0]; | ||
trace.lon.push(lonQueue[i]); | ||
trace.lat.push(latQueue[i]); | ||
trace.text.push(textQueue[i]); | ||
trace.lon.shift(); | ||
trace.lat.shift(); | ||
trace.text.shift(); | ||
|
||
if(i === lonQueue.length - 1) { | ||
clearInterval(interval); | ||
done(); | ||
} | ||
|
||
Plotly.plot(gd); | ||
i++; | ||
}, INTERVAL); | ||
}); | ||
|
||
it('should be able to update line/marker/text nodes', function(done) { | ||
var i = 0; | ||
|
||
var interval = setInterval(function() { | ||
expect(countTraces('scattergeo')).toBe(1); | ||
expect(countTraces('choropleth')).toBe(1); | ||
expect(countScatterGeoLines()).toBe(1); | ||
expect(countScatterGeoMarkers()).toBe(N_MARKERS_AT_START); | ||
expect(countScatterGeoTextGroups()).toBe(N_MARKERS_AT_START); | ||
expect(countScatterGeoTextNodes()).toBe(N_MARKERS_AT_START); | ||
checkScatterGeoOrder(); | ||
|
||
var trace = gd.data[0]; | ||
trace.lon.push(lonQueue[i]); | ||
trace.lat.push(latQueue[i]); | ||
trace.text.push(textQueue[i]); | ||
trace.lon.shift(); | ||
trace.lat.shift(); | ||
trace.text.shift(); | ||
|
||
if(i === lonQueue.length - 1) { | ||
clearInterval(interval); | ||
done(); | ||
} | ||
|
||
Plotly.plot(gd); | ||
i++; | ||
}, INTERVAL); | ||
}); | ||
|
||
it('should be able to delete line/marker/text nodes and choropleth paths', function(done) { | ||
var trace0 = gd.data[0]; | ||
trace0.lon.shift(); | ||
trace0.lat.shift(); | ||
trace0.text.shift(); | ||
|
||
var trace1 = gd.data[1]; | ||
trace1.locations.shift(); | ||
|
||
Plotly.plot(gd).then(function() { | ||
expect(countTraces('scattergeo')).toBe(1); | ||
expect(countTraces('choropleth')).toBe(1); | ||
|
||
expect(countScatterGeoLines()).toBe(1); | ||
expect(countScatterGeoMarkers()).toBe(N_MARKERS_AT_START - 1); | ||
expect(countScatterGeoTextGroups()).toBe(N_MARKERS_AT_START - 1); | ||
expect(countScatterGeoTextNodes()).toBe(N_MARKERS_AT_START - 1); | ||
checkScatterGeoOrder(); | ||
|
||
expect(countChoroplethPaths()).toBe(N_LOCATIONS_AT_START - 1); | ||
|
||
done(); | ||
}); | ||
}); | ||
|
||
it('should be able to update line/marker/text nodes and choropleth paths', function(done) { | ||
var trace0 = gd.data[0]; | ||
trace0.lon = lonQueue; | ||
trace0.lat = latQueue; | ||
trace0.text = textQueue; | ||
|
||
var trace1 = gd.data[1]; | ||
trace1.locations = locationsQueue; | ||
trace1.z = zQueue; | ||
|
||
Plotly.plot(gd).then(function() { | ||
expect(countTraces('scattergeo')).toBe(1); | ||
expect(countTraces('choropleth')).toBe(1); | ||
|
||
expect(countScatterGeoLines()).toBe(1); | ||
expect(countScatterGeoMarkers()).toBe(lonQueue.length); | ||
expect(countScatterGeoTextGroups()).toBe(textQueue.length); | ||
expect(countScatterGeoTextNodes()).toBe(textQueue.length); | ||
checkScatterGeoOrder(); | ||
|
||
expect(countChoroplethPaths()).toBe(locationsQueue.length); | ||
|
||
done(); | ||
}); | ||
}); | ||
|
||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
previously, existing choropleth nodes were not removed.