Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[Lens] Categorical color palettes #75309
[Lens] Categorical color palettes #75309
Changes from 91 commits
bebac29
e983014
3fbff62
426f2ce
44a17c3
8c7381e
64ee8ef
9bb7843
c1ddaaa
e51ff5e
e3c6f94
a278772
bfb6898
b3fb6e8
2434486
85c128e
1ece895
34cff58
59ac920
51ac271
57cfda7
f65b9fb
35b421e
e152365
01d1cd8
bf81263
c3cab97
0082554
99266bc
2094bb4
d1cfe2b
d232351
b6a36fb
b482572
7e743fc
d912bcf
e1b12b2
e5e5e56
1707055
1dc530a
20d28a6
776e0a0
5d9d582
c13a2a6
611d4b5
6b7ccc2
5c46182
fea7428
7a99aea
3b49960
aafbbda
161c1d5
86137ef
83cf453
99e3d4a
0b20bdc
977450b
1df62ba
2903a51
d251edf
548a785
5e69f35
c7e8107
21583dd
d0e13f0
adc5b3e
624dec1
c655724
dbbb5b3
5c78a2c
8c0cf88
b216c89
2316149
babe0de
e4b36e0
417346e
4508492
b93e511
9d25e07
90c8cd6
3875de0
e558cb1
559d048
c54b88d
ba3da6c
348e045
dcb7897
64cf4bb
163c8e1
5a23904
e4fd331
382f71e
c2de739
49f2294
2575403
df33fa2
7c085ed
79065b4
6926a6c
0d20d56
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
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.
Ah, forgot about that, thanks for the great suggestion!
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.
This choice of palette for the fallback doesn't make a lot of sense to me- why not default to EUI, or throw an error when missing a palette definition?
I worry that we could get stuck with a palette that can never be changed, similar to the 7-color default vislib palette. Also, the name of the palette here is Paul Tol 14
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.
This is done for BWC with Canvas. The
palette
function is only used by canvas and is already in use there (with these defaults). We can think about changing this with 8.0There 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.
I see, can you add a code comment about this?
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.
I don't understand what this is supposed to contain- or even what the datatype is. Do we need this?
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.
It's not used right now, but this is the place system palettes would use that to serialize custom params. E.g.
{system_palette id="myFancyPalette" params={fancy_palette_params somethingCustom=true }}
. We can also remove it for now if you think that would be better.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.
My preference would be to remove it until needed.