Skip to content

Commit

Permalink
Fix: Mapped bar methods receive incorrect indices
Browse files Browse the repository at this point in the history
The map constructor now expects values to be flattened, and will warn when they aren't. To choose the correct colour, a length can be passed to the overrides option.
  • Loading branch information
stephenhutchings committed Mar 18, 2023
1 parent 081fd6a commit 8c30fb2
Show file tree
Hide file tree
Showing 7 changed files with 103 additions and 40 deletions.
28 changes: 17 additions & 11 deletions src/lib/map.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import utils from "./utils.js"
*
* Each option can be declared as a function. The function is passed the
* original datum and indices that correspond to how deeply the datum is nested.
* For example, bar chart data may be nested up to three levels
*
* However, it's often useful to use a shorthand syntax instead. If the
* property is an array, the array item at the index corresponding to the
Expand Down Expand Up @@ -102,26 +103,31 @@ const recur = (map, data, depth, indices = []) => {
/**
* @private
* @param {MapOptions} options
* @param {any[]} data
* @param {any[]} data - Flattened data
* @param {Object} [overrides]
* @param {number} [overrides.minValue]
* @param {boolean} [overrides.sum]
* @param {number} [overrides.maxDepth]
* @param {number} [overrides.minValue] - Minimum value required in order to
* render a label, as a percentage of the whole between zero and one.
* @param {number} [overrides.maxDepth] - Stop recurring through values early
* when, for example, the deepest array represents different data, like the
* scatter plot.
* @param {number} [overrides.colors] - The total number of colors from which to
* generate a new mapped color.
* @returns {Function} converter
*/
const Map = function (
options,
data = [],
{ minValue = 0, sum = false, maxDepth } = {}
{ minValue = 0, maxDepth, colors = data.length } = {}
) {
const map = Object.assign({}, defaults, options)

const values = data.map((v) => (Array.isArray(v) ? v : [v]).map(map.value))
const places = Math.min(
Math.max(...values.flat().map(utils.decimalPlaces)),
2
)
// Unless the data makes sense to be multi-dimensional, issue a warning
if (data.length > 0 && Array.isArray(map.y ? map.y(data[0]) : data[0])) {
console.warn("Data should be flattened when constructing a Map")
}

const values = data.map(map.y || map.value)
const places = Math.min(Math.max(...values.map(utils.decimalPlaces)), 2)

// By default, a label will only show when it exceeds the minimum value
// specified by a chart. It uses the largest number of decimal places found
Expand Down Expand Up @@ -164,7 +170,7 @@ const Map = function (
// A color function can return a background color, or an array of background
// and foreground colors. This wrapper ensures an array is always returned by
// the color function.
const base = map.color || ((v, i) => getColor(i / (data.length - 1)))
const base = map.color || ((v, i) => getColor(i / (colors - 1)))
map.color = wrapColor(base)

// The return function contains references to each mapping function in cases
Expand Down
47 changes: 33 additions & 14 deletions src/templates/bar.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,9 @@ import wrap from "./wrap.js"
* [34.44, 14.79, 30.64, 18.31, 1.82],
* ],
* map: {
* width: (v, i) => v === 30.64 ? 0.8 : 0.6,
* label: (v, i) => v === 30.64 ? v.toFixed(1) : false,
* key: (v, i) => ["A", "B", "C", "D", "E"][i],
* width: (v) => v === 30.64 ? 0.8 : 0.6,
* label: (v) => v === 30.64 ? v.toFixed(1) : false,
* key: ["A", "B", "C", "D", "E"],
* },
* stack: true,
* xAxis: { label: ["I", "II", "III"] }
Expand All @@ -85,8 +85,9 @@ import wrap from "./wrap.js"
* map: {
* key: ["In", "Out"],
* series: ["A", "B", "C"],
* tally: Math.round,
* label: Math.round,
* value: Math.round,
* tally: true,
* label: true,
* attrs: (d) => ({ "data-value": d })
* },
* xAxis: { label: ["I", "II"] }
Expand All @@ -102,8 +103,10 @@ export default ({
xAxis,
yAxis,
}) => {
// If the data is only one-level deep, it needs an initial wrapper
data = data.map((v) => (Array.isArray(v) ? v : [v]))

// If the data is only two-levels deep, wrap based on the `stack` option
if (data.length && !Array.isArray(data[0][0])) {
data = stack ? data.map((d) => [d]) : data.map((d) => d.map((d) => [d]))
} else {
Expand All @@ -122,27 +125,43 @@ export default ({
}
}

if (map && map.series && Array.isArray(map.key)) {
// Being triple-nested, bar charts need help to make an intuitive choice about
// which index to use when picking an item from an array.
if (Array.isArray(map?.key)) {
const arr = map.key
map.key = (v, i, j) => arr[j]
map.key = (v, k, j, i) => arr[maxStack > 1 ? i : j]
}

if (Array.isArray(map?.series)) {
const arr = map.series
map.series = (v, k, j) => arr[j]
}

if (Array.isArray(map?.color)) {
const arr = map.color
map.color = (v, k, j, i) => arr[maxStack > 1 ? i : j]
}

map = new Map(
{
color: (v, i, j) =>
getColor(maxStack === 1 ? i / (maxSeries - 1) : j / (maxStack - 1)),
// Other charts use the shallow index, but bar charts select a color
// from the deepest index, or second deepest if unstacked.
color: (v, k, j, i) => {
return getColor(
maxStack === 1 ? j / (maxSeries - 1) : i / (maxStack - 1)
)
},
width: 0.6,
...map,
},
data.flat(),
{ sum: stack, minValue: 0.05 }
stack ? data.flat().map((d) => utils.sum(d)) : data.flat(2),
{ minValue: 0.05 }
)

data = data.map(map)
data = map(data)

const maxWidth = Math.max(...data.flat(2).map((d) => d.width))

const values = data.flat().map(utils.sum)
const values = data.flat().map((d) => utils.sum(d))

xAxis = {
ticks: data.length,
Expand Down
4 changes: 2 additions & 2 deletions src/templates/line.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,8 @@ export default ({
y: (d) => d,
...map,
},
data,
{ minValue: -Infinity }
data.flat(),
{ minValue: -Infinity, colors: data.length }
)

data = map(data)
Expand Down
4 changes: 2 additions & 2 deletions src/templates/scatter.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ export default ({ data, title, description, map, xAxis, yAxis }) => {
r: 1,
...map,
},
data,
{ minValue: -Infinity, maxDepth: 2 }
data.flat(),
{ minValue: -Infinity, maxDepth: 2, colors: data.length }
)

data = map(data)
Expand Down
27 changes: 24 additions & 3 deletions test/lib/map.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { jest } from "@jest/globals"

import map from "../../src/lib/map.js"
import utils from "../../src/lib/utils.js"

describe("map", () => {
test("to return a converter function", () => {
Expand All @@ -9,13 +12,15 @@ describe("map", () => {
const m = { label: (v) => v + "" }
const d = [[[1]]]
const o = { minValue: 0 }
expect(map(m, d, o)(d)).toStrictEqual([

expect(map(m, d.flat(2), o)(d)).toStrictEqual([
[[{ color: ["#0036b0", "#fff"], label: "1", tally: false, value: 1 }]],
])
})

test("to wrap non-functions", () => {
const d = [1]

expect(map({ a: [0], b: 0, c: "0" }, d)(d)).toStrictEqual([
{
a: 0,
Expand All @@ -31,8 +36,15 @@ describe("map", () => {

test("to sum and respect `minValue`", () => {
const d = [[1, 2]]
const o = { sum: true, minValue: 0.34 }
expect(map({}, d, o)(d)).toStrictEqual([
const o = { minValue: 0.34 }

expect(
map(
{},
d.map((d) => utils.sum(d)),
o
)(d)
).toStrictEqual([
[
{
color: ["#0036b0", "#fff"],
Expand All @@ -49,4 +61,13 @@ describe("map", () => {
],
])
})

test("warns about nested data", () => {
const d = [[]]

console.warn = jest.fn()
map({}, d)(d)

expect(console.warn).toHaveBeenCalled()
})
})
21 changes: 18 additions & 3 deletions test/templates/bar.test.js
Original file line number Diff line number Diff line change
@@ -1,24 +1,39 @@
import bar from "../../src/templates/bar.js"

const simple = [1, 2, 3]
const nested = [[[1, 2]], [[3, 4]], [[5, 6]], [[7, 8]]]

describe("bar", () => {
test("is a function", () => {
expect(typeof bar).toBe("function")
})

test("renders basic chart", () => {
expect(bar({ data: [1, 2, 3] })).toMatch(
expect(bar({ data: simple })).toMatch(
'<div class="shown"><div class="chart chart-bar'
)
})

test("renders a single datum", () => {
expect(bar({ data: [1] })).not.toMatch('width="0"')
expect(bar({ data: simple.slice(0, 1) })).not.toMatch('width="0"')
})

test("renders correct classes", () => {
const result = bar({
data: nested,
map: { series: [1, 2] },
xAxis: { label: (d, j, i) => j },
})

expect(result).toMatch("has-series")
expect(result).toMatch("has-xaxis")
expect(result).toMatch("has-yaxis")
})

test("renders custom keys", () => {
expect(
bar({
data: [[[1, 2]], [[3, 4]]],
data: nested,
map: {
series: ["s1", "s2"],
key: ["custom1", "custom2"],
Expand Down
12 changes: 7 additions & 5 deletions types/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ declare module "shown" {
* [34.44, 14.79, 30.64, 18.31, 1.82],
* ],
* map: {
* width: (v, i) => v === 30.64 ? 0.8 : 0.6,
* label: (v, i) => v === 30.64 ? v.toFixed(1) : false,
* key: (v, i) => ["A", "B", "C", "D", "E"][i],
* width: (v) => v === 30.64 ? 0.8 : 0.6,
* label: (v) => v === 30.64 ? v.toFixed(1) : false,
* key: ["A", "B", "C", "D", "E"],
* },
* stack: true,
* xAxis: { label: ["I", "II", "III"] }
Expand All @@ -55,8 +55,9 @@ declare module "shown" {
* map: {
* key: ["In", "Out"],
* series: ["A", "B", "C"],
* tally: Math.round,
* label: Math.round,
* value: Math.round,
* tally: true,
* label: true,
* attrs: (d) => ({ "data-value": d })
* },
* xAxis: { label: ["I", "II"] }
Expand Down Expand Up @@ -292,6 +293,7 @@ declare module "shown" {
*
* Each option can be declared as a function. The function is passed the
* original datum and indices that correspond to how deeply the datum is nested.
* For example, bar chart data may be nested up to three levels
*
* However, it's often useful to use a shorthand syntax instead. If the
* property is an array, the array item at the index corresponding to the
Expand Down

0 comments on commit 8c30fb2

Please sign in to comment.