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
document layouts #713
document layouts #713
Changes from 1 commit
d12bfe0
f7fdc77
bb97d16
47d3bf1
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.
This example does nothing out of the box, because values.fill will always be undefined. Perhaps a more realistic example would be better? For example, maybe you could have a helper function…
And then
I guess even this feels very contrived, since it’d be better to do this on the scale definition. Maybe we should remove the custom layouts section until we can think of a better motivating example?
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.
layout: contrasting on Plot.text could be short (and fix #540). A good contrasting color not as simple as checking a color's lightness, though, so an official implementation would likely diverge from such an example.
Removing this part for now.
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.
Choosing a contrasting color sounds interesting and useful. Perhaps the fill input channel specifies the background color (e.g.,
fill: "imdb_rating"
in the Simpsons example), and then the layout outputs a new fill channel where each output color is either light or dark depending on the lightness of the input? Something like:Though, some sort of magic like this might be better…
Which, I guess, would be like a per-channel layout.
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'll open a new issue and prototype :)
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.
We shouldn’t use/recommend mutation.