Skip to content

Conversation

@mbostock
Copy link
Member

Fixes #1412.

@mbostock mbostock requested a review from Fil May 20, 2023 03:03
src/marker.js Outdated
.attr("markerWidth", 6.67)
.attr("markerHeight", 6.67)
.attr("orient", "auto")
.attr("orient", "auto-start-reverse")
Copy link
Member Author

Choose a reason for hiding this comment

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

This is especially noticeable with tick and rule (as shown in the tests); I think this is a better default for the arrow marker.

Copy link
Contributor

@Fil Fil May 20, 2023

Choose a reason for hiding this comment

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

Agree it's a better default for rules and ticks, that (typically) indicate an extent; but for a line that (typically) indicates a sequence, it feels a bit wrong if the first arrow is pointing outwards.

Copy link
Member Author

Choose a reason for hiding this comment

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

It feels less wrong than having the arrows point the same way for a rule and a line. I don’t see any practical demand for having an arrow at the start of the line; you’d typically use markerMid instead, and possible markerEnd.

The alternative is we have to introduce two types of marker, one for line (arrow if we retain backwards compatibility) which will almost never be used for that purpose, and another for rule and tick (arrow-reverse, say) which won’t be as discoverable.

Copy link
Member Author

Choose a reason for hiding this comment

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

I’ve reverted the changes to the arrow marker and added an arrow-reverse marker in the latest commit.

@mbostock mbostock changed the title rule & tick markers; arrow auto-start-reverse rule & tick markers; arrow-reverse marker May 20, 2023
Copy link
Contributor

@Fil Fil left a comment

Choose a reason for hiding this comment

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

If you wanted to have the best defaults, we could imagine two new options arrow-auto and arrow-reverse, with arrow defaulting to arrow-reverse for rules and ticks, and to arrow-auto for lines?

Just a suggestion, I'm OK with both approaches.

@mbostock
Copy link
Member Author

Interesting; I hadn’t consider mark-specific defaults for markers but that would be possible. I think this is fine for now. Perhaps it’s better to be more explicit when you want bidirectional arrows. And I don’t know if there will be much demand for markers on rules and ticks in any case.

@mbostock mbostock enabled auto-merge (squash) May 21, 2023 17:52
@mbostock mbostock force-pushed the mbostock/rule-marker branch from 38c35f1 to a4b4435 Compare May 21, 2023 17:52
@mbostock mbostock merged commit 294baba into main May 21, 2023
@mbostock mbostock deleted the mbostock/rule-marker branch May 21, 2023 17:54
chaichontat pushed a commit to chaichontat/plot that referenced this pull request Jan 14, 2024
* rule & tick markers; arrow auto-start-reverse

* docs edits

* arrow-reverse marker
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.

The rule and tick marks could support marker options.

3 participants