-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Allow the user to decide how to handle null values in charts #4071
Merged
Merged
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
24bef64
getredash/redash#2629 Refactor Chart visualization, add option for ha…
kravets-levko f7f4604
Handle null values in line/area stacking code; some cleanup
kravets-levko 8300b87
Handle edge case: line/area stacking when last value of one of series…
kravets-levko 094ca7b
Mjnor update to line/area stacking code
kravets-levko acee9f9
Fix line/area normalize to percents feature
kravets-levko f6fa1a5
Unit tests
kravets-levko 1fea462
Refine tests; add tests for prepareLayout function
kravets-levko 93c0ade
Tests for prepareData (heatmap) function
kravets-levko 82d224c
Tests for prepareData (pie) function
kravets-levko de9642a
Tests for prepareData (bar, line, area) function
kravets-levko b3e53cb
Tests for prepareData (scatter, bubble) function
kravets-levko d76845e
Tests for prepareData (box) function
kravets-levko 960454b
Remove unused file
kravets-levko 22601cd
Merge branch 'master' into chart-null-values
kravets-levko File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
40 changes: 40 additions & 0 deletions
40
client/app/visualizations/chart/fixtures/getChartData/multiple-series-grouped.json
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
{ | ||
"input": { | ||
"data": [ | ||
{ "a": 42, "b": 10, "g": "first" }, | ||
{ "a": 62, "b": 73, "g": "first" }, | ||
{ "a": 21, "b": 82, "g": "second" }, | ||
{ "a": 85, "b": 50, "g": "first" }, | ||
{ "a": 95, "b": 32, "g": "second" } | ||
], | ||
"options": { | ||
"columnMapping": { | ||
"a": "x", | ||
"b": "y", | ||
"g": "series" | ||
}, | ||
"seriesOptions": {} | ||
} | ||
}, | ||
"output": { | ||
"data": [ | ||
{ | ||
"name": "first", | ||
"type": "column", | ||
"data": [ | ||
{ "x": 42, "y": 10, "$raw": { "a": 42, "b": 10, "g": "first" } }, | ||
{ "x": 62, "y": 73, "$raw": { "a": 62, "b": 73, "g": "first" } }, | ||
{ "x": 85, "y": 50, "$raw": { "a": 85, "b": 50, "g": "first" } } | ||
] | ||
}, | ||
{ | ||
"name": "second", | ||
"type": "column", | ||
"data": [ | ||
{ "x": 21, "y": 82, "$raw": { "a": 21, "b": 82, "g": "second" } }, | ||
{ "x": 95, "y": 32, "$raw": { "a": 95, "b": 32, "g": "second" } } | ||
] | ||
} | ||
] | ||
} | ||
} |
41 changes: 41 additions & 0 deletions
41
client/app/visualizations/chart/fixtures/getChartData/multiple-series-multiple-y.json
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
{ | ||
"input": { | ||
"data": [ | ||
{ "a": 42, "b": 10, "c": 41, "d": 92 }, | ||
{ "a": 62, "b": 73 }, | ||
{ "a": 21, "b": null, "c": 33 }, | ||
{ "a": 85, "b": 50 }, | ||
{ "a": 95 } | ||
], | ||
"options": { | ||
"columnMapping": { | ||
"a": "x", | ||
"b": "y", | ||
"c": "y" | ||
}, | ||
"seriesOptions": {} | ||
} | ||
}, | ||
"output": { | ||
"data": [ | ||
{ | ||
"name": "b", | ||
"type": "column", | ||
"data": [ | ||
{ "x": 42, "y": 10, "$raw": { "a": 42, "b": 10, "c": 41, "d": 92 } }, | ||
{ "x": 62, "y": 73, "$raw": { "a": 62, "b": 73 } }, | ||
{ "x": 21, "y": null, "$raw": { "a": 21, "b": null, "c": 33 } }, | ||
{ "x": 85, "y": 50, "$raw": { "a": 85, "b": 50 } } | ||
] | ||
}, | ||
{ | ||
"name": "c", | ||
"type": "column", | ||
"data": [ | ||
{ "x": 42, "y": 41, "$raw": { "a": 42, "b": 10, "c": 41, "d": 92 } }, | ||
{ "x": 21, "y": 33, "$raw": { "a": 21, "b": null, "c": 33 } } | ||
] | ||
} | ||
] | ||
} | ||
} |
43 changes: 43 additions & 0 deletions
43
client/app/visualizations/chart/fixtures/getChartData/multiple-series-sorted.json
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
{ | ||
"input": { | ||
"data": [ | ||
{ "a": 42, "b": 10, "g": "first" }, | ||
{ "a": 62, "b": 73, "g": "first" }, | ||
{ "a": 21, "b": 82, "g": "second" }, | ||
{ "a": 85, "b": 50, "g": "first" }, | ||
{ "a": 95, "b": 32, "g": "second" } | ||
], | ||
"options": { | ||
"columnMapping": { | ||
"a": "x", | ||
"b": "y", | ||
"g": "series" | ||
}, | ||
"seriesOptions": { | ||
"first": { "zIndex": 2 }, | ||
"second": { "zIndex": 1 } | ||
} | ||
} | ||
}, | ||
"output": { | ||
"data": [ | ||
{ | ||
"name": "second", | ||
"type": "column", | ||
"data": [ | ||
{ "x": 21, "y": 82, "$raw": { "a": 21, "b": 82, "g": "second" } }, | ||
{ "x": 95, "y": 32, "$raw": { "a": 95, "b": 32, "g": "second" } } | ||
] | ||
}, | ||
{ | ||
"name": "first", | ||
"type": "column", | ||
"data": [ | ||
{ "x": 42, "y": 10, "$raw": { "a": 42, "b": 10, "g": "first" } }, | ||
{ "x": 62, "y": 73, "$raw": { "a": 62, "b": 73, "g": "first" } }, | ||
{ "x": 85, "y": 50, "$raw": { "a": 85, "b": 50, "g": "first" } } | ||
] | ||
} | ||
] | ||
} | ||
} |
32 changes: 32 additions & 0 deletions
32
client/app/visualizations/chart/fixtures/getChartData/single-series.json
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
{ | ||
"input": { | ||
"data": [ | ||
{ "a": 42, "b": 10, "c": 41, "d": 92 }, | ||
{ "a": 62, "b": 73 }, | ||
{ "a": 21, "b": null }, | ||
{ "a": 85, "b": 50 }, | ||
{ "a": 95 } | ||
], | ||
"options": { | ||
"columnMapping": { | ||
"a": "x", | ||
"b": "y" | ||
}, | ||
"seriesOptions": {} | ||
} | ||
}, | ||
"output": { | ||
"data": [ | ||
{ | ||
"name": "b", | ||
"type": "column", | ||
"data": [ | ||
{ "x": 42, "y": 10, "$raw": { "a": 42, "b": 10, "c": 41, "d": 92 } }, | ||
{ "x": 62, "y": 73, "$raw": { "a": 62, "b": 73 } }, | ||
{ "x": 21, "y": null, "$raw": { "a": 21, "b": null } }, | ||
{ "x": 85, "y": 50, "$raw": { "a": 85, "b": 50 } } | ||
] | ||
} | ||
] | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
/* eslint-disable global-require, import/no-unresolved */ | ||
import getChartData from './getChartData'; | ||
|
||
describe('Visualizations', () => { | ||
describe('Chart', () => { | ||
describe('getChartData', () => { | ||
test('Single series', () => { | ||
const { input, output } = require('./fixtures/getChartData/single-series'); | ||
const data = getChartData(input.data, input.options); | ||
expect(data).toEqual(output.data); | ||
}); | ||
|
||
test('Multiple series: multiple Y mappings', () => { | ||
const { input, output } = require('./fixtures/getChartData/multiple-series-multiple-y'); | ||
const data = getChartData(input.data, input.options); | ||
expect(data).toEqual(output.data); | ||
}); | ||
|
||
test('Multiple series: grouped', () => { | ||
const { input, output } = require('./fixtures/getChartData/multiple-series-grouped'); | ||
const data = getChartData(input.data, input.options); | ||
expect(data).toEqual(output.data); | ||
}); | ||
|
||
test('Multiple series: sorted', () => { | ||
const { input, output } = require('./fixtures/getChartData/multiple-series-sorted'); | ||
const data = getChartData(input.data, input.options); | ||
expect(data).toEqual(output.data); | ||
}); | ||
}); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
100 changes: 100 additions & 0 deletions
100
client/app/visualizations/chart/plotly/applyLayoutFixes.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,100 @@ | ||
import { find, pick, reduce } from 'lodash'; | ||
|
||
function fixLegendContainer(plotlyElement) { | ||
const legend = plotlyElement.querySelector('.legend'); | ||
if (legend) { | ||
let node = legend.parentNode; | ||
while (node) { | ||
if (node.tagName.toLowerCase() === 'svg') { | ||
node.style.overflow = 'visible'; | ||
break; | ||
} | ||
node = node.parentNode; | ||
} | ||
} | ||
} | ||
|
||
export default function applyLayoutFixes(plotlyElement, layout, updatePlot) { | ||
// update layout size to plot container | ||
layout.width = Math.floor(plotlyElement.offsetWidth); | ||
layout.height = Math.floor(plotlyElement.offsetHeight); | ||
|
||
const transformName = find([ | ||
'transform', | ||
'WebkitTransform', | ||
'MozTransform', | ||
'MsTransform', | ||
'OTransform', | ||
], prop => prop in plotlyElement.style); | ||
|
||
if (layout.width <= 600) { | ||
// change legend orientation to horizontal; plotly has a bug with this | ||
// legend alignment - it does not preserve enough space under the plot; | ||
// so we'll hack this: update plot (it will re-render legend), compute | ||
// legend height, reduce plot size by legend height (but not less than | ||
// half of plot container's height - legend will have max height equal to | ||
// plot height), re-render plot again and offset legend to the space under | ||
// the plot. | ||
layout.legend = { | ||
orientation: 'h', | ||
// locate legend inside of plot area - otherwise plotly will preserve | ||
// some amount of space under the plot; also this will limit legend height | ||
// to plot's height | ||
y: 0, | ||
x: 0, | ||
xanchor: 'left', | ||
yanchor: 'bottom', | ||
}; | ||
|
||
// set `overflow: visible` to svg containing legend because later we will | ||
// position legend outside of it | ||
fixLegendContainer(plotlyElement); | ||
|
||
updatePlot(plotlyElement, pick(layout, ['width', 'height', 'legend'])).then(() => { | ||
const legend = plotlyElement.querySelector('.legend'); // eslint-disable-line no-shadow | ||
if (legend) { | ||
// compute real height of legend - items may be split into few columnns, | ||
// also scrollbar may be shown | ||
const bounds = reduce(legend.querySelectorAll('.traces'), (result, node) => { | ||
const b = node.getBoundingClientRect(); | ||
result = result || b; | ||
return { | ||
top: Math.min(result.top, b.top), | ||
bottom: Math.max(result.bottom, b.bottom), | ||
}; | ||
}, null); | ||
// here we have two values: | ||
// 1. height of plot container excluding height of legend items; | ||
// it may be any value between 0 and plot container's height; | ||
// 2. half of plot containers height. Legend cannot be larger than | ||
// plot; if legend is too large, plotly will reduce it's height and | ||
// show a scrollbar; in this case, height of plot === height of legend, | ||
// so we can split container's height half by half between them. | ||
layout.height = Math.floor(Math.max( | ||
layout.height / 2, | ||
layout.height - (bounds.bottom - bounds.top), | ||
)); | ||
// offset the legend | ||
legend.style[transformName] = 'translate(0, ' + layout.height + 'px)'; | ||
updatePlot(plotlyElement, pick(layout, ['height'])); | ||
} | ||
}); | ||
} else { | ||
layout.legend = { | ||
orientation: 'v', | ||
// vertical legend will be rendered properly, so just place it to the right | ||
// side of plot | ||
y: 1, | ||
x: 1, | ||
xanchor: 'left', | ||
yanchor: 'top', | ||
}; | ||
|
||
const legend = plotlyElement.querySelector('.legend'); | ||
if (legend) { | ||
legend.style[transformName] = null; | ||
} | ||
|
||
updatePlot(plotlyElement, pick(layout, ['width', 'height', 'legend'])); | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Are you sure this should be the default? (putting aside the risk of messing up existing charts)
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.
That's exactly for not messing up existing charts 🙂 The other option is to update existing charts by migration (which I really don't want to do)
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.
It's the current behavior. If we were to start from the beginning maybe it wouldn't be a default.