Merge pull request #394 from Esri/f/legend

F/legend
Esri · Feb 26, 2018 · ec8ce28 · ec8ce28
2 parents 1b44f26 + f242625
commit ec8ce28
Show file tree

Hide file tree

Showing 5 changed files with 79 additions and 5 deletions.
diff --git 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
+- legend.position determines the location (top, bottom, right, left) of a chart legend
+### Changed
+- legend.enable is now legend.visible
+
 ## 1.0.0-beta.1
 ### Added
 - cedar definition legend property hides/shows chart legend

diff --git a/packages/cedar-amcharts/src/render/render.ts b/packages/cedar-amcharts/src/render/render.ts
@@ -44,6 +44,10 @@ export function fillInSpec(spec: any, definition: any) {
 
   // Add a legend in case it's not on the spec
   if (!spec.legend) { spec.legend = {} }
+  // TODO This is needed as 'legend.enable' has been renamed 'legend.visible'. We are only introducing
+  // breaking changes on major releases.
+  // Remove the line below on next breaking change
+  if (definition.legend && definition.legend.hasOwnProperty('enable')) { definition.legend.visible = definition.legend.enable }
 
   // adjust legend and axis labels for single series charts
   if (definition.series.length === 1 && (definition.type !== 'pie' && definition.type !== 'radar')) {
@@ -75,8 +79,16 @@ export function fillInSpec(spec: any, definition: any) {
     }
   }
 
+  // Handle Legend in case.
   if (definition.legend) {
-    spec.legend.enabled = definition.legend.enable
+    const legend = definition.legend
+    const supportedLegendPositions: string[] = ['top', 'bottom', 'left', 'right']
+    if (legend.hasOwnProperty('visible')) {
+      spec.legend.enabled = legend.visible
+    }
+    if (legend.position && supportedLegendPositions.indexOf(legend.position) > -1) {
+      spec.legend.position = legend.position
+    }
   }
 
   // Iterate over datasets

diff --git a/packages/cedar-amcharts/test/render/render.spec.ts b/packages/cedar-amcharts/test/render/render.spec.ts
@@ -142,7 +142,7 @@ describe('when filling in a stacked bar spec', () => {
         }
       ],
       "legend": {
-        "enable": false
+        "visible": false
       },
     }
     /* tslint:enable */
@@ -163,6 +163,54 @@ describe('when filling in a stacked bar spec', () => {
   })
 })
 
+describe('Legend is properly effected by legend props', () => {
+  let result
+  beforeAll(() => {
+    const spec = (bar as any)
+    /* tslint:disable */
+    const definition = {
+      "type": "bar",
+      "datasets": [
+        {
+          "url": "https://services.arcgis.com/uDTUpUPbk8X8mXwl/arcgis/rest/services/Public_Schools_in_Onondaga_County/FeatureServer/0",
+          "name": "Jordan",
+          "query": {
+            "where": "City='Jordan'",
+            "orderByFields": "Number_of_SUM DESC",
+            "groupByFieldsForStatistics": "Type",
+            "outStatistics": [{
+              "statisticType": "sum",
+              "onStatisticField": "Number_of",
+              "outStatisticFieldName": "Number_of_SUM"
+            }]
+          },
+          "join": "Type"
+        }
+      ],
+      "series": [
+        {
+          "category": {"field": "Type", "label": "Type"},
+          "value": { "field": "Number_of_SUM", "label": "Jordan Students"},
+          "source": "Jordan",
+          "stack": true
+        }
+      ],
+      "legend": {
+        "visible": true,
+        "position": "right"
+      },
+    }
+    /* tslint:enable */
+    result = render.fillInSpec(spec, definition)
+  })
+  test('Legend should be true if visible: true passed in', () => {
+    expect(result.legend.enabled).toBe(true)
+  })
+  test('Legend position should be right when position: right is passedin', () => {
+    expect(result.legend.position).toEqual('right')
+  })
+})
+
 describe('when filling in a pie spec', () => {
   let result
   beforeAll(() => {
@@ -233,7 +281,7 @@ describe('when passed an interim (v0.9.x) bar definition', () => {
   test('should use series category field ', () => {
     expect(result.categoryField).toEqual('Type')
   })
-  test('should disable legend', () => {
+  test('single series chart should be disabled by default', () => {
     expect(result.legend.enabled).toEqual(false)
   })
   test('should set x axis', () => {
@@ -272,7 +320,7 @@ describe('when passed an interim (v0.9.x) scatter definition', () => {
   test('should use series category field ', () => {
     expect(result.categoryField).toEqual('Number_of')
   })
-  test('should disable legend', () => {
+  test('single series chart should have legend disabled by default', () => {
     expect(result.legend.enabled).toEqual(false)
   })
   test('should set x axis', () => {

diff --git a/packages/cedar/src/Chart.ts b/packages/cedar/src/Chart.ts
@@ -58,7 +58,8 @@ export interface ISeries {
 }
 
 export interface ILegend {
-  enable: boolean
+  visible?: boolean,
+  position?: 'top' | 'bottom' | 'left' | 'right'
 }
 
 export interface IDefinition {

diff --git a/packages/cedar/test/Chart.spec.ts b/packages/cedar/test/Chart.spec.ts
@@ -33,6 +33,10 @@ const barDefinition = {
     "categoryAxis": {
       "labelRotation": -45
     }
+  },
+  "legend": {
+    "visible": true,
+    "position": "right"
   }
 }
 /* tslint:enable */
@@ -66,6 +70,9 @@ describe('new Chart w/o definition', () => {
   test('overrides should set the definition.overrides', () => {
     expect(chart.overrides(barDefinition.overrides).overrides()).toEqual(barDefinition.overrides)
   })
+  test('legend should set the definition.legend', () => {
+    expect(chart.legend(barDefinition.legend).legend()).toEqual(barDefinition.legend)
+  })
 })
 
 describe('when updating data', () => {