Skip to content

color, opacity and radius legends #432

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

Closed
wants to merge 16 commits into from
Closed

Conversation

Fil
Copy link
Contributor

@Fil Fil commented Jun 10, 2021

sequel of #425

build at https://observablehq.com/d/c324a9a466e66329

todo:

  • fix API (1 options object)
  • examples
  • remove dependency on htl
  • fix the figure so it gets the same width as the plot
  • expose and document Plot.colorLegend
  • continuous date color legend

closes #23
closes #236

related: #397 #92

@Fil Fil mentioned this pull request Jun 10, 2021
5 tasks
@mbostock
Copy link
Member

The color legend should inherit the scale’s automatic label, no? E.g., here both the x-axis and color legend should be labeled with “random”, without you needing to specify a label explicitly.

Plot.plot({
  color: {
    legend: true,
    width: 120,
    nice: true
  },
  marks: [
    Plot.dot({length: 100}, {
      x: Object.assign(() => Math.random(), {label: "random"}),
      fill: Object.assign(() => Math.random(), {label: "random"})
    })
  ]
})

@Fil Fil marked this pull request as draft June 10, 2021 15:48
@Fil
Copy link
Contributor Author

Fil commented Jun 10, 2021

color legend should inherit the scale’s automatic label

Yes that's the intent. I swear it was working at some point :) EDIT: In fact it was working, I just hadn't updated the build on the test page.

Thank you for the review! I should have marked this as a Draft because there's quite a lot of screws to tighten.

I think this shows that the "scale options" object (returned as plot.scales.color etc) should contain all the options that we passed and that were not consumed when building the scale.

Also, I want more unit tests—for example I realize that calling Plot.scale on a diverging scale will crash if we don't tell it that the key is "color".

@Fil Fil force-pushed the fil/color-legends branch from 2f9cf94 to 8b686ff Compare June 12, 2021 18:00
@Fil Fil marked this pull request as ready for review June 12, 2021 18:00
@Fil Fil mentioned this pull request Jun 13, 2021
@Fil Fil changed the title color legends color, opacity and radius legends Jun 13, 2021
@Fil Fil mentioned this pull request Jul 21, 2021
8 tasks
@Fil Fil mentioned this pull request Aug 5, 2021
7 tasks
@Fil
Copy link
Contributor Author

Fil commented Aug 5, 2021

deferring to #484

@Fil Fil closed this Aug 5, 2021
@Fil Fil deleted the fil/color-legends branch August 30, 2021 15:18
@Fil Fil mentioned this pull request Jan 12, 2022
1 task
@Fil
Copy link
Contributor Author

Fil commented May 18, 2022

see #665

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.

2 participants