Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bad color mapping with multiple split series #80760

Closed
nickofthyme opened this issue Oct 15, 2020 · 4 comments · Fixed by #80801
Closed

Bad color mapping with multiple split series #80760

nickofthyme opened this issue Oct 15, 2020 · 4 comments · Fixed by #80801
Assignees
Labels
blocker bug Fixes for quality problems that affect the customer experience Feature:Vislib Vislib chart implementation regression Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.10.0

Comments

@nickofthyme
Copy link
Contributor

Kibana version:
master (maybe other versions, not sure how far back it goes.)

Describe the bug:
When using any vislib chart type (pie, area, bar, line, etc.) and specifying more than one split series aggregation. The color mapping uses a black value instead of the correct color mapping.

Steps to reproduce:

  1. Open and load kibana with a sample dataset
  2. Open visualize and create a new bar visualization
  3. Select a x axis terms agg and single split series agg
  4. Notice the correct colors are shown
  5. Add another simple split series agg
  6. Notice some (usually the last few) or all the series colors are now black.

Sometimes the issue is corrected after refresh and sometimes not

Expected behavior:
Should show correct colors.

Screenshots (if relevant):

Bar Chart

Screen Recording 2020-10-15 at 04 08 PM

Pie Chart

Screen Recording 2020-10-15 at 04 02 PM

Any additional context:
Vislib gets it's color mappings from the createColorLookupFunction in the charts plugin. Likely a change in this plugin is the cause and not in vislib, nut just a hunch.

createColorLookupFunction(
arrayOfStringsOrNumbers?: any,
colorMapping: Partial<Record<string, string>> = {}
) {
if (!Array.isArray(arrayOfStringsOrNumbers)) {
throw new Error(
`createColorLookupFunction expects an array but recived: ${typeof arrayOfStringsOrNumbers}`
);
}
arrayOfStringsOrNumbers.forEach(function (val) {
if (!_.isString(val) && !_.isNumber(val) && !_.isUndefined(val)) {
throw new TypeError(
'createColorLookupFunction expects an array of strings, numbers, or undefined values'
);
}
});
this.mappedColors.mapKeys(arrayOfStringsOrNumbers);
return (value: string) => {
return colorMapping[value] || this.mappedColors.get(value);
};
}

This does not impact the custom set colors coming from uiState as shown in the screenshot.

@nickofthyme nickofthyme added bug Fixes for quality problems that affect the customer experience Feature:Vislib Vislib chart implementation Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Oct 15, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@markov00
Copy link
Member

Just checked on 7.9 and it doesn't seem to be affected

@flash1293
Copy link
Contributor

flash1293 commented Oct 16, 2020

I have nothing to back this, but one thing to investigate is this change: https://github.com/elastic/kibana/pull/78922/files#diff-deb7e80be32c71c952a5c13e1c7d798414ec8daa4aa51b063f083d5ad0a26908L20 (switching from d3 to the color package for color math for bundle size reasons). It would fit @markov00 s observation about affected versions.

@flash1293
Copy link
Contributor

Not sure whether related, but I noticed the same behavior in TSVB as well:
Screenshot 2020-10-16 at 09 39 51

I planned to open a separate issue for this, but let's check first whether it can be fixed together with the vislib issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker bug Fixes for quality problems that affect the customer experience Feature:Vislib Vislib chart implementation regression Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.10.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants