Interaction: add `entitiesIn` to line plot #3138

Merged
merged 1 commit into from Jan 5, 2017

Projects

None yet

3 participants

@CalvinFernandez
Collaborator

a component of #3122

Calvin Fernandez add entities in to line plot
3ea2ae9
+
+ const xProjector = Plot._scaledAccessor(this.x());
+ const yProjector = Plot._scaledAccessor(this.y());
+ return this.entities().filter((entity) => {
@themadcreator
themadcreator Jan 5, 2017 Collaborator

perf note. I noticed that plot's .entities method does a couple linear time passes on the data. Notably, it uses _lightweightEntities then maps them to "heavyweight" entities, which as far as i can tell, just adds a d3 selection. For perf, we should maybe consider using lightweight entities when filtering, then convert to PlotEntities as a post process. Lots of other methods use this.entities() so, probably not worth in this PR, but for the future.

@themadcreator
themadcreator Jan 5, 2017 Collaborator

Filed as #3145

@themadcreator

Looks good. Keep em coming'

@@ -199,6 +199,60 @@ describe("Plots", () => {
linePlot.renderTo(svg);
});
+ afterEach(() => {
+ svg.remove();
@themadcreator
themadcreator Jan 5, 2017 Collaborator

heh, no brainer

@CalvinFernandez CalvinFernandez merged commit 1dcba44 into develop Jan 5, 2017

3 checks passed

cla/palantir CLA signed via membership in "palantir"
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
+ * @returns {PlotEntity[]}
+ */
+ public entitiesIn(xRange: Range, yRange: Range): PlotEntity[];
+ public entitiesIn(xRangeOrBounds: Range | Bounds, yRange?: Range): PlotEntity[] {
@hellochar
hellochar Jan 5, 2017 Collaborator

nit: should we declare an abstract entitiesIn method on XYPlot?

+ */
+ public entitiesIn(xRange: Range, yRange: Range): PlotEntity[];
+ public entitiesIn(xRangeOrBounds: Range | Bounds, yRange?: Range): PlotEntity[] {
+ let dataXRange: Range;
@hellochar
hellochar Jan 5, 2017 Collaborator

nit: we use this Range type to specify a range of numbers, but I've also seen [number, number] in some places

@CalvinFernandez CalvinFernandez deleted the cfernandez/linePlotEntitiesIn branch Jan 6, 2017
@hellochar hellochar modified the milestone: v2.6.0 Jan 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment