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

bidirectional connection animation #1936

Closed
alixander opened this issue May 8, 2024 · 12 comments · Fixed by #1939
Closed

bidirectional connection animation #1936

alixander opened this issue May 8, 2024 · 12 comments · Fixed by #1939
Assignees
Labels
good first issue Good for newcomers render

Comments

@alixander
Copy link
Collaborator

Right now animated: true does nothing to connections where it's double-ended

  • --
  • <->

The animation can bounce back and forth in these cases.


Context:

Screenshot 2024-05-08 at 9 49 14 AM
@alixander alixander added good first issue Good for newcomers render labels May 8, 2024
@bo-ku-ra
Copy link
Contributor

bo-ku-ra commented May 9, 2024

#654

@danielsuh05
Copy link
Contributor

Hello! I'm interested in contributing to this project and this issue looks like something I'm able to tackle. Would it possible to be assigned to this issue? Thank you!

@alixander
Copy link
Collaborator Author

alixander commented May 9, 2024

@bo-ku-ra ah, your suggestion makes sense now! Thank you for pointing out the duplicate issue too.


@23danielsuh Yes, much appreciated!

I would start by adding a test, and then just modifying d2render/svg.

https://github.com/terrastruct/d2/blob/master/e2etests/testdata/txtar.txt.

After you add this test, you can keep running TA=1 ./ci/test.sh ./e2etests -run TestE2E/txtar/your-test-name -v to see changes and keep opening the new svg to see what it looks like.

@bo-ku-ra
Copy link
Contributor

bo-ku-ra commented May 9, 2024

@danielsuh05
Copy link
Contributor

I am somewhat confused which method I should be implementing. Should I do the bouncing one mentioned in #1936 or should I do the "splitting" one mentioned in #654?
From my understanding, the bouncing one would switch between the two directions every secondish.

@bo-ku-ra
Copy link
Contributor

i prefer #654 to #1936.
but my opinion may be a minority.

@alixander
Copy link
Collaborator Author

alixander commented May 10, 2024

very useful proof of concept @bo-ku-ra .

@23danielsuh Let's go with the splitting one (flowing towards endpoints from middle, for both -- and <->). I think it's smoother than bouncing, especially when there are multiple bouncing at different lengths.

@bo-ku-ra
Copy link
Contributor

(flowing towards endpoints from middle, for both -- and <->).

i want if possible.

style.animated: true;          #flowing towards endpoints from middle
style.animated-reverse: true;  # <- new

@danielsuh05
Copy link
Contributor

@bo-ku-ra For now I'm going to focus on this task (first issue!, so don't want to be be overwhelmed with things). You're welcome to open another issue though

@alixander
Copy link
Collaborator Author

yeah that can be a separate issue down the road if needed. It's hard for me to think of when someone would want that though. Flowing towards endpoints already conveys semantics of bidirectional flow of information. Maybe purely stylistic? But I'd rather not introduce a new keyword for that unless there's a strong reason I'm not seeing.

@alixander
Copy link
Collaborator Author

But if you're joining something, there's always an object in the middle. Just looks like a black hole otherwise

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers render
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants