Skip to content

Commit

Permalink
Fix d3 portrait to landscape brush failure (nightscout#5638)
Browse files Browse the repository at this point in the history
* fix d3 portrait to landscape brush failure

* fix client.renderer.test for highlighBrushPoints function prototype change

* fix highlightBrush

* move brush reset inside check for valid brush
  • Loading branch information
jpcunningh authored May 5, 2020
1 parent 17eb4ae commit 7c5a69d
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 75 deletions.
19 changes: 13 additions & 6 deletions lib/client/chart.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,13 @@ function init (client, d3, $) {

function brushEnded () {
// update the opacity of the context data points to brush extent
var selectedRange = chart.createAdjustedRange();
var from = selectedRange[0].getTime();
var to = selectedRange[1].getTime();

chart.context.selectAll('circle')
.data(client.entries)
.style('opacity', function(d) { return renderer.highlightBrushPoints(d) });
.style('opacity', function(d) { return renderer.highlightBrushPoints(d, from, to) });
}

var extent = client.dataExtent();
Expand Down Expand Up @@ -251,6 +255,7 @@ function init (client, d3, $) {

chart.createBrushedRange = function() {
var brushedRange = chart.theBrush && d3.brushSelection(chart.theBrush.node()) || null;

var range = brushedRange && brushedRange.map(chart.xScale2.invert);
var dataExtent = client.dataExtent();

Expand All @@ -267,6 +272,8 @@ function init (client, d3, $) {
range[1] = new Date(end);
range[0] = new Date(end - client.focusRangeMS);

// console.log('createBrushedRange: ', brushedRange, range);

return range;
}

Expand Down Expand Up @@ -375,7 +382,8 @@ function init (client, d3, $) {

chart.theBrush.selectAll('rect')
.attr('y', 0)
.attr('height', contextHeight);
.attr('height', contextHeight)
.attr('width', '100%');

// disable resizing of brush
chart.context.select('.x.brush').select('.overlay').style('cursor', 'move');
Expand Down Expand Up @@ -507,7 +515,7 @@ function init (client, d3, $) {
.attr('y', 0)
.attr('height', contextHeight);

// console.log('Redrawing old brush with new dimensions: ', currentBrushExtent);
// console.log('chart.update(): Redrawing old brush with new dimensions: ', currentBrushExtent);

// redraw old brush with new dimensions
chart.theBrush.call(chart.brush.move, currentBrushExtent.map(chart.xScale2));
Expand Down Expand Up @@ -578,7 +586,7 @@ function init (client, d3, $) {

chart.xScaleBasals.domain(dataRange);

// console.log('Redrawing brush due to update: ', currentBrushExtent);
// console.log('chart.update(): Redrawing brush due to update: ', currentBrushExtent);

chart.theBrush.call(chart.brush.move, currentBrushExtent.map(chart.xScale2));
};
Expand Down Expand Up @@ -663,8 +671,7 @@ function init (client, d3, $) {
renderer.addTreatmentProfiles(client);
renderer.drawTreatments(client);


// console.log('Redrawing brush due to update: ', currentBrushExtent);
// console.log('scrollUpdate(): Redrawing brush due to update: ', currentBrushExtent);

chart.theBrush.call(chart.brush.move, currentBrushExtent.map(chart.xScale2));

Expand Down
14 changes: 9 additions & 5 deletions lib/client/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -390,9 +390,12 @@ client.load = function load (serverSettings, callback) {

brushExtent[0] = new Date(brushExtent[1].getTime() - client.focusRangeMS);

// console.log('Resetting brush in updateBrushToNow: ', brushExtent);
// console.log('updateBrushToNow(): Resetting brush: ', brushExtent);

chart.theBrush && chart.theBrush.call(chart.brush.move, brushExtent.map(chart.xScale2));
if (chart.theBrush) {
chart.theBrush.call(chart.brush)
chart.theBrush.call(chart.brush.move, brushExtent.map(chart.xScale2));
}

if (!skipBrushing) {
brushed();
Expand All @@ -415,7 +418,6 @@ client.load = function load (serverSettings, callback) {

function brushed () {
// Brush not initialized
console.log("brushed");
if (!chart.theBrush) {
return;
}
Expand All @@ -426,11 +428,13 @@ client.load = function load (serverSettings, callback) {

var brushedRange = d3.brushSelection(chart.theBrush.node());

// console.log("brushed(): coordinates: ", brushedRange);

if (brushedRange) {
brushExtent = brushedRange.map(chart.xScale2.invert);
}

// console.log('Brushed to: ', brushExtent);
console.log('brushed(): Brushed to: ', brushExtent);

if (!brushedRange || (brushExtent[1].getTime() - brushExtent[0].getTime() !== client.focusRangeMS)) {
// ensure that brush updating is with the time range
Expand All @@ -440,7 +444,7 @@ client.load = function load (serverSettings, callback) {
brushExtent[1] = new Date(brushExtent[0].getTime() + client.focusRangeMS);
}

// console.log('Updating brushed to: ', brushExtent);
// console.log('brushed(): updating to: ', brushExtent);

chart.theBrush.call(chart.brush.move, brushExtent.map(chart.xScale2));
}
Expand Down
6 changes: 1 addition & 5 deletions lib/client/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,7 @@ function init (client, d3) {
}

// get the desired opacity for context chart based on the brush extent
renderer.highlightBrushPoints = function highlightBrushPoints (data) {
var selectedRange = chart().createAdjustedRange();
var from = selectedRange[0].getTime();
var to = selectedRange[1].getTime();

renderer.highlightBrushPoints = function highlightBrushPoints (data, from, to) {
if (client.latestSGV && data.mills >= from && data.mills <= to) {
return chart().futureOpacity(data.mills - client.latestSGV.mills);
} else {
Expand Down
114 changes: 57 additions & 57 deletions npm-shrinkwrap.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
"css-loader": "^1.0.1",
"cssmin": "^0.4.3",
"csv-stringify": "^5.3.5",
"d3": "^5.12.0",
"d3": "^5.16.0",
"easyxml": "^2.0.1",
"ejs": "^2.6.2",
"errorhandler": "^1.5.1",
Expand Down
5 changes: 4 additions & 1 deletion tests/client.renderer.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ describe('renderer', () => {

describe(`data.mills ${extent.mills} and chart().brush.extent() times ${extent.times}`, () => {
it(extent.expectation, () => {
renderer(mockClient, {}).highlightBrushPoints(mockData).should.equal(extent.expectedOpacity);
var selectedRange = mockClient.chart.createAdjustedRange();
var from = selectedRange[0].getTime();
var to = selectedRange[1].getTime();
renderer(mockClient, {}).highlightBrushPoints(mockData, from, to).should.equal(extent.expectedOpacity);
});
});
});
Expand Down

0 comments on commit 7c5a69d

Please sign in to comment.