Skip to content

Commit be4fb9f

Browse files
nickofthymenickofthyme
committed
Fix remaining pr comments
1 parent 9449adb commit be4fb9f

7 files changed

Lines changed: 37 additions & 31 deletions

File tree

‎src/legacy/core_plugins/vis_type_vislib/public/vislib/lib/data.js‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,16 @@ import { getFormat } from '../../legacy_imports';
3535
* @param attr {Object|*} Visualization options
3636
*/
3737
export class Data {
38-
constructor(data, uiState, vislibColor) {
38+
constructor(data, uiState, createColorLookupFunction) {
3939
this.uiState = uiState;
40-
this.vislibColor = vislibColor;
40+
this.createColorLookupFunction = createColorLookupFunction;
4141
this.data = this.copyDataObj(data);
4242
this.type = this.getDataType();
4343
this._cleanVisData();
4444
this.labels = this._getLabels(this.data);
45-
this.color = this.labels ? vislibColor(this.labels, uiState.get('vis.colors')) : undefined;
45+
this.color = this.labels
46+
? createColorLookupFunction(this.labels, uiState.get('vis.colors'))
47+
: undefined;
4648
this._normalizeOrdered();
4749
}
4850

@@ -473,7 +475,7 @@ export class Data {
473475
const defaultColors = this.uiState.get('vis.defaultColors');
474476
const overwriteColors = this.uiState.get('vis.colors');
475477
const colors = defaultColors ? _.defaults({}, overwriteColors, defaultColors) : overwriteColors;
476-
return this.vislibColor(this.getLabels(), colors);
478+
return this.createColorLookupFunction(this.getLabels(), colors);
477479
}
478480

479481
/**
@@ -483,7 +485,7 @@ export class Data {
483485
* @returns {Function} Performs lookup on string and returns hex color
484486
*/
485487
getPieColorFunc() {
486-
return this.vislibColor(
488+
return this.createColorLookupFunction(
487489
this.pieNames(this.getVisData()).map(function(d) {
488490
return d.label;
489491
}),

‎src/legacy/core_plugins/vis_type_vislib/public/vislib/lib/vis_config.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@ const DEFAULT_VIS_CONFIG = {
3535
};
3636

3737
export class VisConfig {
38-
constructor(visConfigArgs, data, uiState, el, vislibColor) {
39-
this.data = new Data(data, uiState, vislibColor);
38+
constructor(visConfigArgs, data, uiState, el, createColorLookupFunction) {
39+
this.data = new Data(data, uiState, createColorLookupFunction);
4040

4141
const visType = visTypes[visConfigArgs.type];
4242
const typeDefaults = visType(visConfigArgs, this.data);

‎src/legacy/core_plugins/vis_type_vislib/public/vislib/vis.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ export class Vis extends EventEmitter {
5555
this.data,
5656
this.uiState,
5757
this.element,
58-
this.deps.charts.colors.vislibColor.bind(this.deps.charts.colors)
58+
this.deps.charts.colors.createColorLookupFunction.bind(this.deps.charts.colors)
5959
);
6060
}
6161

‎src/plugins/charts/public/services/colors/colors.test.ts‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ describe('Vislib Color Service', () => {
4646
beforeEach(() => {
4747
previousConfig = config.get('visualization:colorMapping');
4848
config.set('visualization:colorMapping', {});
49-
color = colors.vislibColor(arr, {});
49+
color = colors.createColorLookupFunction(arr, {});
5050
});
5151

5252
afterEach(() => {
@@ -56,69 +56,69 @@ describe('Vislib Color Service', () => {
5656
it('should throw error if not initialized', () => {
5757
const colorsBad = new ColorsService();
5858

59-
expect(() => colorsBad.vislibColor(arr, {})).toThrowError();
59+
expect(() => colorsBad.createColorLookupFunction(arr, {})).toThrowError();
6060
});
6161

6262
it('should throw an error if input is not an array', () => {
6363
expect(() => {
64-
colors.vislibColor(200);
64+
colors.createColorLookupFunction(200);
6565
}).toThrowError();
6666

6767
expect(() => {
68-
colors.vislibColor('help');
68+
colors.createColorLookupFunction('help');
6969
}).toThrowError();
7070

7171
expect(() => {
72-
colors.vislibColor(true);
72+
colors.createColorLookupFunction(true);
7373
}).toThrowError();
7474

7575
expect(() => {
76-
colors.vislibColor();
76+
colors.createColorLookupFunction();
7777
}).toThrowError();
7878

7979
expect(() => {
80-
colors.vislibColor(nullValue);
80+
colors.createColorLookupFunction(nullValue);
8181
}).toThrowError();
8282

8383
expect(() => {
84-
colors.vislibColor(emptyObject);
84+
colors.createColorLookupFunction(emptyObject);
8585
}).toThrowError();
8686
});
8787

8888
describe('when array is not composed of numbers, strings, or undefined values', () => {
8989
it('should throw an error', () => {
9090
expect(() => {
91-
colors.vislibColor(arrayOfObjects);
91+
colors.createColorLookupFunction(arrayOfObjects);
9292
}).toThrowError();
9393

9494
expect(() => {
95-
colors.vislibColor(arrayOfBooleans);
95+
colors.createColorLookupFunction(arrayOfBooleans);
9696
}).toThrowError();
9797

9898
expect(() => {
99-
colors.vislibColor(arrayOfNullValues);
99+
colors.createColorLookupFunction(arrayOfNullValues);
100100
}).toThrowError();
101101
});
102102
});
103103

104104
describe('when input is an array of strings, numbers, or undefined values', () => {
105105
it('should not throw an error', () => {
106106
expect(() => {
107-
colors.vislibColor(arr);
107+
colors.createColorLookupFunction(arr);
108108
}).not.toThrowError();
109109

110110
expect(() => {
111-
colors.vislibColor(arrayOfNumbers);
111+
colors.createColorLookupFunction(arrayOfNumbers);
112112
}).not.toThrowError();
113113

114114
expect(() => {
115-
colors.vislibColor(arrayOfUndefinedValues);
115+
colors.createColorLookupFunction(arrayOfUndefinedValues);
116116
}).not.toThrowError();
117117
});
118118
});
119119

120120
it('should be a function', () => {
121-
expect(typeof colors.vislibColor).toBe('function');
121+
expect(typeof colors.createColorLookupFunction).toBe('function');
122122
});
123123

124124
it('should return a function', () => {
@@ -134,7 +134,7 @@ describe('Vislib Color Service', () => {
134134
});
135135

136136
it('should return the value from the specified color mapping overrides', () => {
137-
const colorFn = colors.vislibColor(arr, { good: 'red' });
137+
const colorFn = colors.createColorLookupFunction(arr, { good: 'red' });
138138
expect(colorFn('good')).toBe('red');
139139
});
140140
});

‎src/plugins/charts/public/services/colors/colors.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,23 +47,27 @@ export class ColorsService {
4747
this._mappedColors = new MappedColors(uiSettings);
4848
}
4949

50-
vislibColor(arrayOfStringsOrNumbers?: any, colorMapping = {}) {
50+
createColorLookupFunction(
51+
arrayOfStringsOrNumbers?: any,
52+
colorMapping: Partial<Record<string, string>> = {}
53+
) {
5154
if (!Array.isArray(arrayOfStringsOrNumbers)) {
5255
throw new Error(
53-
`vislibColor expects an array but recived: ${typeof arrayOfStringsOrNumbers}`
56+
`createColorLookupFunction expects an array but recived: ${typeof arrayOfStringsOrNumbers}`
5457
);
5558
}
5659

5760
arrayOfStringsOrNumbers.forEach(function(val) {
5861
if (!_.isString(val) && !_.isNumber(val) && !_.isUndefined(val)) {
59-
throw new TypeError('ColorUtil expects an array of strings, numbers, or undefined values');
62+
throw new TypeError(
63+
'createColorLookupFunction expects an array of strings, numbers, or undefined values'
64+
);
6065
}
6166
});
6267

6368
this.mappedColors.mapKeys(arrayOfStringsOrNumbers);
6469

6570
return (value: string) => {
66-
// @ts-ignore
6771
return colorMapping[value] || this.mappedColors.get(value);
6872
};
6973
}

‎src/plugins/charts/public/services/colors/mock.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,5 @@ const colors = new ColorsService();
2424
colors.init(coreMock.createSetup().uiSettings);
2525

2626
export const colorsServiceMock: ColorsService = {
27-
vislibColor: jest.fn(colors.vislibColor),
27+
createColorLookupFunction: jest.fn(colors.createColorLookupFunction),
2828
} as any;

‎x-pack/plugins/watcher/__jest__/client_integration/helpers/app_context.mock.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,10 @@ export const mockContextValue = {
3838
toasts: notificationServiceMock.createSetupContract().toasts,
3939
theme: {
4040
useChartsTheme: jest.fn(),
41-
},
41+
} as any,
4242
// For our test harness, we don't use this mocked out http service
4343
http: httpServiceMock.createSetupContract(),
44-
} as any;
44+
};
4545

4646
export const withAppContext = (Component: ComponentType<any>) => (props: any) => {
4747
return (

0 commit comments

Comments
 (0)