Skip to content
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.

Sign up for GitHub

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

feat(legend): Add shapes to legend #289

Closed
wants to merge 3 commits into from
Closed

feat(legend): Add shapes to legend #289

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6b4001c
feat(legend): Add shapes to legend
Feb 13, 2018
9ea7c14
feat(legend): Add test for custom points
Feb 16, 2018
18d99be
feat(legend): Add `legend.usePoint` option
Feb 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions spec/internals/legend-spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,4 +345,42 @@ describe("LEGEND", () => {
expect(items.size()).to.be.equal(1);
});
});

describe("when using custom points", () => {

before(() => {
args = {
data: {
columns: [
["data1", 30, 200, 100, 400, 150, 250],
["data2", 130, 100, 200, 100, 250, 150],
["data3", 60, 190, 320, 520, 20, 300],
["data4", 80, 20, 250, 320, 180, 50]
]
},
point: {
pattern: [
"circle",
"rectangle",
"<polygon points='2.5 0 0 5 5 5'></polygon>"
]
}
};
});

it("should render custom points in legend", () => {
const nodes = chart.internal.svg.selectAll(".bb-legend-item .bb-legend-item-point");

nodes.each((data, idx, selection) => {
const node = selection[idx];
const nodeName = node.nodeName.toLowerCase();
const expected = (idx === 0 || idx === 3) ?
"circle" : (idx === 1) ? "rect" : (idx === 2) ? "use" : "";

expect(nodeName).to.be.equal(expected);
});

expect(nodes.size()).to.be.equal(4);
});
});
});
1 change: 1 addition & 0 deletions src/config/classes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ export default {
legendItem: "bb-legend-item",
legendItemEvent: "bb-legend-item-event",
legendItemTile: "bb-legend-item-tile",
legendItemPoint: "bb-legend-item-point",
legendItemHidden: "bb-legend-item-hidden",
legendItemFocused: "bb-legend-item-focused",
dragarea: "bb-dragarea",
Expand Down
113 changes: 94 additions & 19 deletions src/internals/legend.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@
*/
import {
select as d3Select,
event as d3Event
event as d3Event,
namespaces as d3Namespaces
} from "d3";
import ChartInternal from "./ChartInternal";
import CLASS from "../config/classes";
import {extend, isDefined, getOption, isEmpty, isFunction} from "./util";
import {extend, isDefined, getOption, isEmpty, isFunction, notEmpty} from "./util";

extend(ChartInternal.prototype, {
/**
Expand Down Expand Up @@ -466,6 +467,7 @@ extend(ChartInternal.prototype, {
}
};


if ($$.isLegendInset) {
step = config.legend_inset_step ? config.legend_inset_step : targetIdz.length;
$$.updateLegendStep(step);
Expand Down Expand Up @@ -513,15 +515,47 @@ extend(ChartInternal.prototype, {
.attr("x", $$.isLegendRight || $$.isLegendInset ? xForLegendRect : -200)
.attr("y", $$.isLegendRight || $$.isLegendInset ? -200 : yForLegendRect);

l.append("line")
.attr("class", CLASS.legendItemTile)
.style("stroke", $$.color)
.style("pointer-events", "none")
.attr("x1", $$.isLegendRight || $$.isLegendInset ? x1ForLegendTile : -200)
.attr("y1", $$.isLegendRight || $$.isLegendInset ? -200 : yForLegendTile)
.attr("x2", $$.isLegendRight || $$.isLegendInset ? x2ForLegendTile : -200)
.attr("y2", $$.isLegendRight || $$.isLegendInset ? -200 : yForLegendTile)
.attr("stroke-width", config.legend_item_tile_height);
const hasCustomPoints = $$.config.point_pattern.length;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current just is forcing to be displayed with the defined point patterns. In this case I think there're limitation of:

  • When point patterns are defined, there're no option to handle legend display style
  • When point.type="rectangle"(or 'circle') is set, they're displayed as the old square legend style

So, how about having an option like legend.usePoint.

bb.generate({
    legend: {
        usePoint: true // default false
    }
}

When set this option to true, the legend will likely, displayed as data point, rather than the current square.


if (!hasCustomPoints) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't a big deal, but to be more clear the condition block can be written in opposite way.

if (hasCustomPoints) {
    const ids = [];
    l.append(d => {
        ...
} else {
    l.append("line")
        ...
}

l.append("line")
.attr("class", CLASS.legendItemTile)
.style("stroke", $$.color)
.style("pointer-events", "none")
.attr("x1", $$.isLegendRight || $$.isLegendInset ? x1ForLegendTile : -200)
.attr("y1", $$.isLegendRight || $$.isLegendInset ? -200 : yForLegendTile)
.attr("x2", $$.isLegendRight || $$.isLegendInset ? x2ForLegendTile : -200)
.attr("y2", $$.isLegendRight || $$.isLegendInset ? -200 : yForLegendTile)
.attr("stroke-width", config.legend_item_tile_height);
} else {
const ids = [];

l.append(d => {
const pattern = notEmpty(config.point_pattern) ? config.point_pattern : [config.point_type];

if (ids.indexOf(d) === -1) {
ids.push(d);
}
let point = pattern[ids.indexOf(d) % pattern.length];

if (point === "rectangle") {
point = "rect";
}

const nodeType = $$.hasValidPointType(point) ? point : "use";

return document.createElementNS(d3Namespaces.svg, nodeType);
})
.attr("class", CLASS.legendItemPoint)
.style("fill", d => $$.color(d))
.style("pointer-events", "none")
.attr("href", (data, idx, selection) => {
const node = selection[idx];
const nodeName = node.nodeName.toLowerCase();

return nodeName === "use" ? `#${$$.datetimeId}-point-${data}` : undefined;
});
}

// Set background for inset legend
background = $$.legend.select(`.${CLASS.legendBackground} rect`);
Expand Down Expand Up @@ -552,15 +586,56 @@ extend(ChartInternal.prototype, {
.attr("x", xForLegendRect)
.attr("y", yForLegendRect);

const tiles = $$.legend.selectAll(`line.${CLASS.legendItemTile}`)
.data(targetIdz);

(withTransition ? tiles.transition() : tiles)
.style("stroke", $$.color)
.attr("x1", x1ForLegendTile)
.attr("y1", yForLegendTile)
.attr("x2", x2ForLegendTile)
.attr("y2", yForLegendTile);
if (!hasCustomPoints) {
const tiles = $$.legend.selectAll(`line.${CLASS.legendItemTile}`)
.data(targetIdz);


(withTransition ? tiles.transition() : tiles)
.style("stroke", $$.color)
.attr("x1", x1ForLegendTile)
.attr("y1", yForLegendTile)
.attr("x2", x2ForLegendTile)
.attr("y2", yForLegendTile);
} else {
const tiles = $$.legend.selectAll(`.${CLASS.legendItemPoint}`)
.data(targetIdz);

(withTransition ? tiles.transition() : tiles)
.each((data, idx, selection) => {
const node = selection[idx];
const nodeName = node.nodeName.toLowerCase();
const d3Node = d3Select(node);
let x = "x";
let y = "y";
let yOffset = 2;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The size for circle and rectangle point displayed in the legend, is quite small.
How about increase little bit more?

let yOffset = 2.5;
let radius;
let width;
let height;

if (nodeName === "circle") {
	x = "cx";
	y = "cy";
	radius = $$.config.point_r + ($$.config.point_r * 0.2);
	yOffset = -($$.config.point_r);
} else if (nodeName === "rect") {
	width = $$.config.point_r * 2.5;
	height = $$.config.point_r * 2.5;
}

let radius;
let width;
let height;

if (nodeName === "circle") {
x = "cx";
y = "cy";
radius = $$.config.point_r;
yOffset = -($$.config.point_r);
}

if (nodeName === "rect") {
width = $$.config.point_r * 2;
height = $$.config.point_r * 2;
yOffset = 0;
}


d3Node
.attr(x, x1ForLegendTile)
.attr(y, d => yForLegendTile(d) - yOffset)
.attr("r", radius)
.attr("width", width)
.attr("height", height);
});
}

if (background) {
(withTransition ? background.transition() : background)
Expand Down