From 9022a66382478f6eff163115e2e401965cac076a Mon Sep 17 00:00:00 2001 From: Ben Stoltz Date: Fri, 13 Apr 2018 10:09:13 -0400 Subject: [PATCH 1/4] styles for margins and pie implemented --- packages/cedar-amcharts/src/render/render.ts | 23 ++++++++++++++- .../cedar-amcharts/test/render/render.spec.ts | 2 +- packages/cedar/src/Chart.ts | 29 ++++++++++++++++--- 3 files changed, 48 insertions(+), 6 deletions(-) diff --git a/packages/cedar-amcharts/src/render/render.ts b/packages/cedar-amcharts/src/render/render.ts index c50d1857..27b9fec2 100644 --- a/packages/cedar-amcharts/src/render/render.ts +++ b/packages/cedar-amcharts/src/render/render.ts @@ -51,7 +51,7 @@ function getPieBalloonText(definition: any) { return `${categoryLabel}[[title]] [[percents]]% (${valueLabel}[[value]])` } -export function fillInSpec(spec: any, definition: any) { +export function fillInSpec(spec: any, definition: any) { // TODO: Figure out how to split this function up // Grab the graphSpec from the spec const graphSpec = spec.graphs.pop() const isJoined = definition.datasets.length > 1 @@ -113,6 +113,27 @@ export function fillInSpec(spec: any, definition: any) { } } + // Handle styles... + if (definition.style) { + // snag out style + const style = definition.style + // handle margins + if (style.hasOwnProperty('autoMargins')) { spec.autoMargins = style.autoMargins } + if (style.hasOwnProperty('marginTop')) { spec.marginTop = style.marginTop } + if (style.hasOwnProperty('marginBottom')) { spec.marginBottom = style.marginBottom } + if (style.hasOwnProperty('marginLeft')) { spec.marginLeft = style.marginLeft } + if (style.hasOwnProperty('marginRight')) { spec.marginRight = style.marginRight } + // If there is a pie property + if (style.pie) { + const pie = style.pie + // A range from 0 - n where n is the inner radius of the pie chart. Anything above a 0 + // turns the chart into a donut chart. Can be a number for pixels or a percent. + if (pie.hasOwnProperty('innerRadius')) { spec.innerRadius = pie.innerRadius } + // How far a pie chart slice will pull out when selected. Can be a number for pixels or a percent + if (pie.hasOwnProperty('pullOutRadius')) { spec.pullOutRadius = pie.pullOutRadius } + } + } + // Iterate over datasets definition.datasets.forEach((dataset, d) => { const datasetName = dataset.name diff --git a/packages/cedar-amcharts/test/render/render.spec.ts b/packages/cedar-amcharts/test/render/render.spec.ts index d2bc039f..3913e8a9 100644 --- a/packages/cedar-amcharts/test/render/render.spec.ts +++ b/packages/cedar-amcharts/test/render/render.spec.ts @@ -1,7 +1,7 @@ /* globals global:false */ /* globals AmCharts:false */ import { } from 'jest' -import { fetchSpec, fillInSpec, getPieBalloonText, renderChart } from '../../src/render/render' +import { fetchSpec, fillInSpec, renderChart } from '../../src/render/render' import bar from '../../src/specs/bar' import scatter from '../../src/specs/scatter' import timeline from '../../src/specs/timeline' diff --git a/packages/cedar/src/Chart.ts b/packages/cedar/src/Chart.ts index 11fd7d8d..395566b8 100644 --- a/packages/cedar/src/Chart.ts +++ b/packages/cedar/src/Chart.ts @@ -8,17 +8,32 @@ function clone(json) { // TODO: where should these interfaces live? export interface ILegend { - visible?: boolean, + visible?: boolean position?: 'top' | 'bottom' | 'left' | 'right' } +export interface IPie { + innerRadius?: number | string + pullOutRadius?: number | string +} + +export interface IStyle { + pie?: IPie + autoMargins?: boolean + marginTop?: number + marginBottom?: number + marginLeft?: number + marginRight?: number +} + export interface IDefinition { - datasets?: IDataset[], - series?: ISeries[], + datasets?: IDataset[] + series?: ISeries[] type?: string specification?: {} - overrides?: {}, + overrides?: {} legend?: ILegend + style?: IStyle } export default class Chart { @@ -86,6 +101,12 @@ export default class Chart { return this._definitionAccessor('legend', newLegend) } + public style(newStyle: IStyle): Chart + public style(): IStyle + public style(newStyle?: any): any { + return this._definitionAccessor('style', newStyle) + } + // data is read only public data() { return this._data From 9ae2e558e3d942c43d340cea63c1cc45a44ce9be Mon Sep 17 00:00:00 2001 From: Ben Stoltz Date: Fri, 13 Apr 2018 12:23:46 -0400 Subject: [PATCH 2/4] tests and renaming --- packages/cedar-amcharts/src/render/render.ts | 14 +++--- .../cedar-amcharts/test/render/render.spec.ts | 44 +++++++++++++++++++ packages/cedar/src/Chart.ts | 15 ++++--- packages/cedar/test/Chart.spec.ts | 3 ++ packages/cedar/test/data/definitions.ts | 12 +++++ 5 files changed, 76 insertions(+), 12 deletions(-) diff --git a/packages/cedar-amcharts/src/render/render.ts b/packages/cedar-amcharts/src/render/render.ts index 27b9fec2..73d73ae7 100644 --- a/packages/cedar-amcharts/src/render/render.ts +++ b/packages/cedar-amcharts/src/render/render.ts @@ -118,11 +118,13 @@ export function fillInSpec(spec: any, definition: any) { // TODO: Figure out how // snag out style const style = definition.style // handle margins - if (style.hasOwnProperty('autoMargins')) { spec.autoMargins = style.autoMargins } - if (style.hasOwnProperty('marginTop')) { spec.marginTop = style.marginTop } - if (style.hasOwnProperty('marginBottom')) { spec.marginBottom = style.marginBottom } - if (style.hasOwnProperty('marginLeft')) { spec.marginLeft = style.marginLeft } - if (style.hasOwnProperty('marginRight')) { spec.marginRight = style.marginRight } + if (style.padding) { + const padding = style.padding + if (padding.hasOwnProperty('top')) { spec.marginTop = padding.top } + if (padding.hasOwnProperty('bottom')) { spec.marginBottom = padding.bottom } + if (padding.hasOwnProperty('left')) { spec.marginLeft = padding.left } + if (padding.hasOwnProperty('right')) { spec.marginRight = padding.right } + } // If there is a pie property if (style.pie) { const pie = style.pie @@ -130,7 +132,7 @@ export function fillInSpec(spec: any, definition: any) { // TODO: Figure out how // turns the chart into a donut chart. Can be a number for pixels or a percent. if (pie.hasOwnProperty('innerRadius')) { spec.innerRadius = pie.innerRadius } // How far a pie chart slice will pull out when selected. Can be a number for pixels or a percent - if (pie.hasOwnProperty('pullOutRadius')) { spec.pullOutRadius = pie.pullOutRadius } + if (pie.hasOwnProperty('expand')) { spec.pullOutRadius = pie.expand } } } diff --git a/packages/cedar-amcharts/test/render/render.spec.ts b/packages/cedar-amcharts/test/render/render.spec.ts index 3913e8a9..303bde22 100644 --- a/packages/cedar-amcharts/test/render/render.spec.ts +++ b/packages/cedar-amcharts/test/render/render.spec.ts @@ -135,6 +135,50 @@ describe('when overriding legend defaults', () => { }) }) +describe('When filling in style', () => { + let result + let definition + beforeAll(() => { + definition = definitions.pie + definition.style = { + pie: { + innerRadius: '50%', + expand: 0 + }, + padding: { + top: 10, + bottom: 10, + right: 10, + left: 10 + } + } + const spec = fetchSpec(definition.type) + result = fillInSpec(spec, definition) + }) + test('spec.innerRadius should be 50% if definition.style.pie.innerRadius: 50% is passed in', () => { + expect(result.innerRadius).toEqual('50%') + }) + test('spec.pullOutRadius should be 0 if definition.style.pie.expand: 0 is passed in', () => { + expect(result.pullOutRadius).toEqual(0) + }) + test('spec.marginTop should be 10 if definition.style.padding.top: 10 is passed in', () => { + expect(result.marginTop).toEqual(10) + }) + test('spec.marginBottom should be 10 if definition.style.padding.bottom: 10 is passed in', () => { + expect(result.marginBottom).toEqual(10) + }) + test('spec.marginLeft should be 10 if definition.style.padding.left: 10 is passed in', () => { + expect(result.marginLeft).toEqual(10) + }) + test('spec.marginRight should be 10 if definition.style.padding.right: 10 is passed in', () => { + expect(result.marginRight).toEqual(10) + }) + afterAll(() => { + // clean up + delete definition.style + }) +}) + describe('when filling in a line spec', () => { let result beforeAll(() => { diff --git a/packages/cedar/src/Chart.ts b/packages/cedar/src/Chart.ts index 395566b8..e46f7782 100644 --- a/packages/cedar/src/Chart.ts +++ b/packages/cedar/src/Chart.ts @@ -14,16 +14,19 @@ export interface ILegend { export interface IPie { innerRadius?: number | string - pullOutRadius?: number | string + expand?: number | string +} + +export interface IPadding { + top?: number + bottom?: number + left?: number + right?: number } export interface IStyle { pie?: IPie - autoMargins?: boolean - marginTop?: number - marginBottom?: number - marginLeft?: number - marginRight?: number + padding?: IPadding } export interface IDefinition { diff --git a/packages/cedar/test/Chart.spec.ts b/packages/cedar/test/Chart.spec.ts index cd514512..0f2781c8 100644 --- a/packages/cedar/test/Chart.spec.ts +++ b/packages/cedar/test/Chart.spec.ts @@ -58,6 +58,9 @@ describe('new Chart w/o definition', () => { test('legend should set the definition.legend', () => { expect(chart.legend(barDefinition.legend).legend()).toEqual(barDefinition.legend) }) + test('style should set the definition.style', () => { + expect(chart.style(barDefinition.style).style()).toEqual(barDefinition.style) + }) }) describe('when updating data', () => { diff --git a/packages/cedar/test/data/definitions.ts b/packages/cedar/test/data/definitions.ts index 8bba4fe6..6a20bec4 100644 --- a/packages/cedar/test/data/definitions.ts +++ b/packages/cedar/test/data/definitions.ts @@ -33,6 +33,18 @@ export const bar = { "legend": { "visible": true, "position": "right" + }, + "style": { + "pie": { + "expand": 0, + "innerRadius": "50%" + }, + "padding": { + "top": 10, + "bottom": 10, + "left": 10, + "right": 10 + } } } From ecc7ff6928f1dff36f2f196fe7ed4566942ccac8 Mon Sep 17 00:00:00 2001 From: Ben Stoltz Date: Fri, 13 Apr 2018 12:54:36 -0400 Subject: [PATCH 3/4] coverage comments because hasOwnProperty --- packages/cedar-amcharts/src/render/render.ts | 5 +++++ packages/cedar-amcharts/test/render/render.spec.ts | 3 +++ 2 files changed, 8 insertions(+) diff --git a/packages/cedar-amcharts/src/render/render.ts b/packages/cedar-amcharts/src/render/render.ts index 73d73ae7..79465fe7 100644 --- a/packages/cedar-amcharts/src/render/render.ts +++ b/packages/cedar-amcharts/src/render/render.ts @@ -114,18 +114,23 @@ export function fillInSpec(spec: any, definition: any) { // TODO: Figure out how } // Handle styles... + /* istanbul ignore if */ if (definition.style) { // snag out style const style = definition.style // handle margins + /* istanbul ignore if */ if (style.padding) { const padding = style.padding + // Assume we need to set auto margins false + spec.autoMargins = false if (padding.hasOwnProperty('top')) { spec.marginTop = padding.top } if (padding.hasOwnProperty('bottom')) { spec.marginBottom = padding.bottom } if (padding.hasOwnProperty('left')) { spec.marginLeft = padding.left } if (padding.hasOwnProperty('right')) { spec.marginRight = padding.right } } // If there is a pie property + /* istanbul ignore if */ if (style.pie) { const pie = style.pie // A range from 0 - n where n is the inner radius of the pie chart. Anything above a 0 diff --git a/packages/cedar-amcharts/test/render/render.spec.ts b/packages/cedar-amcharts/test/render/render.spec.ts index 303bde22..863a0274 100644 --- a/packages/cedar-amcharts/test/render/render.spec.ts +++ b/packages/cedar-amcharts/test/render/render.spec.ts @@ -161,6 +161,9 @@ describe('When filling in style', () => { test('spec.pullOutRadius should be 0 if definition.style.pie.expand: 0 is passed in', () => { expect(result.pullOutRadius).toEqual(0) }) + test('spec.autoMargins should be false if definition.style.padding exists', () => { + expect(result.autoMargins).toBeFalsy() + }) test('spec.marginTop should be 10 if definition.style.padding.top: 10 is passed in', () => { expect(result.marginTop).toEqual(10) }) From 894ceb2b36cad997e0da7b224e80abd2cca59184 Mon Sep 17 00:00:00 2001 From: Ben Stoltz Date: Fri, 13 Apr 2018 13:02:16 -0400 Subject: [PATCH 4/4] changelogs --- packages/cedar-amcharts/CHANGELOG.md | 6 ++++++ packages/cedar/CHANGELOG.md | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/packages/cedar-amcharts/CHANGELOG.md b/packages/cedar-amcharts/CHANGELOG.md index 7c4c7c7c..d8d8e298 100644 --- a/packages/cedar-amcharts/CHANGELOG.md +++ b/packages/cedar-amcharts/CHANGELOG.md @@ -3,6 +3,12 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). +## Unreleased +### Added +- One can now add a `style` property which contains `padding` and `pie` properties to `definition` +### changed +- popup text for pie charts has a new format + ## 1.0.0-beta.4 ### Changed - treat arcgis libraries as external diff --git a/packages/cedar/CHANGELOG.md b/packages/cedar/CHANGELOG.md index 28a86081..50c4a30e 100644 --- a/packages/cedar/CHANGELOG.md +++ b/packages/cedar/CHANGELOG.md @@ -3,6 +3,10 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). +## Unreleased +### Added +- One can now add a `style` property which contains `padding` and `pie` properties to `definition` + ## [1.0.0-beta.4] ### Changed - treat arcgis libraries as external