Skip to content

Conversation

Fil
Copy link
Contributor

@Fil Fil commented Mar 7, 2023

Because of f028a08 color scale overrides didn't work for Plot.raster

Capture d’écran 2023-03-07 à 14 22 10

closes #1317

@Fil Fil requested a review from mbostock March 7, 2023 13:25
@Fil Fil force-pushed the fil/raster-recolor branch from 9a1aa45 to 124e49b Compare March 14, 2023 12:04
return super.scale(channels, scales, context);
}
render(index, scales, channels, dimensions, context) {
const color = scales.color ?? ((x) => x);
const color = this.recolor && scales.color ? scales.color : (x) => x;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you want something like this:

Suggested change
const color = this.recolor && scales.color ? scales.color : (x) => x;
const color = scales[this.channels.fill?.scale] ?? (x) => x;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hadn't thought about that, but it makes sense!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this.channels.fill.scale comes in as "auto" and not "color", so I had to adapt a bit

Fil and others added 2 commits March 14, 2023 14:52
Copy link
Member

@mbostock mbostock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it wasn’t clear, but I’m trying to avoid needing a side-effect in the raster.scale method. Why can’t we just figure out the appropriate color scale in raster.render instead?

Edit: You may need to call inferChannelScale to convert auto to something more specific.

@mbostock mbostock merged commit 820144e into main Mar 14, 2023
@mbostock mbostock deleted the fil/raster-recolor branch March 14, 2023 21:14
chaichontat pushed a commit to chaichontat/plot that referenced this pull request Jan 14, 2024
* closes observablehq#1317

* Update src/marks/raster.js

Co-authored-by: Mike Bostock <mbostock@gmail.com>

* don't hardcode that fill is scaled with {color}

* expose channel states to render

---------

Co-authored-by: Mike Bostock <mbostock@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Per-channel scale override fails with Plot.raster
2 participants