From 2d32c53a15a9cb18a71c0deb427bd6aa7e18d8fd Mon Sep 17 00:00:00 2001 From: Daniel Mane Date: Thu, 15 May 2014 13:57:50 -0700 Subject: [PATCH 01/17] Components now keep track of their parents --- src/core/component.ts | 10 ++++++++-- src/core/componentGroup.ts | 8 ++++---- src/core/table.ts | 6 +++--- test/componentGroupTests.ts | 6 +++--- test/componentTests.ts | 26 +++++++++++++------------- test/gridlinesTests.ts | 2 +- test/labelTests.ts | 8 ++++---- 7 files changed, 36 insertions(+), 30 deletions(-) diff --git a/src/core/component.ts b/src/core/component.ts index f43d3c5362..595171abee 100644 --- a/src/core/component.ts +++ b/src/core/component.ts @@ -16,6 +16,7 @@ module Plottable { private rootSVG: D3.Selection; private isTopLevelComponent = false; + private parent: Component; public availableWidth : number; // Width and height of the component. Used to size the hitbox, bounding box, etc public availableHeight: number; @@ -37,7 +38,7 @@ module Plottable { * @param {D3.Selection} element A D3 selection consisting of the element to anchor under. * @returns {Component} The calling component. */ - public _anchor(element: D3.Selection) { + public _anchor(element: D3.Selection, parent?: Component) { if (element.node().nodeName === "svg") { // svg node gets the "plottable" CSS class this.rootSVG = element; @@ -46,6 +47,10 @@ module Plottable { this.rootSVG.style("overflow", "visible"); this.isTopLevelComponent = true; } + if (parent == null && !this.isTopLevelComponent) { + throw new Error("Components must be top-level or have a parent"); + } + this.parent = parent; if (this.element != null) { // reattach existing element @@ -176,7 +181,7 @@ module Plottable { } else { selection = d3.select(element); } - this._anchor(selection); + this._anchor(selection, null); } this._computeLayout()._render(); return this; @@ -400,6 +405,7 @@ module Plottable { public remove() { this.element.remove(); this.isAnchored = false; + this.parent = null; return this; } } diff --git a/src/core/componentGroup.ts b/src/core/componentGroup.ts index 2c31539104..9b08161e65 100644 --- a/src/core/componentGroup.ts +++ b/src/core/componentGroup.ts @@ -35,7 +35,7 @@ module Plottable { this.components.push(c); } if (this.element != null) { - c._anchor(this.content); + c._anchor(this.content, this); } return this; } @@ -72,9 +72,9 @@ module Plottable { return this; } - public _anchor(element: D3.Selection): ComponentGroup { - super._anchor(element); - this.components.forEach((c) => c._anchor(this.content)); + public _anchor(element: D3.Selection, parent: Component): ComponentGroup { + super._anchor(element, parent); + this.components.forEach((c) => c._anchor(this.content, this)); return this; } diff --git a/src/core/table.ts b/src/core/table.ts index 0df1ef3e88..9466dcf35a 100644 --- a/src/core/table.ts +++ b/src/core/table.ts @@ -64,13 +64,13 @@ module Plottable { return this; } - public _anchor(element: D3.Selection) { - super._anchor(element); + public _anchor(element: D3.Selection, parent: Component) { + super._anchor(element, parent); // recursively anchor children this.rows.forEach((row: Component[], rowIndex: number) => { row.forEach((component: Component, colIndex: number) => { if (component != null) { - component._anchor(this.content); + component._anchor(this.content, this); } }); }); diff --git a/test/componentGroupTests.ts b/test/componentGroupTests.ts index ed875d77d7..edd75155f5 100644 --- a/test/componentGroupTests.ts +++ b/test/componentGroupTests.ts @@ -11,7 +11,7 @@ describe("ComponentGroups", () => { var cg = new Plottable.ComponentGroup([c1, c2, c3]); var svg = generateSVG(400, 400); - cg._anchor(svg); + cg._anchor(svg, null); ( c1).addBox("test-box1"); ( c2).addBox("test-box2"); ( c3).addBox("test-box3"); @@ -32,7 +32,7 @@ describe("ComponentGroups", () => { var cg = new Plottable.ComponentGroup([c1]); var svg = generateSVG(400, 400); - cg.merge(c2)._anchor(svg); + cg.merge(c2)._anchor(svg, null); ( c1).addBox("test-box1"); ( c2).addBox("test-box2"); cg._computeLayout()._render(); @@ -73,7 +73,7 @@ describe("ComponentGroups", () => { cg.merge(c1).merge(c2); var svg = generateSVG(); - cg._anchor(svg); + cg._anchor(svg, null); cg._computeLayout(50, 50, 350, 350); var cgTranslate = d3.transform(cg.element.attr("transform")).translate; diff --git a/test/componentTests.ts b/test/componentTests.ts index 04f80b9c99..01b257ecf6 100644 --- a/test/componentTests.ts +++ b/test/componentTests.ts @@ -24,14 +24,14 @@ describe("Component behavior", () => { describe("anchor", () => { it("anchoring works as expected", () => { - c._anchor(svg); + c._anchor(svg, null); assert.equal(c.element.node(), svg.select("g").node(), "the component anchored to a beneath the "); assert.isTrue(svg.classed("plottable"), " was given \"plottable\" CSS class"); svg.remove(); }); it("can re-anchor to a different element", () => { - c._anchor(svg); + c._anchor(svg, null); var svg2 = generateSVG(SVG_WIDTH, SVG_HEIGHT); c._anchor(svg2); @@ -45,7 +45,7 @@ describe("Component behavior", () => { describe("computeLayout", () => { it("computeLayout defaults and updates intelligently", () => { - c._anchor(svg)._computeLayout(); + c._anchor(svg, null)._computeLayout(); assert.equal(c.availableWidth , SVG_WIDTH, "computeLayout defaulted width to svg width"); assert.equal(c.availableHeight, SVG_HEIGHT, "computeLayout defaulted height to svg height"); assert.equal(( c).xOrigin, 0 ,"xOrigin defaulted to 0"); @@ -72,7 +72,7 @@ describe("Component behavior", () => { svg.style("width", "50%"); svg.style("height", "50%"); - c._anchor(svg)._computeLayout(); + c._anchor(svg, null)._computeLayout(); assert.equal(c.availableWidth , 100, "computeLayout defaulted width to svg width"); assert.equal(c.availableHeight, 200, "computeLayout defaulted height to svg height"); @@ -96,7 +96,7 @@ describe("Component behavior", () => { it("computeLayout will not default when attached to non-root node", () => { var g = svg.append("g"); - c._anchor(g); + c._anchor(g, new Plottable.Component()); assert.throws(() => c._computeLayout(), "null arguments"); svg.remove(); }); @@ -112,7 +112,7 @@ describe("Component behavior", () => { var yOff = 20; var width = 100; var height = 200; - c._anchor(g)._computeLayout(xOff, yOff, width, height); + c._anchor(g, new Plottable.Component())._computeLayout(xOff, yOff, width, height); var translate = getTranslate(c.element); assert.deepEqual(translate, [xOff, yOff], "the element translated appropriately"); assert.equal(c.availableWidth , width, "the width set properly"); @@ -137,7 +137,7 @@ describe("Component behavior", () => { it("fixed-width component will align to the right spot", () => { fixComponentSize(c, 100, 100); - c._anchor(svg); + c._anchor(svg, null); c._computeLayout(); assertComponentXY(c, 0, 0, "top-left component aligns correctly"); @@ -153,7 +153,7 @@ describe("Component behavior", () => { it("components can be offset relative to their alignment, and throw errors if there is insufficient space", () => { fixComponentSize(c, 100, 100); - c._anchor(svg); + c._anchor(svg, null); c.xOffset(20).yOffset(20); c._computeLayout(); assertComponentXY(c, 20, 20, "top-left component offsets correctly"); @@ -194,7 +194,7 @@ it("components can be offset relative to their alignment, and throw errors if th assert.isFalse(c.clipPathEnabled, "clipPathEnabled defaults to false"); c.clipPathEnabled = true; var expectedClipPathID = c._plottableID; - c._anchor(svg)._computeLayout(0, 0, 100, 100)._render(); + c._anchor(svg, null)._computeLayout(0, 0, 100, 100)._render(); var expectedClipPathURL = "url(#clipPath" + expectedClipPathID+ ")"; assert.equal(c.element.attr("clip-path"), expectedClipPathURL, "the element has clip-path url attached"); var clipRect = ( c).boxContainer.select(".clip-rect"); @@ -240,20 +240,20 @@ it("components can be offset relative to their alignment, and throw errors if th assert.equal(hitBox.style("opacity"), "0", "the hitBox is transparent, otherwise it would look weird"); } - c._anchor(svg); + c._anchor(svg, null); assert.isUndefined(( c).hitBox, "no hitBox was created when there were no registered interactions"); svg.remove(); svg = generateSVG(); c = new Plottable.Component(); var i = new Plottable.Interaction(c).registerWithComponent(); - c._anchor(svg); + c._anchor(svg, null); verifyHitbox(c); svg.remove(); svg = generateSVG(); c = new Plottable.Component(); - c._anchor(svg); + c._anchor(svg, null); i = new Plottable.Interaction(c).registerWithComponent(); verifyHitbox(c); svg.remove(); @@ -288,7 +288,7 @@ it("components can be offset relative to their alignment, and throw errors if th c.classed("CSS-PREANCHOR-REMOVE", false); assert.isFalse(c.classed("CSS-PREANCHOR-REMOVE")); - c._anchor(svg); + c._anchor(svg, null); assert.isTrue(c.classed("CSS-PREANCHOR-KEEP")); assert.isFalse(c.classed("CSS-PREANCHOR-REMOVE")); assert.isFalse(c.classed("CSS-POSTANCHOR")); diff --git a/test/gridlinesTests.ts b/test/gridlinesTests.ts index 74a843bd52..606c4ac8d2 100644 --- a/test/gridlinesTests.ts +++ b/test/gridlinesTests.ts @@ -19,7 +19,7 @@ describe("Gridlines", () => { .addComponent(0, 1, gridlines) .addComponent(1, 1, xAxis); - basicTable._anchor(svg); + basicTable._anchor(svg, null); basicTable._computeLayout(); xScale.range([0, xAxis.availableWidth ]); // manually set range since we don't have a renderer yScale.range([yAxis.availableHeight, 0]); diff --git a/test/labelTests.ts b/test/labelTests.ts index 477a79135f..05f84b34f6 100644 --- a/test/labelTests.ts +++ b/test/labelTests.ts @@ -8,7 +8,7 @@ describe("Labels", () => { it("Standard text title label generates properly", () => { var svg = generateSVG(400, 80); var label = new Plottable.TitleLabel("A CHART TITLE"); - label._anchor(svg); + label._anchor(svg, null); label._computeLayout(); var content = label.content; @@ -27,7 +27,7 @@ describe("Labels", () => { it("Left-rotated text is handled properly", () => { var svg = generateSVG(100, 400); var label = new Plottable.AxisLabel("LEFT-ROTATED LABEL", "vertical-left"); - label._anchor(svg); + label._anchor(svg, null); var content = label.content; var text = content.select("text"); label._computeLayout(); @@ -42,7 +42,7 @@ describe("Labels", () => { it("Right-rotated text is handled properly", () => { var svg = generateSVG(100, 400); var label = new Plottable.AxisLabel("RIGHT-ROTATED LABEL", "vertical-right"); - label._anchor(svg); + label._anchor(svg, null); var content = label.content; var text = content.select("text"); label._computeLayout(); @@ -72,7 +72,7 @@ describe("Labels", () => { var svgWidth = 400; var svg = generateSVG(svgWidth, 80); var label = new Plottable.TitleLabel("THIS LABEL IS SO LONG WHOEVER WROTE IT WAS PROBABLY DERANGED"); - label._anchor(svg); + label._anchor(svg, null); var content = label.content; var text = content.select("text"); label._computeLayout(); From ec9e849920a9b0477e4b5d3f392e756180bb1d94 Mon Sep 17 00:00:00 2001 From: Daniel Mane Date: Thu, 15 May 2014 14:44:24 -0700 Subject: [PATCH 02/17] Add and test Component._invalidateLayout, which causes layout re-computation on rAF cycle. Add hooks in all functions which cause layout to change. --- src/components/axis.ts | 4 +++- src/components/label.ts | 1 + src/components/legend.ts | 2 +- src/core/component.ts | 26 ++++++++++++++++++++++++++ src/core/componentGroup.ts | 3 +++ src/core/renderController.ts | 22 ++++++++++++++++++++-- src/core/table.ts | 3 +++ test/componentTests.ts | 13 +++++++++++++ test/legendTests.ts | 10 ++++++---- 9 files changed, 76 insertions(+), 8 deletions(-) diff --git a/src/components/axis.ts b/src/components/axis.ts index 294cbd7c00..594f64bf19 100644 --- a/src/components/axis.ts +++ b/src/components/axis.ts @@ -261,7 +261,7 @@ module Plottable { return this.d3Axis.tickFormat(); } else { this.d3Axis.tickFormat(formatter); - this._render(); + this._invalidateLayout(); return this; } } @@ -288,6 +288,7 @@ module Plottable { public height(h: number) { this._height = h; + this._invalidateLayout(); return this; } @@ -425,6 +426,7 @@ module Plottable { public width(w: number) { this._width = w; + this._invalidateLayout(); return this; } diff --git a/src/components/label.ts b/src/components/label.ts index 938196f499..6d70af3717 100644 --- a/src/components/label.ts +++ b/src/components/label.ts @@ -64,6 +64,7 @@ module Plottable { this.textElement.text(text); this.measureAndSetTextSize(); } + this._invalidateLayout(); return this; } diff --git a/src/components/legend.ts b/src/components/legend.ts index 0e160ee0ed..65e5084861 100644 --- a/src/components/legend.ts +++ b/src/components/legend.ts @@ -44,7 +44,7 @@ module Plottable { this._deregisterFromBroadcaster(this.colorScale); } this.colorScale = scale; - this._registerToBroadcaster(this.colorScale, () => this._render()); + this._registerToBroadcaster(this.colorScale, () => this._invalidateLayout()); return this; } else { return this.colorScale; diff --git a/src/core/component.ts b/src/core/component.ts index 595171abee..9eb10f51e6 100644 --- a/src/core/component.ts +++ b/src/core/component.ts @@ -163,10 +163,28 @@ module Plottable { return this; } + public _scheduleComputeLayout() { + if (this.isAnchored && this.isSetup) { + RenderController.registerToComputeLayout(this); + } + return this + } + public _doRender() { return this; //no-op } + + public _invalidateLayout() { + if (this.isAnchored && this.isSetup) { + if (this.isTopLevelComponent) { + this._scheduleComputeLayout(); + } else { + this.parent._invalidateLayout(); + } + } + } + /** * Renders the Component into a given DOM element. * @@ -217,6 +235,7 @@ module Plottable { } else { throw new Error("Unsupported alignment"); } + this._invalidateLayout(); return this; } @@ -237,6 +256,7 @@ module Plottable { } else { throw new Error("Unsupported alignment"); } + this._invalidateLayout(); return this; } @@ -248,6 +268,7 @@ module Plottable { */ public xOffset(offset: number): Component { this._xOffset = offset; + this._invalidateLayout(); return this; } @@ -259,6 +280,7 @@ module Plottable { */ public yOffset(offset: number): Component { this._yOffset = offset; + this._invalidateLayout(); return this; } @@ -389,6 +411,9 @@ module Plottable { */ public merge(c: Component): ComponentGroup { var cg: ComponentGroup; + if (this.isSetup || this.isAnchored) { + throw new Error("Can't presently merge a component that's already been anchored"); + } if (ComponentGroup.prototype.isPrototypeOf(c)) { cg = ( c); cg._addComponentToGroup(this, true); @@ -404,6 +429,7 @@ module Plottable { */ public remove() { this.element.remove(); + this._invalidateLayout(); this.isAnchored = false; this.parent = null; return this; diff --git a/src/core/componentGroup.ts b/src/core/componentGroup.ts index 9b08161e65..5c1bb18efb 100644 --- a/src/core/componentGroup.ts +++ b/src/core/componentGroup.ts @@ -29,6 +29,7 @@ module Plottable { } public _addComponentToGroup(c: Component, prepend = false): ComponentGroup { + this._invalidateLayout(); if (prepend) { this.components.unshift(c); } else { @@ -57,6 +58,7 @@ module Plottable { if (removeIndex >= 0) { this.components.splice(removeIndex, 1); c.remove(); + this._invalidateLayout(); } return this; } @@ -69,6 +71,7 @@ module Plottable { public empty(): ComponentGroup { this.components.forEach((c: Component) => c.remove()); this.components = []; + this._invalidateLayout(); return this; } diff --git a/src/core/renderController.ts b/src/core/renderController.ts index 46d39406f2..8c6891225b 100644 --- a/src/core/renderController.ts +++ b/src/core/renderController.ts @@ -3,6 +3,7 @@ module Plottable { export class RenderController { private static componentsNeedingRender: {[key: string]: Component} = {}; + private static componentsNeedingComputeLayout: {[key: string]: Component} = {}; private static animationRequested = false; public static enabled = ( window).PlottableTestCode == null && (window.requestAnimationFrame) != null; @@ -12,6 +13,20 @@ module Plottable { return; } RenderController.componentsNeedingRender[c._plottableID] = c; + RenderController.requestFrame(); + } + + public static registerToComputeLayout(c: Component) { + if (!Plottable.RenderController.enabled) { + c._computeLayout()._render(); + return; + } + RenderController.componentsNeedingComputeLayout[c._plottableID] = c; + RenderController.componentsNeedingRender[c._plottableID] = c; + RenderController.requestFrame(); + } + + private static requestFrame() { if (!RenderController.animationRequested) { requestAnimationFrame(RenderController.doRender); RenderController.animationRequested = true; @@ -19,8 +34,11 @@ module Plottable { } public static doRender() { - var components = d3.values(RenderController.componentsNeedingRender); - components.forEach((c) => c._doRender()); + var toCompute = d3.values(RenderController.componentsNeedingComputeLayout); + toCompute.forEach((c) => c._computeLayout()); + var toRender = d3.values(RenderController.componentsNeedingRender); + toRender.forEach((c) => c._doRender()); + RenderController.componentsNeedingComputeLayout = {}; RenderController.componentsNeedingRender = {}; RenderController.animationRequested = false; } diff --git a/src/core/table.ts b/src/core/table.ts index 9466dcf35a..32c567ddd0 100644 --- a/src/core/table.ts +++ b/src/core/table.ts @@ -261,6 +261,7 @@ module Plottable { public padding(rowPadding: number, colPadding: number) { this.rowPadding = rowPadding; this.colPadding = colPadding; + this._invalidateLayout(); return this; } @@ -274,6 +275,7 @@ module Plottable { */ public rowWeight(index: number, weight: number) { this.rowWeights[index] = weight; + this._invalidateLayout(); return this; } @@ -287,6 +289,7 @@ module Plottable { */ public colWeight(index: number, weight: number) { this.colWeights[index] = weight; + this._invalidateLayout(); return this; } diff --git a/test/componentTests.ts b/test/componentTests.ts index 01b257ecf6..07d202f539 100644 --- a/test/componentTests.ts +++ b/test/componentTests.ts @@ -322,4 +322,17 @@ it("components can be offset relative to their alignment, and throw errors if th svg.remove(); }); + + it("_invalidateLayout works as expected", () => { + var cg = new Plottable.ComponentGroup(); + var c = makeFixedSizeComponent(10, 10); + cg._addComponentToGroup(c); + cg.renderTo(svg); + assert.equal(cg.availableHeight, 10, "availableHeight initially 10 for fixed-size component"); + assert.equal(cg.availableWidth, 10, "availableWidth initially 10 for fixed-size component"); + fixComponentSize(c, 50, 50); + c._invalidateLayout(); + assert.equal(cg.availableHeight, 50, "invalidateLayout propagated to parent and caused resized height"); + assert.equal(cg.availableWidth, 50, "invalidateLayout propagated to parent and caused resized width"); + }); }); diff --git a/test/legendTests.ts b/test/legendTests.ts index 824b45ddf9..83643206a0 100644 --- a/test/legendTests.ts +++ b/test/legendTests.ts @@ -31,16 +31,18 @@ describe("Legends", () => { svg.remove(); }); - it("legend domain can be updated after initialization, and minimumHeight updates as well", () => { + it("legend domain can be updated after initialization, and height updates as well", () => { legend.renderTo(svg); legend.scale(color); assert.equal(legend._requestedSpace(200, 200).height, 0, "there is no requested height when domain is empty"); color.domain(["foo", "bar"]); var height1 = legend._requestedSpace(400, 400).height; - assert.operator(height1, ">", 0, "changing the domain gives a positive minimumHeight"); + var actualHeight1 = legend.availableHeight; + assert.operator(height1, ">", 0, "changing the domain gives a positive height"); color.domain(["foo", "bar", "baz"]); - assert.operator(legend._requestedSpace(400, 400).height, ">", height1, "adding to the domain increases the minimumHeight"); - legend.renderTo(svg); + assert.operator(legend._requestedSpace(400, 400).height, ">", height1, "adding to the domain increases the height requested"); + var actualHeight2 = legend.availableHeight; + assert.operator(actualHeight1, "<", actualHeight2, "Changing the domain caused the legend to re-layout with more height"); var numRows = legend.content.selectAll(".legend-row")[0].length; assert.equal(numRows, 3, "there are 3 rows"); svg.remove(); From 999828854af978ad419b668ac719119402dabf52 Mon Sep 17 00:00:00 2001 From: Daniel Mane Date: Thu, 15 May 2014 14:44:41 -0700 Subject: [PATCH 03/17] Missing svg.remove --- test/componentTests.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/test/componentTests.ts b/test/componentTests.ts index 07d202f539..99b1e4fd8c 100644 --- a/test/componentTests.ts +++ b/test/componentTests.ts @@ -334,5 +334,6 @@ it("components can be offset relative to their alignment, and throw errors if th c._invalidateLayout(); assert.equal(cg.availableHeight, 50, "invalidateLayout propagated to parent and caused resized height"); assert.equal(cg.availableWidth, 50, "invalidateLayout propagated to parent and caused resized width"); + svg.remove(); }); }); From 29dc0cef269c1b8d558d606d490093ff39a6cd4a Mon Sep 17 00:00:00 2001 From: Daniel Mane Date: Thu, 15 May 2014 14:47:28 -0700 Subject: [PATCH 04/17] Fix _anchor type sig on Table, ComponentGroup --- src/core/componentGroup.ts | 2 +- src/core/table.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/componentGroup.ts b/src/core/componentGroup.ts index 5c1bb18efb..1c6a5bde72 100644 --- a/src/core/componentGroup.ts +++ b/src/core/componentGroup.ts @@ -75,7 +75,7 @@ module Plottable { return this; } - public _anchor(element: D3.Selection, parent: Component): ComponentGroup { + public _anchor(element: D3.Selection, parent?: Component): ComponentGroup { super._anchor(element, parent); this.components.forEach((c) => c._anchor(this.content, this)); return this; diff --git a/src/core/table.ts b/src/core/table.ts index 32c567ddd0..240348874e 100644 --- a/src/core/table.ts +++ b/src/core/table.ts @@ -64,7 +64,7 @@ module Plottable { return this; } - public _anchor(element: D3.Selection, parent: Component) { + public _anchor(element: D3.Selection, parent?: Component) { super._anchor(element, parent); // recursively anchor children this.rows.forEach((row: Component[], rowIndex: number) => { From fa513a668e9a206624c77a5068e753521cdd5ed1 Mon Sep 17 00:00:00 2001 From: Daniel Mane Date: Thu, 15 May 2014 14:50:56 -0700 Subject: [PATCH 05/17] Missing semicolon! ;) --- src/core/component.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/component.ts b/src/core/component.ts index 9eb10f51e6..cec73def8f 100644 --- a/src/core/component.ts +++ b/src/core/component.ts @@ -167,7 +167,7 @@ module Plottable { if (this.isAnchored && this.isSetup) { RenderController.registerToComputeLayout(this); } - return this + return this; } public _doRender() { From 1954fdce4c1778b06d2e6e5c79b613abb29a584f Mon Sep 17 00:00:00 2001 From: Daniel Mane Date: Thu, 15 May 2014 15:44:06 -0700 Subject: [PATCH 06/17] Component.remove() now takes responsibility for detatching the component from the Plottable Component tree. We accomplish this by creating the class AbstractComponentContainer with a _removeComponent method that detaches that component from the tree. _removeComponent not yet implemented on Table. --- src/core/abstractComponentContainer.ts | 30 +++++++++++++ src/core/component.ts | 10 +++-- src/core/componentGroup.ts | 58 +++++++------------------- src/core/table.ts | 14 +++++-- src/reference.ts | 1 + test/componentGroupTests.ts | 10 ++--- test/componentTests.ts | 6 +-- 7 files changed, 69 insertions(+), 60 deletions(-) create mode 100644 src/core/abstractComponentContainer.ts diff --git a/src/core/abstractComponentContainer.ts b/src/core/abstractComponentContainer.ts new file mode 100644 index 0000000000..8a97701d73 --- /dev/null +++ b/src/core/abstractComponentContainer.ts @@ -0,0 +1,30 @@ +/// + +module Plottable { + export class AbstractComponentContainer extends Component { + public _components: Component[] = []; + public _removeComponent(c: Component) { + var removeIndex = this._components.indexOf(c); + if (removeIndex < 0) { + throw new Error("Attempt to remove component, but component not found"); + } + this._components.splice(removeIndex, 1); + this._invalidateLayout(); + return this; + } + + public _addComponent(c: Component) { + this._components.push(c); + this._invalidateLayout(); + return this; + } + + public getComponents(): Component[] { + return this._components; + } + + public empty() { + this._components.forEach((c) => this._removeComponent(c)); + } + } +} diff --git a/src/core/component.ts b/src/core/component.ts index cec73def8f..0e6820ee3b 100644 --- a/src/core/component.ts +++ b/src/core/component.ts @@ -16,7 +16,7 @@ module Plottable { private rootSVG: D3.Selection; private isTopLevelComponent = false; - private parent: Component; + private parent: AbstractComponentContainer; public availableWidth : number; // Width and height of the component. Used to size the hitbox, bounding box, etc public availableHeight: number; @@ -38,7 +38,7 @@ module Plottable { * @param {D3.Selection} element A D3 selection consisting of the element to anchor under. * @returns {Component} The calling component. */ - public _anchor(element: D3.Selection, parent?: Component) { + public _anchor(element: D3.Selection, parent?: AbstractComponentContainer) { if (element.node().nodeName === "svg") { // svg node gets the "plottable" CSS class this.rootSVG = element; @@ -416,7 +416,7 @@ module Plottable { } if (ComponentGroup.prototype.isPrototypeOf(c)) { cg = ( c); - cg._addComponentToGroup(this, true); + cg._addComponent(this, true); return cg; } else { cg = new ComponentGroup([this, c]); @@ -429,7 +429,9 @@ module Plottable { */ public remove() { this.element.remove(); - this._invalidateLayout(); + if (this.parent != null) { + this.parent._removeComponent(this); + } this.isAnchored = false; this.parent = null; return this; diff --git a/src/core/componentGroup.ts b/src/core/componentGroup.ts index 1c6a5bde72..16f652c75c 100644 --- a/src/core/componentGroup.ts +++ b/src/core/componentGroup.ts @@ -1,8 +1,7 @@ /// module Plottable { - export class ComponentGroup extends Component { - private components: Component[]; + export class ComponentGroup extends AbstractComponentContainer { /** * Creates a ComponentGroup. @@ -13,11 +12,11 @@ module Plottable { constructor(components: Component[] = []){ super(); this.classed("component-group", true); - this.components = components; + components.forEach((c: Component) => this._addComponent(c)); } public _requestedSpace(offeredWidth: number, offeredHeight: number): ISpaceRequest { - var requests = this.components.map((c: Component) => c._requestedSpace(offeredWidth, offeredHeight)); + var requests = this._components.map((c: Component) => c._requestedSpace(offeredWidth, offeredHeight)); var desiredWidth = d3.max(requests, (l: ISpaceRequest) => l.width ); var desiredHeight = d3.max(requests, (l: ISpaceRequest) => l.height); return { @@ -28,56 +27,27 @@ module Plottable { }; } - public _addComponentToGroup(c: Component, prepend = false): ComponentGroup { - this._invalidateLayout(); + public _addComponent(c: Component, prepend = false): ComponentGroup { if (prepend) { - this.components.unshift(c); + this._components.unshift(c); } else { - this.components.push(c); + this._components.push(c); } if (this.element != null) { c._anchor(this.content, this); } + this._invalidateLayout(); return this; } public merge(c: Component): ComponentGroup { - this._addComponentToGroup(c); - return this; - } - - /** - * If the given component exists in the ComponentGroup, removes it from the - * group and the DOM. - * - * @param {Component} c The component to be removed. - * @returns {ComponentGroup} The calling ComponentGroup. - */ - public removeComponent(c: Component): ComponentGroup { - var removeIndex = this.components.indexOf(c); - if (removeIndex >= 0) { - this.components.splice(removeIndex, 1); - c.remove(); - this._invalidateLayout(); - } - return this; - } - - /** - * Removes all Components in the ComponentGroup from the group and the DOM. - * - * @returns {ComponentGroup} The calling ComponentGroup. - */ - public empty(): ComponentGroup { - this.components.forEach((c: Component) => c.remove()); - this.components = []; - this._invalidateLayout(); + this._addComponent(c); return this; } - public _anchor(element: D3.Selection, parent?: Component): ComponentGroup { + public _anchor(element: D3.Selection, parent?: AbstractComponentContainer): ComponentGroup { super._anchor(element, parent); - this.components.forEach((c) => c._anchor(this.content, this)); + this._components.forEach((c) => c._anchor(this.content, this)); return this; } @@ -86,7 +56,7 @@ module Plottable { availableWidth?: number, availableHeight?: number): ComponentGroup { super._computeLayout(xOrigin, yOrigin, availableWidth, availableHeight); - this.components.forEach((c) => { + this._components.forEach((c) => { c._computeLayout(0, 0, this.availableWidth, this.availableHeight); }); return this; @@ -94,16 +64,16 @@ module Plottable { public _doRender() { super._doRender(); - this.components.forEach((c) => c._doRender()); + this._components.forEach((c) => c._doRender()); return this; } public isFixedWidth(): boolean { - return this.components.every((c) => c.isFixedWidth()); + return this._components.every((c) => c.isFixedWidth()); } public isFixedHeight(): boolean { - return this.components.every((c) => c.isFixedHeight()); + return this._components.every((c) => c.isFixedHeight()); } } } diff --git a/src/core/table.ts b/src/core/table.ts index 240348874e..61d3257d11 100644 --- a/src/core/table.ts +++ b/src/core/table.ts @@ -8,7 +8,7 @@ module Plottable { wantsWidthArr : boolean[]; wantsHeightArr: boolean[]; } - export class Table extends Component { + export class Table extends AbstractComponentContainer { private rowPadding = 0; private colPadding = 0; @@ -48,7 +48,7 @@ module Plottable { */ public addComponent(row: number, col: number, component: Component): Table { if (this.element != null) { - throw new Error("addComponent cannot be called after anchoring (for the moment)"); + throw new Error("Table.addComponent cannot be called after anchoring (for the moment)"); } this.nRows = Math.max(row + 1, this.nRows); @@ -57,14 +57,20 @@ module Plottable { var currentComponent = this.rows[row][col]; if (currentComponent != null) { - throw new Error("addComponent cannot be called on a cell where a component already exists (for the moment)"); + throw new Error("Table.addComponent cannot be called on a cell where a component already exists (for the moment)"); } this.rows[row][col] = component; + this._addComponent(component); return this; } - public _anchor(element: D3.Selection, parent?: Component) { + public _removeComponent(c: Component) { + throw new Error("_removeComponent not yet implemented on Table"); + return this; + } + + public _anchor(element: D3.Selection, parent?: AbstractComponentContainer) { super._anchor(element, parent); // recursively anchor children this.rows.forEach((row: Component[], rowIndex: number) => { diff --git a/src/reference.ts b/src/reference.ts index 14bd76b1c1..d2711e2115 100644 --- a/src/reference.ts +++ b/src/reference.ts @@ -5,6 +5,7 @@ /// /// /// +/// /// /// /// diff --git a/test/componentGroupTests.ts b/test/componentGroupTests.ts index edd75155f5..5210fa3152 100644 --- a/test/componentGroupTests.ts +++ b/test/componentGroupTests.ts @@ -102,7 +102,7 @@ describe("ComponentGroups", () => { assert.isNotNull(c1Node, "component 1 was added to the DOM"); assert.isNotNull(c2Node, "component 2 was added to the DOM"); - cg.removeComponent(c2); + c2.remove(); c1Node = svg.select(".component-1").node(); c2Node = svg.select(".comopnent-2").node(); @@ -135,7 +135,7 @@ describe("ComponentGroups", () => { it("Component.merge works as expected (Component.merge Component)", () => { var cg: Plottable.ComponentGroup = c1.merge(c2); - var innerComponents: Plottable.Component[] = ( cg).components; + var innerComponents: Plottable.Component[] = cg._components; assert.lengthOf(innerComponents, 2, "There are two components"); assert.equal(innerComponents[0], c1, "first component correct"); assert.equal(innerComponents[1], c2, "second component correct"); @@ -145,7 +145,7 @@ describe("ComponentGroups", () => { var cg = new Plottable.ComponentGroup([c2,c3,c4]); var cg2 = c1.merge(cg); assert.equal(cg, cg2, "c.merge(cg) returns cg"); - var components: Plottable.Component[] = ( cg).components; + var components: Plottable.Component[] = cg._components; assert.lengthOf(components, 4, "four components"); assert.equal(components[0], c1, "first component in front"); assert.equal(components[1], c2, "second component is second"); @@ -155,7 +155,7 @@ describe("ComponentGroups", () => { var cg = new Plottable.ComponentGroup([c1,c2,c3]); var cg2 = cg.merge(c4); assert.equal(cg, cg2, "cg.merge(c) returns cg"); - var components: Plottable.Component[] = ( cg).components; + var components: Plottable.Component[] = cg._components; assert.lengthOf(components, 4, "there are four components"); assert.equal(components[0], c1, "first is first"); assert.equal(components[3], c4, "fourth is fourth"); @@ -167,7 +167,7 @@ describe("ComponentGroups", () => { var cg = cg1.merge(cg2); assert.equal(cg, cg1, "merged == cg1"); assert.notEqual(cg, cg2, "merged != cg2"); - var components: Plottable.Component[] = ( cg).components; + var components: Plottable.Component[] = cg._components; assert.lengthOf(components, 3, "there are three inner components"); assert.equal(components[0], c1, "components are inside"); assert.equal(components[1], c2, "components are inside"); diff --git a/test/componentTests.ts b/test/componentTests.ts index 99b1e4fd8c..c5f489a3f0 100644 --- a/test/componentTests.ts +++ b/test/componentTests.ts @@ -96,7 +96,7 @@ describe("Component behavior", () => { it("computeLayout will not default when attached to non-root node", () => { var g = svg.append("g"); - c._anchor(g, new Plottable.Component()); + c._anchor(g, new Plottable.ComponentGroup()); assert.throws(() => c._computeLayout(), "null arguments"); svg.remove(); }); @@ -112,7 +112,7 @@ describe("Component behavior", () => { var yOff = 20; var width = 100; var height = 200; - c._anchor(g, new Plottable.Component())._computeLayout(xOff, yOff, width, height); + c._anchor(g, new Plottable.ComponentGroup())._computeLayout(xOff, yOff, width, height); var translate = getTranslate(c.element); assert.deepEqual(translate, [xOff, yOff], "the element translated appropriately"); assert.equal(c.availableWidth , width, "the width set properly"); @@ -326,7 +326,7 @@ it("components can be offset relative to their alignment, and throw errors if th it("_invalidateLayout works as expected", () => { var cg = new Plottable.ComponentGroup(); var c = makeFixedSizeComponent(10, 10); - cg._addComponentToGroup(c); + cg._addComponent(c); cg.renderTo(svg); assert.equal(cg.availableHeight, 10, "availableHeight initially 10 for fixed-size component"); assert.equal(cg.availableWidth, 10, "availableWidth initially 10 for fixed-size component"); From f40a631bcce302d6717335722068246286409730 Mon Sep 17 00:00:00 2001 From: Daniel Mane Date: Thu, 15 May 2014 15:45:26 -0700 Subject: [PATCH 07/17] Update built files --- plottable.d.ts | 33 ++++---- plottable.js | 219 ++++++++++++++++++++++++++++++++----------------- test/tests.js | 156 +++++++++++++++++++++++------------ 3 files changed, 266 insertions(+), 142 deletions(-) diff --git a/plottable.d.ts b/plottable.d.ts index 10789b64f7..ce1508ce6c 100644 --- a/plottable.d.ts +++ b/plottable.d.ts @@ -314,7 +314,13 @@ declare module Plottable { } } declare module Plottable { - class ComponentGroup extends Component { + class AbstractComponentContainer extends Component { + public getComponents(): Component[]; + public empty(): void; + } +} +declare module Plottable { + class ComponentGroup extends AbstractComponentContainer { /** * Creates a ComponentGroup. * @@ -323,26 +329,12 @@ declare module Plottable { */ constructor(components?: Component[]); public merge(c: Component): ComponentGroup; - /** - * If the given component exists in the ComponentGroup, removes it from the - * group and the DOM. - * - * @param {Component} c The component to be removed. - * @returns {ComponentGroup} The calling ComponentGroup. - */ - public removeComponent(c: Component): ComponentGroup; - /** - * Removes all Components in the ComponentGroup from the group and the DOM. - * - * @returns {ComponentGroup} The calling ComponentGroup. - */ - public empty(): ComponentGroup; public isFixedWidth(): boolean; public isFixedHeight(): boolean; } } declare module Plottable { - class Table extends Component { + class Table extends AbstractComponentContainer { /** * Creates a Table. * @@ -474,6 +466,7 @@ declare module Plottable { class RenderController { static enabled: boolean; static registerToRender(c: Component): void; + static registerToComputeLayout(c: Component): void; static doRender(): void; } } @@ -1114,8 +1107,6 @@ declare module Plottable { public showEndTickLabels(show: boolean): Axis; public scale(): Scale; public scale(newScale: Scale): Axis; - public formatter(): (d: any) => string; - public formatter(formatFunction: (d: any) => string): Axis; /** * Sets or gets the tick label position relative to the tick marks. * The exact consequences of particular tick label positionings depends on the subclass implementation. @@ -1140,6 +1131,12 @@ declare module Plottable { public outerTickSize(val: number): Axis; public tickPadding(): number; public tickPadding(val: number): Axis; + /** + * Gets the current tick formatting function, or sets the tick formatting function. + * + * @param {(value: any) => string} [formatter] The new tick formatting function. + * @returns The current tick formatting function, or the calling Axis. + */ public tickFormat(): (value: any) => string; public tickFormat(formatter: (value: any) => string): Axis; } diff --git a/plottable.js b/plottable.js index 01e0d27e27..98049c270c 100644 --- a/plottable.js +++ b/plottable.js @@ -551,7 +551,7 @@ var Plottable; * @param {D3.Selection} element A D3 selection consisting of the element to anchor under. * @returns {Component} The calling component. */ - Component.prototype._anchor = function (element) { + Component.prototype._anchor = function (element, parent) { if (element.node().nodeName === "svg") { // svg node gets the "plottable" CSS class this.rootSVG = element; @@ -561,6 +561,10 @@ var Plottable; this.rootSVG.style("overflow", "visible"); this.isTopLevelComponent = true; } + if (parent == null && !this.isTopLevelComponent) { + throw new Error("Components must be top-level or have a parent"); + } + this.parent = parent; if (this.element != null) { // reattach existing element @@ -680,10 +684,27 @@ var Plottable; return this; }; + Component.prototype._scheduleComputeLayout = function () { + if (this.isAnchored && this.isSetup) { + Plottable.RenderController.registerToComputeLayout(this); + } + return this; + }; + Component.prototype._doRender = function () { return this; }; + Component.prototype._invalidateLayout = function () { + if (this.isAnchored && this.isSetup) { + if (this.isTopLevelComponent) { + this._scheduleComputeLayout(); + } else { + this.parent._invalidateLayout(); + } + } + }; + /** * Renders the Component into a given DOM element. * @@ -698,7 +719,7 @@ var Plottable; } else { selection = d3.select(element); } - this._anchor(selection); + this._anchor(selection, null); } this._computeLayout()._render(); return this; @@ -734,6 +755,7 @@ var Plottable; } else { throw new Error("Unsupported alignment"); } + this._invalidateLayout(); return this; }; @@ -754,6 +776,7 @@ var Plottable; } else { throw new Error("Unsupported alignment"); } + this._invalidateLayout(); return this; }; @@ -765,6 +788,7 @@ var Plottable; */ Component.prototype.xOffset = function (offset) { this._xOffset = offset; + this._invalidateLayout(); return this; }; @@ -776,6 +800,7 @@ var Plottable; */ Component.prototype.yOffset = function (offset) { this._yOffset = offset; + this._invalidateLayout(); return this; }; @@ -899,9 +924,12 @@ var Plottable; */ Component.prototype.merge = function (c) { var cg; + if (this.isSetup || this.isAnchored) { + throw new Error("Can't presently merge a component that's already been anchored"); + } if (Plottable.ComponentGroup.prototype.isPrototypeOf(c)) { cg = c; - cg._addComponentToGroup(this, true); + cg._addComponent(this, true); return cg; } else { cg = new Plottable.ComponentGroup([this, c]); @@ -914,7 +942,11 @@ var Plottable; */ Component.prototype.remove = function () { this.element.remove(); + if (this.parent != null) { + this.parent._removeComponent(this); + } this.isAnchored = false; + this.parent = null; return this; }; return Component; @@ -923,6 +955,45 @@ var Plottable; })(Plottable || (Plottable = {})); /// var Plottable; +(function (Plottable) { + var AbstractComponentContainer = (function (_super) { + __extends(AbstractComponentContainer, _super); + function AbstractComponentContainer() { + _super.apply(this, arguments); + this._components = []; + } + AbstractComponentContainer.prototype._removeComponent = function (c) { + var removeIndex = this._components.indexOf(c); + if (removeIndex < 0) { + throw new Error("Attempt to remove component, but component not found"); + } + this._components.splice(removeIndex, 1); + this._invalidateLayout(); + return this; + }; + + AbstractComponentContainer.prototype._addComponent = function (c) { + this._components.push(c); + this._invalidateLayout(); + return this; + }; + + AbstractComponentContainer.prototype.getComponents = function () { + return this._components; + }; + + AbstractComponentContainer.prototype.empty = function () { + var _this = this; + this._components.forEach(function (c) { + return _this._removeComponent(c); + }); + }; + return AbstractComponentContainer; + })(Plottable.Component); + Plottable.AbstractComponentContainer = AbstractComponentContainer; +})(Plottable || (Plottable = {})); +/// +var Plottable; (function (Plottable) { var ComponentGroup = (function (_super) { __extends(ComponentGroup, _super); @@ -934,12 +1005,15 @@ var Plottable; */ function ComponentGroup(components) { if (typeof components === "undefined") { components = []; } + var _this = this; _super.call(this); this.classed("component-group", true); - this.components = components; + components.forEach(function (c) { + return _this._addComponent(c); + }); } ComponentGroup.prototype._requestedSpace = function (offeredWidth, offeredHeight) { - var requests = this.components.map(function (c) { + var requests = this._components.map(function (c) { return c._requestedSpace(offeredWidth, offeredHeight); }); var desiredWidth = d3.max(requests, function (l) { @@ -956,58 +1030,30 @@ var Plottable; }; }; - ComponentGroup.prototype._addComponentToGroup = function (c, prepend) { + ComponentGroup.prototype._addComponent = function (c, prepend) { if (typeof prepend === "undefined") { prepend = false; } if (prepend) { - this.components.unshift(c); + this._components.unshift(c); } else { - this.components.push(c); + this._components.push(c); } if (this.element != null) { - c._anchor(this.content); + c._anchor(this.content, this); } + this._invalidateLayout(); return this; }; ComponentGroup.prototype.merge = function (c) { - this._addComponentToGroup(c); - return this; - }; - - /** - * If the given component exists in the ComponentGroup, removes it from the - * group and the DOM. - * - * @param {Component} c The component to be removed. - * @returns {ComponentGroup} The calling ComponentGroup. - */ - ComponentGroup.prototype.removeComponent = function (c) { - var removeIndex = this.components.indexOf(c); - if (removeIndex >= 0) { - this.components.splice(removeIndex, 1); - c.remove(); - } + this._addComponent(c); return this; }; - /** - * Removes all Components in the ComponentGroup from the group and the DOM. - * - * @returns {ComponentGroup} The calling ComponentGroup. - */ - ComponentGroup.prototype.empty = function () { - this.components.forEach(function (c) { - return c.remove(); - }); - this.components = []; - return this; - }; - - ComponentGroup.prototype._anchor = function (element) { + ComponentGroup.prototype._anchor = function (element, parent) { var _this = this; - _super.prototype._anchor.call(this, element); - this.components.forEach(function (c) { - return c._anchor(_this.content); + _super.prototype._anchor.call(this, element, parent); + this._components.forEach(function (c) { + return c._anchor(_this.content, _this); }); return this; }; @@ -1015,7 +1061,7 @@ var Plottable; ComponentGroup.prototype._computeLayout = function (xOrigin, yOrigin, availableWidth, availableHeight) { var _this = this; _super.prototype._computeLayout.call(this, xOrigin, yOrigin, availableWidth, availableHeight); - this.components.forEach(function (c) { + this._components.forEach(function (c) { c._computeLayout(0, 0, _this.availableWidth, _this.availableHeight); }); return this; @@ -1023,25 +1069,25 @@ var Plottable; ComponentGroup.prototype._doRender = function () { _super.prototype._doRender.call(this); - this.components.forEach(function (c) { + this._components.forEach(function (c) { return c._doRender(); }); return this; }; ComponentGroup.prototype.isFixedWidth = function () { - return this.components.every(function (c) { + return this._components.every(function (c) { return c.isFixedWidth(); }); }; ComponentGroup.prototype.isFixedHeight = function () { - return this.components.every(function (c) { + return this._components.every(function (c) { return c.isFixedHeight(); }); }; return ComponentGroup; - })(Plottable.Component); + })(Plottable.AbstractComponentContainer); Plottable.ComponentGroup = ComponentGroup; })(Plottable || (Plottable = {})); /// @@ -1083,7 +1129,7 @@ var Plottable; */ Table.prototype.addComponent = function (row, col, component) { if (this.element != null) { - throw new Error("addComponent cannot be called after anchoring (for the moment)"); + throw new Error("Table.addComponent cannot be called after anchoring (for the moment)"); } this.nRows = Math.max(row + 1, this.nRows); @@ -1092,22 +1138,28 @@ var Plottable; var currentComponent = this.rows[row][col]; if (currentComponent != null) { - throw new Error("addComponent cannot be called on a cell where a component already exists (for the moment)"); + throw new Error("Table.addComponent cannot be called on a cell where a component already exists (for the moment)"); } this.rows[row][col] = component; + this._addComponent(component); return this; }; - Table.prototype._anchor = function (element) { + Table.prototype._removeComponent = function (c) { + throw new Error("_removeComponent not yet implemented on Table"); + return this; + }; + + Table.prototype._anchor = function (element, parent) { var _this = this; - _super.prototype._anchor.call(this, element); + _super.prototype._anchor.call(this, element, parent); // recursively anchor children this.rows.forEach(function (row, rowIndex) { row.forEach(function (component, colIndex) { if (component != null) { - component._anchor(_this.content); + component._anchor(_this.content, _this); } }); }); @@ -1217,6 +1269,13 @@ var Plottable; break; } } + + // Redo the proportional space one last time, to ensure we use the real weights not the wantsWidth/Height weights + freeWidth = availableWidthAfterPadding - d3.sum(guarantees.guaranteedWidths); + freeHeight = availableHeightAfterPadding - d3.sum(guarantees.guaranteedHeights); + colProportionalSpace = Table.calcProportionalSpace(colWeights, freeWidth); + rowProportionalSpace = Table.calcProportionalSpace(rowWeights, freeHeight); + return { colProportionalSpace: colProportionalSpace, rowProportionalSpace: rowProportionalSpace, @@ -1312,6 +1371,7 @@ var Plottable; Table.prototype.padding = function (rowPadding, colPadding) { this.rowPadding = rowPadding; this.colPadding = colPadding; + this._invalidateLayout(); return this; }; @@ -1325,6 +1385,7 @@ var Plottable; */ Table.prototype.rowWeight = function (index, weight) { this.rowWeights[index] = weight; + this._invalidateLayout(); return this; }; @@ -1338,6 +1399,7 @@ var Plottable; */ Table.prototype.colWeight = function (index, weight) { this.colWeights[index] = weight; + this._invalidateLayout(); return this; }; @@ -1392,10 +1454,7 @@ var Plottable; Table.calcProportionalSpace = function (weights, freeSpace) { var weightSum = d3.sum(weights); if (weightSum === 0) { - var numGroups = weights.length; - return weights.map(function (w) { - return freeSpace / numGroups; - }); + return Plottable.Utils.createFilledArray(0, weights.length); } else { return weights.map(function (w) { return freeSpace * w / weightSum; @@ -1415,7 +1474,7 @@ var Plottable; return all(componentGroup.map(groupIsFixed)); }; return Table; - })(Plottable.Component); + })(Plottable.AbstractComponentContainer); Plottable.Table = Table; })(Plottable || (Plottable = {})); /// @@ -1679,6 +1738,20 @@ var Plottable; return; } RenderController.componentsNeedingRender[c._plottableID] = c; + RenderController.requestFrame(); + }; + + RenderController.registerToComputeLayout = function (c) { + if (!Plottable.RenderController.enabled) { + c._computeLayout()._render(); + return; + } + RenderController.componentsNeedingComputeLayout[c._plottableID] = c; + RenderController.componentsNeedingRender[c._plottableID] = c; + RenderController.requestFrame(); + }; + + RenderController.requestFrame = function () { if (!RenderController.animationRequested) { requestAnimationFrame(RenderController.doRender); RenderController.animationRequested = true; @@ -1686,14 +1759,20 @@ var Plottable; }; RenderController.doRender = function () { - var components = d3.values(RenderController.componentsNeedingRender); - components.forEach(function (c) { + var toCompute = d3.values(RenderController.componentsNeedingComputeLayout); + toCompute.forEach(function (c) { + return c._computeLayout(); + }); + var toRender = d3.values(RenderController.componentsNeedingRender); + toRender.forEach(function (c) { return c._doRender(); }); + RenderController.componentsNeedingComputeLayout = {}; RenderController.componentsNeedingRender = {}; RenderController.animationRequested = false; }; RenderController.componentsNeedingRender = {}; + RenderController.componentsNeedingComputeLayout = {}; RenderController.animationRequested = false; RenderController.enabled = window.PlottableTestCode == null && (window.requestAnimationFrame) != null; return RenderController; @@ -2307,6 +2386,7 @@ var Plottable; this.textElement.text(text); this.measureAndSetTextSize(); } + this._invalidateLayout(); return this; }; @@ -2407,7 +2487,7 @@ var Plottable; } this.colorScale = scale; this._registerToBroadcaster(this.colorScale, function () { - return _this._render(); + return _this._invalidateLayout(); }); return this; } else { @@ -3594,6 +3674,7 @@ var Plottable; /// /// /// +/// /// /// /// @@ -3664,8 +3745,7 @@ var Plottable; return d; }; } - this.formatFunction = formatter; - this.d3Axis.tickFormat(this.formatFunction); + this.tickFormat(formatter); this._registerToBroadcaster(this._axisScale, function () { return _this.rescale(); }); @@ -3785,16 +3865,6 @@ var Plottable; } }; - Axis.prototype.formatter = function (formatFunction) { - if (formatFunction == null) { - return this.formatFunction; - } - this.formatFunction = formatFunction; - this.d3Axis.tickFormat(this.formatFunction); - this._render(); - return this; - }; - Axis.prototype.tickLabelPosition = function (position) { if (position == null) { return this.tickPositioning; @@ -3883,6 +3953,7 @@ var Plottable; return this.d3Axis.tickFormat(); } else { this.d3Axis.tickFormat(formatter); + this._invalidateLayout(); return this; } }; @@ -3912,6 +3983,7 @@ var Plottable; } XAxis.prototype.height = function (h) { this._height = h; + this._invalidateLayout(); return this; }; @@ -4049,6 +4121,7 @@ var Plottable; YAxis.prototype.width = function (w) { this._width = w; + this._invalidateLayout(); return this; }; diff --git a/test/tests.js b/test/tests.js index 9b20ea3102..9c6be8ea93 100644 --- a/test/tests.js +++ b/test/tests.js @@ -119,7 +119,7 @@ describe("Axes", function () { var tickTexts = ticks.select("text")[0].map(function (t) { return d3.select(t).text(); }); - var generatedTicks = xScale.ticks().map(axis.formatter()); + var generatedTicks = xScale.ticks().map(axis.tickFormat()); assert.deepEqual(tickTexts, generatedTicks, "The correct tick texts are displayed"); svg.remove(); }); @@ -135,17 +135,17 @@ describe("Axes", function () { var tickTexts = svg.selectAll(".tick text")[0].map(function (t) { return d3.select(t).text(); }); - var generatedTicks = xScale.ticks().map(axis.formatter()); + var generatedTicks = xScale.ticks().map(axis.tickFormat()); assert.deepEqual(tickTexts, generatedTicks, "The correct tick texts are displayed"); var blarghFormatter = function (d) { return "blargh"; }; - axis.formatter(blarghFormatter); + axis.tickFormat(blarghFormatter); tickTexts = svg.selectAll(".tick text")[0].map(function (t) { return d3.select(t).text(); }); - generatedTicks = xScale.ticks().map(axis.formatter()); + generatedTicks = xScale.ticks().map(axis.tickFormat()); assert.deepEqual(tickTexts, generatedTicks, "Tick texts updated based on new formatter"); svg.remove(); @@ -492,7 +492,7 @@ describe("ComponentGroups", function () { var cg = new Plottable.ComponentGroup([c1, c2, c3]); var svg = generateSVG(400, 400); - cg._anchor(svg); + cg._anchor(svg, null); c1.addBox("test-box1"); c2.addBox("test-box2"); c3.addBox("test-box3"); @@ -513,7 +513,7 @@ describe("ComponentGroups", function () { var cg = new Plottable.ComponentGroup([c1]); var svg = generateSVG(400, 400); - cg.merge(c2)._anchor(svg); + cg.merge(c2)._anchor(svg, null); c1.addBox("test-box1"); c2.addBox("test-box2"); cg._computeLayout()._render(); @@ -554,7 +554,7 @@ describe("ComponentGroups", function () { cg.merge(c1).merge(c2); var svg = generateSVG(); - cg._anchor(svg); + cg._anchor(svg, null); cg._computeLayout(50, 50, 350, 350); var cgTranslate = d3.transform(cg.element.attr("transform")).translate; @@ -583,7 +583,7 @@ describe("ComponentGroups", function () { assert.isNotNull(c1Node, "component 1 was added to the DOM"); assert.isNotNull(c2Node, "component 2 was added to the DOM"); - cg.removeComponent(c2); + c2.remove(); c1Node = svg.select(".component-1").node(); c2Node = svg.select(".comopnent-2").node(); @@ -616,7 +616,7 @@ describe("ComponentGroups", function () { it("Component.merge works as expected (Component.merge Component)", function () { var cg = c1.merge(c2); - var innerComponents = cg.components; + var innerComponents = cg._components; assert.lengthOf(innerComponents, 2, "There are two components"); assert.equal(innerComponents[0], c1, "first component correct"); assert.equal(innerComponents[1], c2, "second component correct"); @@ -626,7 +626,7 @@ describe("ComponentGroups", function () { var cg = new Plottable.ComponentGroup([c2, c3, c4]); var cg2 = c1.merge(cg); assert.equal(cg, cg2, "c.merge(cg) returns cg"); - var components = cg.components; + var components = cg._components; assert.lengthOf(components, 4, "four components"); assert.equal(components[0], c1, "first component in front"); assert.equal(components[1], c2, "second component is second"); @@ -636,7 +636,7 @@ describe("ComponentGroups", function () { var cg = new Plottable.ComponentGroup([c1, c2, c3]); var cg2 = cg.merge(c4); assert.equal(cg, cg2, "cg.merge(c) returns cg"); - var components = cg.components; + var components = cg._components; assert.lengthOf(components, 4, "there are four components"); assert.equal(components[0], c1, "first is first"); assert.equal(components[3], c4, "fourth is fourth"); @@ -648,7 +648,7 @@ describe("ComponentGroups", function () { var cg = cg1.merge(cg2); assert.equal(cg, cg1, "merged == cg1"); assert.notEqual(cg, cg2, "merged != cg2"); - var components = cg.components; + var components = cg._components; assert.lengthOf(components, 3, "there are three inner components"); assert.equal(components[0], c1, "components are inside"); assert.equal(components[1], c2, "components are inside"); @@ -680,14 +680,14 @@ describe("Component behavior", function () { describe("anchor", function () { it("anchoring works as expected", function () { - c._anchor(svg); + c._anchor(svg, null); assert.equal(c.element.node(), svg.select("g").node(), "the component anchored to a beneath the "); assert.isTrue(svg.classed("plottable"), " was given \"plottable\" CSS class"); svg.remove(); }); it("can re-anchor to a different element", function () { - c._anchor(svg); + c._anchor(svg, null); var svg2 = generateSVG(SVG_WIDTH, SVG_HEIGHT); c._anchor(svg2); @@ -701,7 +701,7 @@ describe("Component behavior", function () { describe("computeLayout", function () { it("computeLayout defaults and updates intelligently", function () { - c._anchor(svg)._computeLayout(); + c._anchor(svg, null)._computeLayout(); assert.equal(c.availableWidth, SVG_WIDTH, "computeLayout defaulted width to svg width"); assert.equal(c.availableHeight, SVG_HEIGHT, "computeLayout defaulted height to svg height"); assert.equal(c.xOrigin, 0, "xOrigin defaulted to 0"); @@ -728,7 +728,7 @@ describe("Component behavior", function () { svg.style("width", "50%"); svg.style("height", "50%"); - c._anchor(svg)._computeLayout(); + c._anchor(svg, null)._computeLayout(); assert.equal(c.availableWidth, 100, "computeLayout defaulted width to svg width"); assert.equal(c.availableHeight, 200, "computeLayout defaulted height to svg height"); @@ -752,7 +752,7 @@ describe("Component behavior", function () { it("computeLayout will not default when attached to non-root node", function () { var g = svg.append("g"); - c._anchor(g); + c._anchor(g, new Plottable.ComponentGroup()); assert.throws(function () { return c._computeLayout(); }, "null arguments"); @@ -772,7 +772,7 @@ describe("Component behavior", function () { var yOff = 20; var width = 100; var height = 200; - c._anchor(g)._computeLayout(xOff, yOff, width, height); + c._anchor(g, new Plottable.ComponentGroup())._computeLayout(xOff, yOff, width, height); var translate = getTranslate(c.element); assert.deepEqual(translate, [xOff, yOff], "the element translated appropriately"); assert.equal(c.availableWidth, width, "the width set properly"); @@ -797,7 +797,7 @@ describe("Component behavior", function () { it("fixed-width component will align to the right spot", function () { fixComponentSize(c, 100, 100); - c._anchor(svg); + c._anchor(svg, null); c._computeLayout(); assertComponentXY(c, 0, 0, "top-left component aligns correctly"); @@ -813,7 +813,7 @@ describe("Component behavior", function () { it("components can be offset relative to their alignment, and throw errors if there is insufficient space", function () { fixComponentSize(c, 100, 100); - c._anchor(svg); + c._anchor(svg, null); c.xOffset(20).yOffset(20); c._computeLayout(); assertComponentXY(c, 20, 20, "top-left component offsets correctly"); @@ -854,7 +854,7 @@ describe("Component behavior", function () { assert.isFalse(c.clipPathEnabled, "clipPathEnabled defaults to false"); c.clipPathEnabled = true; var expectedClipPathID = c._plottableID; - c._anchor(svg)._computeLayout(0, 0, 100, 100)._render(); + c._anchor(svg, null)._computeLayout(0, 0, 100, 100)._render(); var expectedClipPathURL = "url(#clipPath" + expectedClipPathID + ")"; assert.equal(c.element.attr("clip-path"), expectedClipPathURL, "the element has clip-path url attached"); var clipRect = c.boxContainer.select(".clip-rect"); @@ -902,20 +902,20 @@ describe("Component behavior", function () { assert.equal(hitBox.style("opacity"), "0", "the hitBox is transparent, otherwise it would look weird"); } - c._anchor(svg); + c._anchor(svg, null); assert.isUndefined(c.hitBox, "no hitBox was created when there were no registered interactions"); svg.remove(); svg = generateSVG(); c = new Plottable.Component(); var i = new Plottable.Interaction(c).registerWithComponent(); - c._anchor(svg); + c._anchor(svg, null); verifyHitbox(c); svg.remove(); svg = generateSVG(); c = new Plottable.Component(); - c._anchor(svg); + c._anchor(svg, null); i = new Plottable.Interaction(c).registerWithComponent(); verifyHitbox(c); svg.remove(); @@ -958,7 +958,7 @@ describe("Component behavior", function () { c.classed("CSS-PREANCHOR-REMOVE", false); assert.isFalse(c.classed("CSS-PREANCHOR-REMOVE")); - c._anchor(svg); + c._anchor(svg, null); assert.isTrue(c.classed("CSS-PREANCHOR-KEEP")); assert.isFalse(c.classed("CSS-PREANCHOR-REMOVE")); assert.isFalse(c.classed("CSS-POSTANCHOR")); @@ -994,6 +994,20 @@ describe("Component behavior", function () { svg.remove(); }); + + it("_invalidateLayout works as expected", function () { + var cg = new Plottable.ComponentGroup(); + var c = makeFixedSizeComponent(10, 10); + cg._addComponent(c); + cg.renderTo(svg); + assert.equal(cg.availableHeight, 10, "availableHeight initially 10 for fixed-size component"); + assert.equal(cg.availableWidth, 10, "availableWidth initially 10 for fixed-size component"); + fixComponentSize(c, 50, 50); + c._invalidateLayout(); + assert.equal(cg.availableHeight, 50, "invalidateLayout propagated to parent and caused resized height"); + assert.equal(cg.availableWidth, 50, "invalidateLayout propagated to parent and caused resized width"); + svg.remove(); + }); }); /// var assert = chai.assert; @@ -1092,7 +1106,7 @@ describe("Gridlines", function () { var gridlines = new Plottable.Gridlines(xScale, yScale); var basicTable = new Plottable.Table().addComponent(0, 0, yAxis).addComponent(0, 1, gridlines).addComponent(1, 1, xAxis); - basicTable._anchor(svg); + basicTable._anchor(svg, null); basicTable._computeLayout(); xScale.range([0, xAxis.availableWidth]); // manually set range since we don't have a renderer yScale.range([yAxis.availableHeight, 0]); @@ -1332,7 +1346,7 @@ describe("Labels", function () { it("Standard text title label generates properly", function () { var svg = generateSVG(400, 80); var label = new Plottable.TitleLabel("A CHART TITLE"); - label._anchor(svg); + label._anchor(svg, null); label._computeLayout(); var content = label.content; @@ -1351,7 +1365,7 @@ describe("Labels", function () { it("Left-rotated text is handled properly", function () { var svg = generateSVG(100, 400); var label = new Plottable.AxisLabel("LEFT-ROTATED LABEL", "vertical-left"); - label._anchor(svg); + label._anchor(svg, null); var content = label.content; var text = content.select("text"); label._computeLayout(); @@ -1366,7 +1380,7 @@ describe("Labels", function () { it("Right-rotated text is handled properly", function () { var svg = generateSVG(100, 400); var label = new Plottable.AxisLabel("RIGHT-ROTATED LABEL", "vertical-right"); - label._anchor(svg); + label._anchor(svg, null); var content = label.content; var text = content.select("text"); label._computeLayout(); @@ -1396,7 +1410,7 @@ describe("Labels", function () { var svgWidth = 400; var svg = generateSVG(svgWidth, 80); var label = new Plottable.TitleLabel("THIS LABEL IS SO LONG WHOEVER WROTE IT WAS PROBABLY DERANGED"); - label._anchor(svg); + label._anchor(svg, null); var content = label.content; var text = content.select("text"); label._computeLayout(); @@ -1465,16 +1479,18 @@ describe("Legends", function () { svg.remove(); }); - it("legend domain can be updated after initialization, and minimumHeight updates as well", function () { + it("legend domain can be updated after initialization, and height updates as well", function () { legend.renderTo(svg); legend.scale(color); assert.equal(legend._requestedSpace(200, 200).height, 0, "there is no requested height when domain is empty"); color.domain(["foo", "bar"]); var height1 = legend._requestedSpace(400, 400).height; - assert.operator(height1, ">", 0, "changing the domain gives a positive minimumHeight"); + var actualHeight1 = legend.availableHeight; + assert.operator(height1, ">", 0, "changing the domain gives a positive height"); color.domain(["foo", "bar", "baz"]); - assert.operator(legend._requestedSpace(400, 400).height, ">", height1, "adding to the domain increases the minimumHeight"); - legend.renderTo(svg); + assert.operator(legend._requestedSpace(400, 400).height, ">", height1, "adding to the domain increases the height requested"); + var actualHeight2 = legend.availableHeight; + assert.operator(actualHeight1, "<", actualHeight2, "Changing the domain caused the legend to re-layout with more height"); var numRows = legend.content.selectAll(".legend-row")[0].length; assert.equal(numRows, 3, "there are 3 rows"); svg.remove(); @@ -2737,18 +2753,8 @@ describe("Tables", function () { verifySpaceRequest(spaceRequest, 70, 100, false, false, "4"); }); - it("table.iterateLayout works properly", function () { - // This unit test would have caught #405 - var c1 = new Plottable.Component(); - var c2 = new Plottable.Component(); - var c3 = new Plottable.Component(); - var c4 = new Plottable.Component(); - fixComponentSize(c1, 50, 50); - fixComponentSize(c4, 20, 10); - var table = new Plottable.Table([ - [c1, c2], - [c3, c4]]); - + describe("table.iterateLayout works properly", function () { + // This test battery would have caught #405 function verifyLayoutResult(result, cPS, rPS, gW, gH, wW, wH, id) { assert.deepEqual(result.colProportionalSpace, cPS, "colProportionalSpace:" + id); assert.deepEqual(result.rowProportionalSpace, rPS, "rowProportionalSpace:" + id); @@ -2758,12 +2764,60 @@ describe("Tables", function () { assert.deepEqual(result.wantsHeight, wH, "wantsHeight:" + id); } - var result = table.iterateLayout(500, 500); - verifyLayoutResult(result, [215, 215], [220, 220], [50, 20], [50, 10], false, false, "1"); + var c1 = new Plottable.Component(); + var c2 = new Plottable.Component(); + var c3 = new Plottable.Component(); + var c4 = new Plottable.Component(); + var table = new Plottable.Table([ + [c1, c2], + [c3, c4]]); + + it("iterateLayout works in the easy case where there is plenty of space and everything is satisfied on first go", function () { + fixComponentSize(c1, 50, 50); + fixComponentSize(c4, 20, 10); + var result = table.iterateLayout(500, 500); + verifyLayoutResult(result, [215, 215], [220, 220], [50, 20], [50, 10], false, false, ""); + }); + + it("iterateLayout works in the difficult case where there is a shortage of space and layout requires iterations", function () { + fixComponentSize(c1, 490, 50); + var result = table.iterateLayout(500, 500); + verifyLayoutResult(result, [0, 0], [220, 220], [480, 20], [50, 10], true, false, ""); + }); - fixComponentSize(c1, 490, 50); - result = table.iterateLayout(500, 500); - verifyLayoutResult(result, [0, 0], [220, 220], [480, 20], [50, 10], true, false, "2"); + it("iterateLayout works in the case where all components are fixed-size", function () { + fixComponentSize(c1, 50, 50); + fixComponentSize(c2, 50, 50); + fixComponentSize(c3, 50, 50); + fixComponentSize(c4, 50, 50); + var result = table.iterateLayout(100, 100); + verifyLayoutResult(result, [0, 0], [0, 0], [50, 50], [50, 50], false, false, "..when there's exactly enough space"); + + result = table.iterateLayout(80, 80); + verifyLayoutResult(result, [0, 0], [0, 0], [40, 40], [40, 40], true, true, "..when there's not enough space"); + + result = table.iterateLayout(120, 120); + + // If there is extra space in a fixed-size table, the extra space should not be allocated to proportional space + verifyLayoutResult(result, [0, 0], [0, 0], [50, 50], [50, 50], false, false, "..when there's extra space"); + }); + + it("iterateLayout works in the tricky case when components can be unsatisfied but request little space", function () { + table = new Plottable.Table([[c1, c2]]); + fixComponentSize(c1, null, null); + c2._requestedSpace = function (w, h) { + return { + width: w >= 200 ? 200 : 0, + height: h >= 200 ? 200 : 0, + wantsWidth: w < 200, + wantsHeight: h < 200 + }; + }; + var result = table.iterateLayout(200, 200); + verifyLayoutResult(result, [0, 0], [0], [0, 200], [200], false, false, "when there's sufficient space"); + result = table.iterateLayout(150, 200); + verifyLayoutResult(result, [150, 0], [0], [0, 0], [200], true, false, "when there's insufficient space"); + }); }); }); /// From 98520662922498581a6fc3e2fbc65399159eb1b3 Mon Sep 17 00:00:00 2001 From: Daniel Mane Date: Thu, 15 May 2014 15:53:21 -0700 Subject: [PATCH 08/17] Disable tslint no-unreachable for the on unimplemented function --- src/core/table.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/core/table.ts b/src/core/table.ts index 61d3257d11..9d6ace255f 100644 --- a/src/core/table.ts +++ b/src/core/table.ts @@ -67,7 +67,9 @@ module Plottable { public _removeComponent(c: Component) { throw new Error("_removeComponent not yet implemented on Table"); + /* tslint:disable:no-unreachable */ return this; + /* tslint:enable:no-unreachable */ } public _anchor(element: D3.Selection, parent?: AbstractComponentContainer) { From 81d95471823e16c23efac05565a84f90142397e6 Mon Sep 17 00:00:00 2001 From: Daniel Mane Date: Thu, 15 May 2014 16:13:23 -0700 Subject: [PATCH 09/17] Fix the label width bug that Cassie found, and add unit test --- src/components/label.ts | 2 +- src/utils.ts | 2 +- test/labelTests.ts | 9 +++++++++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/components/label.ts b/src/components/label.ts index 6d70af3717..19192bb609 100644 --- a/src/components/label.ts +++ b/src/components/label.ts @@ -71,7 +71,7 @@ module Plottable { private measureAndSetTextSize() { var bbox = Utils.getBBox(this.textElement); this.textHeight = bbox.height; - this.textLength = bbox.width; + this.textLength = this.text === "" ? 0 : bbox.width; } private truncateTextAndRemeasure(availableLength: number) { diff --git a/src/utils.ts b/src/utils.ts index 3177ce477b..d20645628f 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -107,7 +107,7 @@ module Plottable { export function getTextWidth(textElement: D3.Selection, text: string) { var originalText = textElement.text(); textElement.text(text); - var width = Utils.getBBox(textElement).width; + var width = text === "" ? 0 : Utils.getBBox(textElement).width; textElement.text(originalText); return width; } diff --git a/test/labelTests.ts b/test/labelTests.ts index 05f84b34f6..4a192f10e7 100644 --- a/test/labelTests.ts +++ b/test/labelTests.ts @@ -105,6 +105,15 @@ describe("Labels", () => { svg.remove(); }); + it("if a label text is changed to empty string, width updates to 0", () => { + var svg = generateSVG(400, 400); + var label = new Plottable.TitleLabel("foo"); + label.renderTo(svg); + label.setText(""); + assert.equal(label.availableWidth, 0, "width updated to 0"); + svg.remove(); + }); + it("unsupported alignments and orientations are unsupported", () => { assert.throws(() => new Plottable.Label("foo", "bar"), Error, "not a valid orientation"); }); From fbb6dd0ef53c4bda5c795132df23120a327218d5 Mon Sep 17 00:00:00 2001 From: Daniel Mane Date: Thu, 15 May 2014 16:17:44 -0700 Subject: [PATCH 10/17] Update built files --- plottable.js | 23 ++++++++++++++++------- test/tests.js | 9 +++++++++ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/plottable.js b/plottable.js index 98049c270c..f49bfdbea0 100644 --- a/plottable.js +++ b/plottable.js @@ -113,7 +113,7 @@ var Plottable; function getTextWidth(textElement, text) { var originalText = textElement.text(); textElement.text(text); - var width = Utils.getBBox(textElement).width; + var width = text === "" ? 0 : Utils.getBBox(textElement).width; textElement.text(originalText); return width; } @@ -1148,7 +1148,10 @@ var Plottable; Table.prototype._removeComponent = function (c) { throw new Error("_removeComponent not yet implemented on Table"); + + /* tslint:disable:no-unreachable */ return this; + /* tslint:enable:no-unreachable */ }; Table.prototype._anchor = function (element, parent) { @@ -2172,8 +2175,9 @@ var Plottable; scale = d3.scale.pow(); break; } - if (scale == null) + if (scale == null) { throw new Error("unknown quantitive scale type " + scaleType); + } return scale.range([0, 1]).interpolate(InterpolatedColorScale.interpolateColors(colors)); }; @@ -2189,8 +2193,10 @@ var Plottable; * values in hex ("#FFFFFF") or keywords ("white"). */ InterpolatedColorScale.interpolateColors = function (colors) { - if (colors.length < 2) + if (colors.length < 2) { throw new Error("Color scale arrays must have at least two elements."); + } + ; return function (ignored) { return function (t) { // Clamp t parameter to [0,1] @@ -2209,23 +2215,26 @@ var Plottable; }; InterpolatedColorScale.prototype.colorRange = function (colorRange) { - if (colorRange == null) + if (colorRange == null) { return this._colorRange; + } this._colorRange = this._resolveColorValues(colorRange); this._resetScale(); }; InterpolatedColorScale.prototype.scaleType = function (scaleType) { - if (scaleType == null) + if (scaleType == null) { return this._scaleType; + } this._scaleType = scaleType; this._resetScale(); }; InterpolatedColorScale.prototype._resetScale = function () { this._d3Scale = InterpolatedColorScale.getD3InterpolatedScale(this._colorRange, this._scaleType); - if (this._autoDomain) + if (this._autoDomain) { this.autoDomain(); + } this._broadcast(); }; @@ -2393,7 +2402,7 @@ var Plottable; Label.prototype.measureAndSetTextSize = function () { var bbox = Plottable.Utils.getBBox(this.textElement); this.textHeight = bbox.height; - this.textLength = bbox.width; + this.textLength = this.text === "" ? 0 : bbox.width; }; Label.prototype.truncateTextAndRemeasure = function (availableLength) { diff --git a/test/tests.js b/test/tests.js index 9c6be8ea93..6636a99ee7 100644 --- a/test/tests.js +++ b/test/tests.js @@ -1442,6 +1442,15 @@ describe("Labels", function () { svg.remove(); }); + it("if a label text is changed to empty string, width updates to 0", function () { + var svg = generateSVG(400, 400); + var label = new Plottable.TitleLabel("foo"); + label.renderTo(svg); + label.setText(""); + assert.equal(label.availableWidth, 0, "width updated to 0"); + svg.remove(); + }); + it("unsupported alignments and orientations are unsupported", function () { assert.throws(function () { return new Plottable.Label("foo", "bar"); From 3c1b4c5e5edd9b2a56f31af68fe4163ce8f24fc2 Mon Sep 17 00:00:00 2001 From: Daniel Mane Date: Fri, 16 May 2014 14:47:56 -0700 Subject: [PATCH 11/17] Removing component that is already out of the tree no longer throws an error. Close #425. --- src/core/abstractComponentContainer.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/core/abstractComponentContainer.ts b/src/core/abstractComponentContainer.ts index 8a97701d73..51a32a4d1a 100644 --- a/src/core/abstractComponentContainer.ts +++ b/src/core/abstractComponentContainer.ts @@ -5,11 +5,10 @@ module Plottable { public _components: Component[] = []; public _removeComponent(c: Component) { var removeIndex = this._components.indexOf(c); - if (removeIndex < 0) { - throw new Error("Attempt to remove component, but component not found"); + if (removeIndex >= 0) { + this._components.splice(removeIndex, 1); + this._invalidateLayout(); } - this._components.splice(removeIndex, 1); - this._invalidateLayout(); return this; } From 2d60094aa4ba3e0023349e97f5e90237d861f82f Mon Sep 17 00:00:00 2001 From: Daniel Mane Date: Fri, 16 May 2014 14:56:53 -0700 Subject: [PATCH 12/17] Change empty() to a boolean test, and add removeAll for ditching all the components --- src/core/abstractComponentContainer.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/core/abstractComponentContainer.ts b/src/core/abstractComponentContainer.ts index 51a32a4d1a..ed85cc111f 100644 --- a/src/core/abstractComponentContainer.ts +++ b/src/core/abstractComponentContainer.ts @@ -23,7 +23,11 @@ module Plottable { } public empty() { - this._components.forEach((c) => this._removeComponent(c)); + return this._components.length === 0; + } + + public removeAll() { + this._components.forEach((c: Component) => c.remove()); } } } From c23fa67d4a1655bec813d8f87fc96b02f6bea105 Mon Sep 17 00:00:00 2001 From: Daniel Mane Date: Fri, 16 May 2014 15:54:04 -0700 Subject: [PATCH 13/17] ComponentGroup returns sensible requestedSpace when empty, with unit testing --- src/core/componentGroup.ts | 9 +++++---- test/componentGroupTests.ts | 29 +++++++++++++++++++++++++++++ test/tableTests.ts | 7 ------- test/testUtils.ts | 7 +++++++ 4 files changed, 41 insertions(+), 11 deletions(-) diff --git a/src/core/componentGroup.ts b/src/core/componentGroup.ts index 16f652c75c..d553ab1742 100644 --- a/src/core/componentGroup.ts +++ b/src/core/componentGroup.ts @@ -17,13 +17,14 @@ module Plottable { public _requestedSpace(offeredWidth: number, offeredHeight: number): ISpaceRequest { var requests = this._components.map((c: Component) => c._requestedSpace(offeredWidth, offeredHeight)); - var desiredWidth = d3.max(requests, (l: ISpaceRequest) => l.width ); - var desiredHeight = d3.max(requests, (l: ISpaceRequest) => l.height); + var isEmpty = this.empty(); + var desiredWidth = isEmpty ? 0 : d3.max(requests, (l: ISpaceRequest) => l.width ); + var desiredHeight = isEmpty ? 0 : d3.max(requests, (l: ISpaceRequest) => l.height); return { width : Math.min(desiredWidth , offeredWidth ), height: Math.min(desiredHeight, offeredHeight), - wantsWidth : desiredWidth > offeredWidth, - wantsHeight: desiredHeight > offeredHeight + wantsWidth : isEmpty ? false : requests.map((r: ISpaceRequest) => r.wantsWidth ).some((x: boolean) => x), + wantsHeight: isEmpty ? false : requests.map((r: ISpaceRequest) => r.wantsHeight).some((x: boolean) => x) }; } diff --git a/test/componentGroupTests.ts b/test/componentGroupTests.ts index 5210fa3152..1e607ee26a 100644 --- a/test/componentGroupTests.ts +++ b/test/componentGroupTests.ts @@ -127,6 +127,35 @@ describe("ComponentGroups", () => { svg.remove(); }); + describe("ComponentGroup._requestedSpace works as expected", () => { + it("_works for an empty ComponentGroup", () => { + var cg = new Plottable.ComponentGroup(); + var request = cg._requestedSpace(10, 10); + verifySpaceRequest(request, 0, 0, false, false, ""); + }); + + it("works for a ComponentGroup with only proportional-size components", () => { + var cg = new Plottable.ComponentGroup(); + var c1 = new Plottable.Component(); + var c2 = new Plottable.Component(); + cg.merge(c1).merge(c2); + var request = cg._requestedSpace(10, 10); + verifySpaceRequest(request, 0, 0, false, false, ""); + }); + + it("works when there are fixed-size components", () => { + var cg = new Plottable.ComponentGroup(); + var c1 = new Plottable.Component(); + var c2 = new Plottable.Component(); + var c3 = new Plottable.Component(); + cg.merge(c1).merge(c2).merge(c3); + fixComponentSize(c1, null, 10); + fixComponentSize(c2, null, 50); + var request = cg._requestedSpace(10, 10); + verifySpaceRequest(request, 0, 10, false, true, ""); + }); + }); + describe("Component.merge works as expected", () => { var c1 = new Plottable.Component(); var c2 = new Plottable.Component(); diff --git a/test/tableTests.ts b/test/tableTests.ts index 33fb1f6b7f..cb67f365ef 100644 --- a/test/tableTests.ts +++ b/test/tableTests.ts @@ -193,13 +193,6 @@ describe("Tables", () => { var c2 = makeFixedSizeComponent(20, 50); var c3 = makeFixedSizeComponent(20, 20); - function verifySpaceRequest(sr: Plottable.ISpaceRequest, w: number, h: number, ww: boolean, wh: boolean, id: string) { - assert.equal(sr.width, w, "width requested is as expected #" + id); - assert.equal(sr.height, h, "height requested is as expected #" + id); - assert.equal(sr.wantsWidth, ww, "needs more width is as expected #" + id); - assert.equal(sr.wantsHeight, wh, "needs more height is as expected #" + id); - } - var table = new Plottable.Table([[c0, c1], [c2, c3]]); var spaceRequest = table._requestedSpace(30, 30); diff --git a/test/testUtils.ts b/test/testUtils.ts index ee73e9cc88..bad2e1c596 100644 --- a/test/testUtils.ts +++ b/test/testUtils.ts @@ -16,6 +16,13 @@ function getSVGParent(): D3.Selection { } } +function verifySpaceRequest(sr: Plottable.ISpaceRequest, w: number, h: number, ww: boolean, wh: boolean, id: string) { + assert.equal(sr.width, w, "width requested is as expected #" + id); + assert.equal(sr.height, h, "height requested is as expected #" + id); + assert.equal(sr.wantsWidth , ww, "needs more width is as expected #" + id); + assert.equal(sr.wantsHeight, wh, "needs more height is as expected #" + id); +} + function fixComponentSize(c: Plottable.Component, fixedWidth?: number, fixedHeight?: number) { c._requestedSpace = function(w, h) { return { From 87eb97dc62ff1b9e5d1198a02b9570c66c95c2b4 Mon Sep 17 00:00:00 2001 From: Daniel Mane Date: Fri, 16 May 2014 16:02:45 -0700 Subject: [PATCH 14/17] Add JSDoc to AbstractComponentContainer --- src/core/abstractComponentContainer.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/core/abstractComponentContainer.ts b/src/core/abstractComponentContainer.ts index ed85cc111f..cd01aa2f51 100644 --- a/src/core/abstractComponentContainer.ts +++ b/src/core/abstractComponentContainer.ts @@ -18,16 +18,32 @@ module Plottable { return this; } + /** + * Returns a list of components in the ComponentContainer + * + * @returns{Component[]} the contained Components + */ public getComponents(): Component[] { return this._components; } + /** + * Returns true iff the ComponentContainer is empty. + * + * @returns {boolean} Whether the calling ComponentContainer is empty. + */ public empty() { return this._components.length === 0; } + /** + * Remove all components contained in the ComponentContainer + * + * @returns {AbstractComponentContainer} The calling ComponentContainer + */ public removeAll() { this._components.forEach((c: Component) => c.remove()); + return this; } } } From 00de0818ed640e8454dd980eed6f03ae645d392f Mon Sep 17 00:00:00 2001 From: Daniel Mane Date: Fri, 16 May 2014 16:19:07 -0700 Subject: [PATCH 15/17] Rename AbstractComponentContainer to ComponentContainer --- ...bstractComponentContainer.ts => componentContainer.ts} | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) rename src/core/{abstractComponentContainer.ts => componentContainer.ts} (78%) diff --git a/src/core/abstractComponentContainer.ts b/src/core/componentContainer.ts similarity index 78% rename from src/core/abstractComponentContainer.ts rename to src/core/componentContainer.ts index cd01aa2f51..d75a6d35ec 100644 --- a/src/core/abstractComponentContainer.ts +++ b/src/core/componentContainer.ts @@ -1,7 +1,11 @@ /// module Plottable { - export class AbstractComponentContainer extends Component { + export class ComponentContainer extends Component { + /* + * An abstract ComponentContainer class to encapsulate Table and ComponentGroup's shared functionality. + * It will not do anything if instantiated directly. + */ public _components: Component[] = []; public _removeComponent(c: Component) { var removeIndex = this._components.indexOf(c); @@ -39,7 +43,7 @@ module Plottable { /** * Remove all components contained in the ComponentContainer * - * @returns {AbstractComponentContainer} The calling ComponentContainer + * @returns {ComponentContainer} The calling ComponentContainer */ public removeAll() { this._components.forEach((c: Component) => c.remove()); From 6cb9313eb3a6f2e67f9f264e356f100d8114583d Mon Sep 17 00:00:00 2001 From: Daniel Mane Date: Fri, 16 May 2014 16:30:45 -0700 Subject: [PATCH 16/17] Fix compilation errors introduced by renaming AbstractComponentContainer --- src/core/component.ts | 4 ++-- src/core/componentGroup.ts | 4 ++-- src/core/table.ts | 4 ++-- src/reference.ts | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/core/component.ts b/src/core/component.ts index 0e6820ee3b..48402b829e 100644 --- a/src/core/component.ts +++ b/src/core/component.ts @@ -16,7 +16,7 @@ module Plottable { private rootSVG: D3.Selection; private isTopLevelComponent = false; - private parent: AbstractComponentContainer; + private parent: ComponentContainer; public availableWidth : number; // Width and height of the component. Used to size the hitbox, bounding box, etc public availableHeight: number; @@ -38,7 +38,7 @@ module Plottable { * @param {D3.Selection} element A D3 selection consisting of the element to anchor under. * @returns {Component} The calling component. */ - public _anchor(element: D3.Selection, parent?: AbstractComponentContainer) { + public _anchor(element: D3.Selection, parent?: ComponentContainer) { if (element.node().nodeName === "svg") { // svg node gets the "plottable" CSS class this.rootSVG = element; diff --git a/src/core/componentGroup.ts b/src/core/componentGroup.ts index d553ab1742..b7c164a6eb 100644 --- a/src/core/componentGroup.ts +++ b/src/core/componentGroup.ts @@ -1,7 +1,7 @@ /// module Plottable { - export class ComponentGroup extends AbstractComponentContainer { + export class ComponentGroup extends ComponentContainer { /** * Creates a ComponentGroup. @@ -46,7 +46,7 @@ module Plottable { return this; } - public _anchor(element: D3.Selection, parent?: AbstractComponentContainer): ComponentGroup { + public _anchor(element: D3.Selection, parent?: ComponentContainer): ComponentGroup { super._anchor(element, parent); this._components.forEach((c) => c._anchor(this.content, this)); return this; diff --git a/src/core/table.ts b/src/core/table.ts index 9d6ace255f..3b982f2306 100644 --- a/src/core/table.ts +++ b/src/core/table.ts @@ -8,7 +8,7 @@ module Plottable { wantsWidthArr : boolean[]; wantsHeightArr: boolean[]; } - export class Table extends AbstractComponentContainer { + export class Table extends ComponentContainer { private rowPadding = 0; private colPadding = 0; @@ -72,7 +72,7 @@ module Plottable { /* tslint:enable:no-unreachable */ } - public _anchor(element: D3.Selection, parent?: AbstractComponentContainer) { + public _anchor(element: D3.Selection, parent?: ComponentContainer) { super._anchor(element, parent); // recursively anchor children this.rows.forEach((row: Component[], rowIndex: number) => { diff --git a/src/reference.ts b/src/reference.ts index d2711e2115..21715ad8c7 100644 --- a/src/reference.ts +++ b/src/reference.ts @@ -5,7 +5,7 @@ /// /// /// -/// +/// /// /// /// From 9d976a461b5caecc2a5c3991b8314e9c29586723 Mon Sep 17 00:00:00 2001 From: Daniel Mane Date: Fri, 16 May 2014 16:37:26 -0700 Subject: [PATCH 17/17] Update built files --- plottable.d.ts | 24 +++++++++++++--- plottable.js | 75 +++++++++++++++++++++++++++++++++++--------------- test/tests.js | 50 +++++++++++++++++++++++---------- 3 files changed, 109 insertions(+), 40 deletions(-) diff --git a/plottable.d.ts b/plottable.d.ts index ce1508ce6c..65d6d5a274 100644 --- a/plottable.d.ts +++ b/plottable.d.ts @@ -314,13 +314,29 @@ declare module Plottable { } } declare module Plottable { - class AbstractComponentContainer extends Component { + class ComponentContainer extends Component { + /** + * Returns a list of components in the ComponentContainer + * + * @returns{Component[]} the contained Components + */ public getComponents(): Component[]; - public empty(): void; + /** + * Returns true iff the ComponentContainer is empty. + * + * @returns {boolean} Whether the calling ComponentContainer is empty. + */ + public empty(): boolean; + /** + * Remove all components contained in the ComponentContainer + * + * @returns {ComponentContainer} The calling ComponentContainer + */ + public removeAll(): ComponentContainer; } } declare module Plottable { - class ComponentGroup extends AbstractComponentContainer { + class ComponentGroup extends ComponentContainer { /** * Creates a ComponentGroup. * @@ -334,7 +350,7 @@ declare module Plottable { } } declare module Plottable { - class Table extends AbstractComponentContainer { + class Table extends ComponentContainer { /** * Creates a Table. * diff --git a/plottable.js b/plottable.js index 7fd64294d9..2385c3af5b 100644 --- a/plottable.js +++ b/plottable.js @@ -966,41 +966,63 @@ var Plottable; /// var Plottable; (function (Plottable) { - var AbstractComponentContainer = (function (_super) { - __extends(AbstractComponentContainer, _super); - function AbstractComponentContainer() { + var ComponentContainer = (function (_super) { + __extends(ComponentContainer, _super); + function ComponentContainer() { _super.apply(this, arguments); + /* + * An abstract ComponentContainer class to encapsulate Table and ComponentGroup's shared functionality. + * It will not do anything if instantiated directly. + */ this._components = []; } - AbstractComponentContainer.prototype._removeComponent = function (c) { + ComponentContainer.prototype._removeComponent = function (c) { var removeIndex = this._components.indexOf(c); - if (removeIndex < 0) { - throw new Error("Attempt to remove component, but component not found"); + if (removeIndex >= 0) { + this._components.splice(removeIndex, 1); + this._invalidateLayout(); } - this._components.splice(removeIndex, 1); - this._invalidateLayout(); return this; }; - AbstractComponentContainer.prototype._addComponent = function (c) { + ComponentContainer.prototype._addComponent = function (c) { this._components.push(c); this._invalidateLayout(); return this; }; - AbstractComponentContainer.prototype.getComponents = function () { + /** + * Returns a list of components in the ComponentContainer + * + * @returns{Component[]} the contained Components + */ + ComponentContainer.prototype.getComponents = function () { return this._components; }; - AbstractComponentContainer.prototype.empty = function () { - var _this = this; + /** + * Returns true iff the ComponentContainer is empty. + * + * @returns {boolean} Whether the calling ComponentContainer is empty. + */ + ComponentContainer.prototype.empty = function () { + return this._components.length === 0; + }; + + /** + * Remove all components contained in the ComponentContainer + * + * @returns {ComponentContainer} The calling ComponentContainer + */ + ComponentContainer.prototype.removeAll = function () { this._components.forEach(function (c) { - return _this._removeComponent(c); + return c.remove(); }); + return this; }; - return AbstractComponentContainer; + return ComponentContainer; })(Plottable.Component); - Plottable.AbstractComponentContainer = AbstractComponentContainer; + Plottable.ComponentContainer = ComponentContainer; })(Plottable || (Plottable = {})); /// var Plottable; @@ -1026,17 +1048,26 @@ var Plottable; var requests = this._components.map(function (c) { return c._requestedSpace(offeredWidth, offeredHeight); }); - var desiredWidth = d3.max(requests, function (l) { + var isEmpty = this.empty(); + var desiredWidth = isEmpty ? 0 : d3.max(requests, function (l) { return l.width; }); - var desiredHeight = d3.max(requests, function (l) { + var desiredHeight = isEmpty ? 0 : d3.max(requests, function (l) { return l.height; }); return { width: Math.min(desiredWidth, offeredWidth), height: Math.min(desiredHeight, offeredHeight), - wantsWidth: desiredWidth > offeredWidth, - wantsHeight: desiredHeight > offeredHeight + wantsWidth: isEmpty ? false : requests.map(function (r) { + return r.wantsWidth; + }).some(function (x) { + return x; + }), + wantsHeight: isEmpty ? false : requests.map(function (r) { + return r.wantsHeight; + }).some(function (x) { + return x; + }) }; }; @@ -1097,7 +1128,7 @@ var Plottable; }); }; return ComponentGroup; - })(Plottable.AbstractComponentContainer); + })(Plottable.ComponentContainer); Plottable.ComponentGroup = ComponentGroup; })(Plottable || (Plottable = {})); /// @@ -1487,7 +1518,7 @@ var Plottable; return all(componentGroup.map(groupIsFixed)); }; return Table; - })(Plottable.AbstractComponentContainer); + })(Plottable.ComponentContainer); Plottable.Table = Table; })(Plottable || (Plottable = {})); /// @@ -3693,7 +3724,7 @@ var Plottable; /// /// /// -/// +/// /// /// /// diff --git a/test/tests.js b/test/tests.js index fd4ebf356d..a762225672 100644 --- a/test/tests.js +++ b/test/tests.js @@ -17,6 +17,13 @@ function getSVGParent() { } } +function verifySpaceRequest(sr, w, h, ww, wh, id) { + assert.equal(sr.width, w, "width requested is as expected #" + id); + assert.equal(sr.height, h, "height requested is as expected #" + id); + assert.equal(sr.wantsWidth, ww, "needs more width is as expected #" + id); + assert.equal(sr.wantsHeight, wh, "needs more height is as expected #" + id); +} + function fixComponentSize(c, fixedWidth, fixedHeight) { c._requestedSpace = function (w, h) { return { @@ -608,6 +615,35 @@ describe("ComponentGroups", function () { svg.remove(); }); + describe("ComponentGroup._requestedSpace works as expected", function () { + it("_works for an empty ComponentGroup", function () { + var cg = new Plottable.ComponentGroup(); + var request = cg._requestedSpace(10, 10); + verifySpaceRequest(request, 0, 0, false, false, ""); + }); + + it("works for a ComponentGroup with only proportional-size components", function () { + var cg = new Plottable.ComponentGroup(); + var c1 = new Plottable.Component(); + var c2 = new Plottable.Component(); + cg.merge(c1).merge(c2); + var request = cg._requestedSpace(10, 10); + verifySpaceRequest(request, 0, 0, false, false, ""); + }); + + it("works when there are fixed-size components", function () { + var cg = new Plottable.ComponentGroup(); + var c1 = new Plottable.Component(); + var c2 = new Plottable.Component(); + var c3 = new Plottable.Component(); + cg.merge(c1).merge(c2).merge(c3); + fixComponentSize(c1, null, 10); + fixComponentSize(c2, null, 50); + var request = cg._requestedSpace(10, 10); + verifySpaceRequest(request, 0, 10, false, true, ""); + }); + }); + describe("Component.merge works as expected", function () { var c1 = new Plottable.Component(); var c2 = new Plottable.Component(); @@ -725,13 +761,7 @@ describe("Component behavior", function () { // Remove width/height attributes and style with CSS svg.attr("width", null).attr("height", null); -<<<<<<< HEAD - svg.style("width", "50%"); - svg.style("height", "50%"); - c._anchor(svg, null)._computeLayout(); -======= - c._anchor(svg)._computeLayout(); assert.equal(c.availableWidth, 400, "defaults to width of parent if width is not specified on "); assert.equal(c.availableHeight, 200, "defaults to height of parent if width is not specified on "); assert.equal(c.xOrigin, 0, "xOrigin defaulted to 0"); @@ -739,7 +769,6 @@ describe("Component behavior", function () { svg.style("width", "50%").style("height", "50%"); c._computeLayout(); ->>>>>>> master assert.equal(c.availableWidth, 200, "computeLayout defaulted width to svg width"); assert.equal(c.availableHeight, 100, "computeLayout defaulted height to svg height"); @@ -2751,13 +2780,6 @@ describe("Tables", function () { var c2 = makeFixedSizeComponent(20, 50); var c3 = makeFixedSizeComponent(20, 20); - function verifySpaceRequest(sr, w, h, ww, wh, id) { - assert.equal(sr.width, w, "width requested is as expected #" + id); - assert.equal(sr.height, h, "height requested is as expected #" + id); - assert.equal(sr.wantsWidth, ww, "needs more width is as expected #" + id); - assert.equal(sr.wantsHeight, wh, "needs more height is as expected #" + id); - } - var table = new Plottable.Table([[c0, c1], [c2, c3]]); var spaceRequest = table._requestedSpace(30, 30);