Open wyhleonard opened 3 years ago
Ah excellent! I was worried about this shape too https://observablehq.com/@fil/trail-marks ;-)
Another bit that would need fixing in this shape is when the data has more than 2 points—currently it draws one "loop" for each pair, with the consequence that the stroke is not the outline, and the shape has superpositions.
(You will also need to declare the function in src/index.js, show it in README.md and test it in tests/ for the PR to be complete.)
Thank you for your suggestions, I will further improve this function with your suggestions. Actually, it's my first experience to contribute to open source libraries. So if i make some mistakes, please accept my most sincere apology.
The second commit has improved the function of d3.trail(), including fixing the gotchas of superpositions and declaring the function in src/index.js, showing it in README.md and testing it in tests/trail-test.js. Now, d3.trail() can draw the trail marks as
However, there are still some problems in README.md. For example, it is unable to display pictures properly, and there is an unexpected slant in the text after Lines.
Wow this is beautiful!
(The images will display only when the PR is merged (they link to the master branch).)
Two small issues:
Have you pushed all the code? To build this branch I had to add:
import array from "./array.js";
Secondly, the data from https://observablehq.com/@fil/trail-marks breaks it, because I've left a little 💣 with coincident points ( [300, 200, 0], [300, 200, 0.1] ).
The third commit has solved some small issus in d3.trail(). Note that the point will be only drawed when its size is greater than 0.
The fourth commit has completed the changes requested. Now, d3.trail() can draw a trail mark with coincident points as
Thank you for your reminder. I have followed D3's general style conventions in the fifth commit.
Thank you. Beautiful work! I think @mbostock and @jheer will be interested to review.
It's my honor to contribute to D3. Thank you for your help again.
It's great to see someone improving the trail mark in Vega and adapting it to d3! Thanks!
I took a quick glance at the code, but didn't check the adapted logic for correctness. That said, I did notice some possible concerns:
intersectPoint
and commonTangent
generate object-valued outputs, possibly causing some performance overhead due to repeated object instantiation.else{
vs. else {
, if(
vs. if (
, function(test){
vs. function(test) {
.Thank you for your suggestions! The sixth commit has taken the two concerns above.
Thank you for your suggestions! The lastest commit has refined the codes and docs in d3-shape.
This commit added the function as trail marks in Vega, which is similar to line marks, but can have variable width determined by backing data . Moreover, this commit also fixed the two gotchas described in the issue. Users can use d3.trail() like d3.line() to create the trail marks they want. The first picture is produced by the method in Vega of trail marks, and the second picture is produced by the method in this commit.