d2target: add support for box arrowheads #2256
Conversation
| 0., 0., | ||
| 0., height, | ||
| width, height, | ||
| width, 0., |
There was a problem hiding this comment.
I wasn't sure how to calculate the values here - I'm guessing because they're different for every other arrowhead and mine aren't I've missed something
There was a problem hiding this comment.
(Similarly for the unfilled box below)
There was a problem hiding this comment.
This is probably where stroke-width comes in -- see main review comment
There was a problem hiding this comment.
I wasn't sure if these files were meant to be included but as they weren't gitignored I decided to - will remove if that's what I should do!
There was a problem hiding this comment.
nah sorry i should do something about this testing noise. Please exclude the pptx and pdfs from PR.
There was a problem hiding this comment.
Thanks @adamratson ! Sorry for the late review, hope you had a good holiday season.
The math doesn't seem to account for stroke width values. e.g. see circle here vs the boxes that overlap
| 0., 0., | ||
| 0., height, | ||
| width, height, | ||
| width, 0., |
There was a problem hiding this comment.
This is probably where stroke-width comes in -- see main review comment
There was a problem hiding this comment.
nah sorry i should do something about this testing noise. Please exclude the pptx and pdfs from PR.
Add support for box arrowheads in d2target and d2sketch
Fixes #2227
I've updated tests where I've seen I should do so - please let me know if I've missed any!!
Pic:
Please let me know if I've missed anything - first time contributor so good chance I have!
I'll try and act on feedback over the week :)