Skip to content

Dodge: use the mark's this.r if present #711

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 4 commits into from

Conversation

Fil
Copy link
Contributor

@Fil Fil commented Jan 27, 2022

otherwise default to reading the options, then 3.

follows [7a845f0]

@Fil Fil requested a review from mbostock January 27, 2022 13:51
@Fil Fil mentioned this pull request Jan 27, 2022
Co-authored-by: Mike Bostock <[email protected]>
@Fil Fil mentioned this pull request Jan 27, 2022
1 task
symbol: "species",
fill: "species",
stroke: "white",
strokeWidth: .5
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
strokeWidth: .5
strokeWidth: 0.5

Copy link
Member

Choose a reason for hiding this comment

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

Or about instead

Plot.dot(penguins, Plot.dodgeY({x: "body_mass_g", symbol: "species", stroke: "species"}))

Screen Shot 2022-01-27 at 11 05 48 AM

Then we don’t need the little stroke for separation.

strokeWidth: .5
}))
],
symbol: {legend: true}
Copy link
Member

@mbostock mbostock Jan 27, 2022

Choose a reason for hiding this comment

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

For Plot options, I prefer the following order:

  • Layout (e.g., height)
  • Scales
  • Facets
  • Marks

I think (hope) nearly all the tests conform to this pattern, and would like to be consistent.

@Fil Fil changed the title Use the mark's this.r if present Dodge: use the mark's this.r if present Feb 10, 2022
@Fil Fil marked this pull request as draft February 23, 2022 14:23
@Fil Fil mentioned this pull request Feb 23, 2022
@mbostock
Copy link
Member

mbostock commented Mar 1, 2022

Superseded by #775.

@mbostock mbostock closed this Mar 1, 2022
@mbostock mbostock mentioned this pull request Mar 11, 2022
16 tasks
@Fil Fil deleted the fil/dot-dodge-4.5 branch October 16, 2022 18:41
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