Fix remaining pr comments

elastic · Jan 24, 2020 · be4fb9f · be4fb9f
1 parent 9449adb
commit be4fb9f
Show file tree

Hide file tree

Showing 7 changed files with 37 additions and 31 deletions.
diff --git a/src/legacy/core_plugins/vis_type_vislib/public/vislib/lib/data.js b/src/legacy/core_plugins/vis_type_vislib/public/vislib/lib/data.js
@@ -35,14 +35,16 @@ import { getFormat } from '../../legacy_imports';
  * @param attr {Object|*} Visualization options
  */
 export class Data {
-  constructor(data, uiState, vislibColor) {
+  constructor(data, uiState, createColorLookupFunction) {
     this.uiState = uiState;
-    this.vislibColor = vislibColor;
+    this.createColorLookupFunction = createColorLookupFunction;
     this.data = this.copyDataObj(data);
     this.type = this.getDataType();
     this._cleanVisData();
     this.labels = this._getLabels(this.data);
-    this.color = this.labels ? vislibColor(this.labels, uiState.get('vis.colors')) : undefined;
+    this.color = this.labels
+      ? createColorLookupFunction(this.labels, uiState.get('vis.colors'))
+      : undefined;
     this._normalizeOrdered();
   }
 
@@ -473,7 +475,7 @@ export class Data {
     const defaultColors = this.uiState.get('vis.defaultColors');
     const overwriteColors = this.uiState.get('vis.colors');
     const colors = defaultColors ? _.defaults({}, overwriteColors, defaultColors) : overwriteColors;
-    return this.vislibColor(this.getLabels(), colors);
+    return this.createColorLookupFunction(this.getLabels(), colors);
   }
 
   /**
@@ -483,7 +485,7 @@ export class Data {
    * @returns {Function} Performs lookup on string and returns hex color
    */
   getPieColorFunc() {
-    return this.vislibColor(
+    return this.createColorLookupFunction(
       this.pieNames(this.getVisData()).map(function(d) {
         return d.label;
       }),

diff --git a/src/legacy/core_plugins/vis_type_vislib/public/vislib/lib/vis_config.js b/src/legacy/core_plugins/vis_type_vislib/public/vislib/lib/vis_config.js
@@ -35,8 +35,8 @@ const DEFAULT_VIS_CONFIG = {
 };
 
 export class VisConfig {
-  constructor(visConfigArgs, data, uiState, el, vislibColor) {
-    this.data = new Data(data, uiState, vislibColor);
+  constructor(visConfigArgs, data, uiState, el, createColorLookupFunction) {
+    this.data = new Data(data, uiState, createColorLookupFunction);
 
     const visType = visTypes[visConfigArgs.type];
     const typeDefaults = visType(visConfigArgs, this.data);

diff --git a/src/legacy/core_plugins/vis_type_vislib/public/vislib/vis.js b/src/legacy/core_plugins/vis_type_vislib/public/vislib/vis.js
@@ -55,7 +55,7 @@ export class Vis extends EventEmitter {
       this.data,
       this.uiState,
       this.element,
-      this.deps.charts.colors.vislibColor.bind(this.deps.charts.colors)
+      this.deps.charts.colors.createColorLookupFunction.bind(this.deps.charts.colors)
     );
   }
 

diff --git a/src/plugins/charts/public/services/colors/colors.test.ts b/src/plugins/charts/public/services/colors/colors.test.ts
@@ -46,7 +46,7 @@ describe('Vislib Color Service', () => {
   beforeEach(() => {
     previousConfig = config.get('visualization:colorMapping');
     config.set('visualization:colorMapping', {});
-    color = colors.vislibColor(arr, {});
+    color = colors.createColorLookupFunction(arr, {});
   });
 
   afterEach(() => {
@@ -56,69 +56,69 @@ describe('Vislib Color Service', () => {
   it('should throw error if not initialized', () => {
     const colorsBad = new ColorsService();
 
-    expect(() => colorsBad.vislibColor(arr, {})).toThrowError();
+    expect(() => colorsBad.createColorLookupFunction(arr, {})).toThrowError();
   });
 
   it('should throw an error if input is not an array', () => {
     expect(() => {
-      colors.vislibColor(200);
+      colors.createColorLookupFunction(200);
     }).toThrowError();
 
     expect(() => {
-      colors.vislibColor('help');
+      colors.createColorLookupFunction('help');
     }).toThrowError();
 
     expect(() => {
-      colors.vislibColor(true);
+      colors.createColorLookupFunction(true);
     }).toThrowError();
 
     expect(() => {
-      colors.vislibColor();
+      colors.createColorLookupFunction();
     }).toThrowError();
 
     expect(() => {
-      colors.vislibColor(nullValue);
+      colors.createColorLookupFunction(nullValue);
     }).toThrowError();
 
     expect(() => {
-      colors.vislibColor(emptyObject);
+      colors.createColorLookupFunction(emptyObject);
     }).toThrowError();
   });
 
   describe('when array is not composed of numbers, strings, or undefined values', () => {
     it('should throw an error', () => {
       expect(() => {
-        colors.vislibColor(arrayOfObjects);
+        colors.createColorLookupFunction(arrayOfObjects);
       }).toThrowError();
 
       expect(() => {
-        colors.vislibColor(arrayOfBooleans);
+        colors.createColorLookupFunction(arrayOfBooleans);
       }).toThrowError();
 
       expect(() => {
-        colors.vislibColor(arrayOfNullValues);
+        colors.createColorLookupFunction(arrayOfNullValues);
       }).toThrowError();
     });
   });
 
   describe('when input is an array of strings, numbers, or undefined values', () => {
     it('should not throw an error', () => {
       expect(() => {
-        colors.vislibColor(arr);
+        colors.createColorLookupFunction(arr);
       }).not.toThrowError();
 
       expect(() => {
-        colors.vislibColor(arrayOfNumbers);
+        colors.createColorLookupFunction(arrayOfNumbers);
       }).not.toThrowError();
 
       expect(() => {
-        colors.vislibColor(arrayOfUndefinedValues);
+        colors.createColorLookupFunction(arrayOfUndefinedValues);
       }).not.toThrowError();
     });
   });
 
   it('should be a function', () => {
-    expect(typeof colors.vislibColor).toBe('function');
+    expect(typeof colors.createColorLookupFunction).toBe('function');
   });
 
   it('should return a function', () => {
@@ -134,7 +134,7 @@ describe('Vislib Color Service', () => {
   });
 
   it('should return the value from the specified color mapping overrides', () => {
-    const colorFn = colors.vislibColor(arr, { good: 'red' });
+    const colorFn = colors.createColorLookupFunction(arr, { good: 'red' });
     expect(colorFn('good')).toBe('red');
   });
 });
diff --git a/src/plugins/charts/public/services/colors/colors.ts b/src/plugins/charts/public/services/colors/colors.ts
@@ -47,23 +47,27 @@ export class ColorsService {
     this._mappedColors = new MappedColors(uiSettings);
   }
 
-  vislibColor(arrayOfStringsOrNumbers?: any, colorMapping = {}) {
+  createColorLookupFunction(
+    arrayOfStringsOrNumbers?: any,
+    colorMapping: Partial<Record<string, string>> = {}
+  ) {
     if (!Array.isArray(arrayOfStringsOrNumbers)) {
       throw new Error(
-        `vislibColor expects an array but recived: ${typeof arrayOfStringsOrNumbers}`
+        `createColorLookupFunction expects an array but recived: ${typeof arrayOfStringsOrNumbers}`
       );
     }
 
     arrayOfStringsOrNumbers.forEach(function(val) {
       if (!_.isString(val) && !_.isNumber(val) && !_.isUndefined(val)) {
-        throw new TypeError('ColorUtil expects an array of strings, numbers, or undefined values');
+        throw new TypeError(
+          'createColorLookupFunction expects an array of strings, numbers, or undefined values'
+        );
       }
     });
 
     this.mappedColors.mapKeys(arrayOfStringsOrNumbers);
 
     return (value: string) => {
-      // @ts-ignore
       return colorMapping[value] || this.mappedColors.get(value);
     };
   }

diff --git a/src/plugins/charts/public/services/colors/mock.ts b/src/plugins/charts/public/services/colors/mock.ts
@@ -24,5 +24,5 @@ const colors = new ColorsService();
 colors.init(coreMock.createSetup().uiSettings);
 
 export const colorsServiceMock: ColorsService = {
-  vislibColor: jest.fn(colors.vislibColor),
+  createColorLookupFunction: jest.fn(colors.createColorLookupFunction),
 } as any;
diff --git a/x-pack/plugins/watcher/__jest__/client_integration/helpers/app_context.mock.tsx b/x-pack/plugins/watcher/__jest__/client_integration/helpers/app_context.mock.tsx
@@ -38,10 +38,10 @@ export const mockContextValue = {
   toasts: notificationServiceMock.createSetupContract().toasts,
   theme: {
     useChartsTheme: jest.fn(),
-  },
+  } as any,
   // For our test harness, we don't use this mocked out http service
   http: httpServiceMock.createSetupContract(),
-} as any;
+};
 
 export const withAppContext = (Component: ComponentType<any>) => (props: any) => {
   return (