[Lens] Threshold: set default color for new thresholds #113008
Conversation
|
Pinging @elastic/kibana-vis-editors (Team:VisEditors) |
|
@elasticmachine merge upstream |
| @@ -457,6 +458,7 @@ export const getXyVisualization = ({ | |||
| icon: undefined, | |||
| lineStyle: 'solid', | |||
| lineWidth: 1, | |||
| color: euiLightVars.euiColorDarkShade, | |||
There was a problem hiding this comment.
small nit: Should we store the color as long as the user didn't change it or should we just keep it as undefined for this case (and handle it in to_expression and in the config UI)? This will allow us to change the default color later on. Same for line width and line style.
There was a problem hiding this comment.
It was planned to refactor this part anyway, so I'll pick up the ball here
|
@elasticmachine merge upstream |
| icon: undefined, | ||
| lineStyle: 'solid', | ||
| lineWidth: 1, | ||
| // icon: undefined, |
There was a problem hiding this comment.
are these comments to remove?
| @@ -60,6 +64,10 @@ export const ColorPicker = ({ | |||
| const datasource = frame.datasourceLayers[layer.layerId]; | |||
| const sortedAccessors: string[] = getSortedAccessors(datasource, layer); | |||
|
|
|||
| if (layer.layerType === layerTypes.THRESHOLD) { | |||
There was a problem hiding this comment.
nit: could we move this if to line 63 to not calculate datasource or sortedAccessors unnecessarily?
|
The threshold are all gray as expected, but the little indicators on config panel keep the default colors from the palette (blue and red) (screenshot below) |
|
Thanks for the fix Marco. The fix solves the problem, but it keeps assigning the colors for non-data layers - for example in this case in the second data layer, the color from the round-robin assignment should be blue (2nd on the palette) but it's coral pink (3rd) because blue was assigned and ignored for the thresholds. If it's simple, maybe you could look into skipping the thresholds layer when making the assignments?
|
|
With latest fix it should skip the threshold layers when computing color assignments/rankings |
| @@ -111,6 +117,13 @@ export function getAccessorColorConfig( | |||
| triggerIcon: 'disabled', | |||
| }; | |||
| } | |||
| if (layer.layerType === layerTypes.THRESHOLD) { | |||
There was a problem hiding this comment.
nit: could we move it up the line 112? Same as in my previous comment, it's an early return and thanks to that we don't have to execute the lines before (currentYConfig finding)
There was a problem hiding this comment.
finding currentYConfig is still required to return a user defined color for the yConfig
💚 Build Succeeded
Metrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
* 💄 Make dark grey default threshold color * ✅ Fix test * 👌 Integrate feedback * 👌 Fix bug * 👌 Filter threshold layers for color assignments * 👌 Small refactor * 🐛 Fix merging conflicts Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
…3598) * 💄 Make dark grey default threshold color * ✅ Fix test * 👌 Integrate feedback * 👌 Fix bug * 👌 Filter threshold layers for color assignments * 👌 Small refactor * 🐛 Fix merging conflicts Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Marco Liberati <dej611@users.noreply.github.com>
Summary
Fixes #112796
Set dark grey from EUI as default color for new thresholds.
Checklist
Delete any items that are not applicable to this PR.