Skip to content

strokeWidth as a channel #322

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

Merged
merged 2 commits into from
Aug 10, 2021
Merged

strokeWidth as a channel #322

merged 2 commits into from
Aug 10, 2021

Conversation

Fil
Copy link
Contributor

@Fil Fil commented Apr 13, 2021

@Fil Fil requested a review from mbostock April 13, 2021 12:00
@Fil
Copy link
Contributor Author

Fil commented Apr 13, 2021

do you have an idea for a test plot?

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.

I think we’ll want a scale on this, no? It feels like the sort of thing you’d want a scale for, rather than specifying literal pixels. But I guess the same argument could be made for font-size.

@Fil
Copy link
Contributor Author

Fil commented Apr 13, 2021

Can do, but I'm not sure it's worth it. Links tend to come with value=1…10 (co-occurences as in the miserables.json example), or 0…1, or sometimes from distance matrices 0…infty, and I feel it's always more "graphic tweaking" than really using it as an encoding to be shared with another mark.

@tophtucker
Copy link
Contributor

here's my recent real-world example of "stroke as scaled channel" from pairing with a friend on a correlation chart for her psych paper; stroke shows absolute value of correlation, which i just multiplied by 3: https://observablehq.com/@tophtucker/correlation-matrix-dreamcatcher

to fil's point, if i understand it — multiplying by 3 was definitely just "graphic tweaking"; it doesn't encode anything shared with another mark.

@Fil
Copy link
Contributor Author

Fil commented Apr 30, 2021

Related b66b618

@mbostock mbostock force-pushed the fil/strokewidth-channel branch from e8405b2 to a90580f Compare April 30, 2021 21:47
@mbostock
Copy link
Member

I rebased this PR to keep it alive, but I still want to think through whether there should be a scale here (if not, that’s probably fine) and whether we also need to support this on other marks (e.g., dot and line).

@Fil
Copy link
Contributor Author

Fil commented May 6, 2021

Using this for a Sankey diagram, I'll need to set the width with a scale: identity. (Doesn't mean a scale isn't going to be useful in general.)

@Fil Fil force-pushed the fil/strokewidth-channel branch from 3b03443 to 6c637ca Compare May 6, 2021 12:48
@Fil Fil force-pushed the fil/strokewidth-channel branch from 6c637ca to b23eafd Compare May 25, 2021 13:42
@ryoqun
Copy link

ryoqun commented Jul 30, 2021

@Fil @mbostock hi, is this going to be merged soon? :)

Also, I want tick's strokeWidth to be a channel as well.

@Fil
Copy link
Contributor Author

Fil commented Jul 30, 2021

See #175 and #380

@mbostock mbostock force-pushed the fil/strokewidth-channel branch from b23eafd to 5304491 Compare July 31, 2021 15:01
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.

I support not having a scale for strokeWidth (at least for now), but it does feel arbitrary to only expose this property on the link mark. I would prefer to make this a standard channel on all marks, like we do with strokeOpacity. Even if that channel often goes unused, it’s still meaningful, and should be supported for consistency/symmetry. I might take a pass at this now.

@ryoqun
Copy link

ryoqun commented Jul 31, 2021

I support not having a scale for strokeWidth (at least for now), but it does feel arbitrary to only expose this property on the link mark. I would prefer to make this a standard channel on all marks, like we do with strokeOpacity. Even if that channel often goes unused, it’s still meaningful, and should be supported for consistency/symmetry. I might take a pass at this now.

hey, I've always been fan of d3.js. thanks for moving this forward. :)

@Fil Fil marked this pull request as draft August 7, 2021 12:51
Fil added a commit that referenced this pull request Aug 7, 2021
@Fil Fil force-pushed the fil/strokewidth-channel branch 2 times, most recently from 49c0860 to c6b6dfa Compare August 7, 2021 13:27
@Fil Fil requested a review from mbostock August 7, 2021 13:28
@Fil Fil marked this pull request as ready for review August 7, 2021 13:28
@mbostock mbostock mentioned this pull request Aug 8, 2021
11 tasks
@Fil Fil force-pushed the fil/strokewidth-channel branch from c6b6dfa to 6219edf Compare August 9, 2021 10:20
@Fil Fil changed the base branch from main to mbostock/common-styles August 9, 2021 10:21
@ryoqun
Copy link

ryoqun commented Aug 9, 2021

hooray! thanks for working hard on this. so, strokeWidth for all marks? cool. :)

fyi, I'm creating a nice chart like this with plot (simply put, we're debugging any performance issues for distributed computing node's consensus formulation):

image

And I want to use strokeWidth to embed extra scalar values into marks, instead of radius for visual conciseness and proper visual comparison (radius/area isn't cognitively easy than just width for visual measuring).

@Fil
Copy link
Contributor Author

Fil commented Aug 9, 2021

wow! great example, I'd be curious to see it in action if you can share it! If you want to test this branch I've updated the build at https://observablehq.com/@fil/plot-strokewidth-322 (see also https://observablehq.com/@fil/plot-early-bird)

@ryoqun
Copy link

ryoqun commented Aug 9, 2021

I'd be curious to see it in action if you can share it!

yeah, I'd like to share but it's still in bad shape to share currently... needs some preparation to be published to the public..

If you want to test this branch I've updated the build at https://observablehq.com/@fil/plot-strokewidth-322 (see also https://observablehq.com/@fil/plot-early-bird)

seems to work nicely! (I applied strokeWidth to Plot.dot and Plot.tickX. I can't wait official new version release with it. :)

image

Base automatically changed from mbostock/common-styles to main August 9, 2021 19:32
@Fil Fil force-pushed the fil/strokewidth-channel branch from 6219edf to 267623c Compare August 10, 2021 08:45
@Fil Fil requested a review from mbostock August 10, 2021 08:46
@Fil Fil merged commit 8d7fb3b into main Aug 10, 2021
@Fil Fil deleted the fil/strokewidth-channel branch August 10, 2021 16:54
@Fil
Copy link
Contributor Author

Fil commented Aug 20, 2021

example for CHANGELOG:
strokeWidth-channel

Plot.dotX(d3.range(41), { strokeWidth: (d) => (1 + d) / 15 }).plot()

@enjalot
Copy link

enjalot commented Nov 10, 2021

Can do, but I'm not sure it's worth it. Links tend to come with value=1…10 (co-occurences as in the miserables.json example), or 0…1, or sometimes from distance matrices 0…infty, and I feel it's always more "graphic tweaking" than really using it as an encoding to be shared with another mark.

I didn't think about the "shared with another mark" aspect, but from the perspective of "Plot figures out how to map data space to graphic space for me and I just want to tweak the pixels" having a scale here is what I expect. It's similar to how I would think about the r channel in dots, where I usually want to impose some maximum size. That could be considered a graphic tweak but it's a pretty meaningful one.
I suppose for the same reason I also want a scale for fontSize, because without a scale I have to go outside of Plot to calculate the extent of the data and do the math that Plot would do for me in the other channels, like opacity

@Fil
Copy link
Contributor Author

Fil commented Nov 10, 2021

one thing I don't want is to write strokeWidth: d => d.strength > 5 ? 1 : .1 and get something different than 1 or .1 as a result, because a scale has warped the values. But if the default is identity, then we're good?

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.

strokeWidth channel on links
5 participants