Skip to content
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

Add angle, angleref and standoff to marker and add backoff to line as well as adding two new arrow symbols #6297

Merged
merged 30 commits into from
Oct 5, 2022

Conversation

archmoj
Copy link
Contributor

@archmoj archmoj commented Aug 12, 2022

@plotly/plotly_js
cc: @chriddyp @LiamConnors

 - add arrow and arrow-narrow symbols
@nicolaskruchten
Copy link
Contributor

Super exciting!

With this PR, can we have e.g. variable-sized circular markers with arrowheads pointing to them without overlapping them? This is pretty common in maps with paths, and in network diagrams:

image

@archmoj
Copy link
Contributor Author

archmoj commented Aug 12, 2022

Super exciting!

With this PR, can we have e.g. variable-sized circular markers with arrowheads pointing to them without overlapping them?

I could introduce that functionality via marker.offset or something.

@nicolaskruchten
Copy link
Contributor

That probably wouldn't be enough, as to get the effect above, we'd need circular (or square or diamond or...) markers and arrows and for the offset to be dynamic i.e. to match the marker size.

@alexcjohnson
Copy link
Collaborator

Let's leave arrowheads+markers out of this PR. If we did want to do that, I suspect it'd be better to add a separate arrowhead option than to try and shoehorn it into the marker framework - and at that point it could automatically back off to the edge of the marker.

Note for annotations, we called this "pull back from the target point" standoff. We could certainly add that on its own, which would allow certain effects people might occasionally want - use a centered marker as an arrow for example, or give the marker an outline and point the edge of the outline at the target point. And it would allow particularly motivated users to hack your arrowheads+markers too :)

But again, not for this PR.

@archmoj
Copy link
Contributor Author

archmoj commented Aug 16, 2022

@LiamConnors FYI - I pushed some commits after the discussion we had.

@archmoj
Copy link
Contributor Author

archmoj commented Aug 25, 2022

Super exciting!

With this PR, can we have e.g. variable-sized circular markers with arrowheads pointing to them without overlapping them? This is pretty common in maps with paths, and in network diagrams:

image

@nicolaskruchten Good call.
We decided to make this option available too.
See f5e269d.

@alexcjohnson
Copy link
Collaborator

Looks like gl2d has a problem with the original arrow symbols... clipping them but also not drawing the bars when requested. From the test images (open variant as it's easiest to see, but applies to all variants), here's svg:
Screen Shot 2022-08-26 at 11 33 30
and gl2d:
Screen Shot 2022-08-26 at 11 33 46

The missing bars may not be new in this PR, but the clipping I'm guessing is new (or at least more prominent now) since it's based on the angle.

@alexcjohnson
Copy link
Collaborator

We'll need to back off the end of the line, when using angleref='previous' and angle=0 so it's hidden under the marker to the extent possible - obviously a wide enough line can't have its end hidden by the marker, but a medium-width line can. For annotations we did this by giving each arrowhead a specific backoff parameter that causes the line to end where its center is definitely under the arrowhead, but at the widest point possible to accommodate the largest possible width.

You can see the problem a little bit in some of the existing mocks, for example polar-direction there's some pink poking out the front of the blue markers. But it becomes more blatant if you make the line width bigger, for example on z-line-shape-arrow:
Screen Shot 2022-08-26 at 13 25 11

or z-marker-standoff:
Screen Shot 2022-08-26 at 15 06 05

This backoff should only happen at the end of a path; if the path continues through the point in question it should be unchanged. This might produce unexpected results when you add an extra marker-only trace like z-marker-standoff, because you've got two segments and only the second one will pull the line back. But I don't think it makes sense otherwise - you could argue that if there's a nonzero standoff we should pull the line back anyway, but users might add a standoff without an extra marker in order to account for marker.line.width so I don't think we can rely on that. To get the effect where all segments are pulled back you can always double up those middle points and put a null between.

archmoj and others added 2 commits August 26, 2022 18:37
@archmoj archmoj changed the title Add angle, angleref and standoff to marker as well as two new arrow symbols Add angle, angleref and standoff to marker and add backoff to line as well as adding two new arrow symbols Sep 1, 2022
@archmoj
Copy link
Contributor Author

archmoj commented Sep 19, 2022

Looks like gl2d has a problem with the original arrow symbols... clipping them but also not drawing the bars when requested. From the test images (open variant as it's easiest to see, but applies to all variants), here's svg: Screen Shot 2022-08-26 at 11 33 30 and gl2d: Screen Shot 2022-08-26 at 11 33 46

The missing bars may not be new in this PR, but the clipping I'm guessing is new (or at least more prominent now) since it's based on the angle.

I'd be great to propose potential changes to address this edge case in a separate PR. I'll open an issue for it.

Co-authored-by: Alex Johnson <alex@plot.ly>
@alexcjohnson
Copy link
Collaborator

Looks like we're adding backoffs and prohibiting spline in some cases we shouldn't. For example:

Plotly.newPlot(gd,[{
    x:[0,1,1], y:[0,1,0],
    marker:{size:50, symbol:'arrow'},
    line:{width:20, shape:'spline'}
}],
{width:400,height:400})

With or without asking for spline there, the implied line.backoff should have been 0 because angleref isn't included so it defaults to up, and thus spline should have been accepted. Instead we get:
Screen Shot 2022-09-26 at 08 43 35
If I explicitly set line.backoff=0 we get the right behavior:
Screen Shot 2022-09-26 at 08 45 30

@archmoj
Copy link
Contributor Author

archmoj commented Oct 3, 2022

Looks like we're adding backoffs and prohibiting spline in some cases we shouldn't. For example:

Plotly.newPlot(gd,[{
    x:[0,1,1], y:[0,1,0],
    marker:{size:50, symbol:'arrow'},
    line:{width:20, shape:'spline'}
}],
{width:400,height:400})

With or without asking for spline there, the implied line.backoff should have been 0 because angleref isn't included so it defaults to up, and thus spline should have been accepted. Instead we get: Screen Shot 2022-09-26 at 08 43 35 If I explicitly set line.backoff=0 we get the right behavior: Screen Shot 2022-09-26 at 08 45 30

Good catch! Addressed in b4ef2ab.

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

Great work! All my comments have been addressed nicely. 💃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature something new
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants