-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
fix(colors): discover chart colors after getColorPalette update #93792
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
Conversation
@@ -106,7 +106,7 @@ function AggregatesTable({ | |||
|
|||
const numberOfRowsNeedingColor = Math.min(result.data?.length ?? 0, TOP_EVENTS_LIMIT); | |||
|
|||
const palette = theme.chart.getColorPalette(numberOfRowsNeedingColor - 2); // -2 because getColorPalette artificially adds 1, I'm not sure why | |||
const palette = theme.chart.getColorPalette(numberOfRowsNeedingColor); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn’t this still be -1
then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did that here: 6ea7821
the whole logic is a bit weird though because looking at numberOfRowsNeedingColor
, we could get 0
back from that calculation:
const numberOfRowsNeedingColor = Math.min(result.data?.length ?? 0, TOP_EVENTS_LIMIT);
which means we’d go into the negative here, so palette
would be undefined
then :/
and some more in tables, etc.