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

Added relative positioning #2

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

luisllamasbinaburo
Copy link
Contributor

@luisllamasbinaburo luisllamasbinaburo commented May 8, 2024

⚠️ Disclaimer: Significant changes, please review carefully.

With the current implementation, I face two problems:

  • Arrows are incorrectly positioned when inside a parent that has margin.
  • Arrows don't show if the target or source is outside 100% width and height, due to the SVG being sized as 100%.

I fixed this by introducing the concepts of BoundingBox and RelativeBox:

  • SVGs are sized to the BoundingBox, around the source and target BoundingBox + a safeThreshold (because some curved arrows overflow the BoundingBox).
  • SVG components are calculated in relative coordinates related to the BoundingBox.

⚠️I only modified the PerfectArrows workflow and Demo. Probably it brokes other Demos⚠️
And probably the concept needs to be ported to other arrow types or even to the Abstract.
But I don't want to change more things before having your opinion

@ChrisShank
Copy link
Collaborator

ChrisShank commented May 8, 2024

@luisllamasbinaburo Yes this are two big issues that I haven't put in the time to figure out yet, so I appreciate your contributions here! The current assumptions (that I haven’t documented anywhere) is that the arrows are absolutely positioned in the body and the body has no margin.

Im going to be away the next couple days but I’ll try think more about this problem and respond when I’m back!

If we want this logic to be picked up by every arrow, then we can put it in theAbstractArrow class.

@ChrisShank
Copy link
Collaborator

Also I’d love to learn more about what you’re planning to use quiver for and what sparked the need to have relatively position arrows?

@luisllamasbinaburo
Copy link
Contributor Author

luisllamasbinaburo commented May 8, 2024

Hello. I think I haven't explained well about the arrows positioning relative to the containers. The arrows still get positioned absolutely.

What happens is that to solve both issues, instead of creating a large SVG with 100% height and 100% width, at 0,0, in the proposal, I create an SVG adjusted to the size of the target and source boxes.

This way, now the SVG has a size that is given by the target and source, and also has an offset from top left.

Since the SVG now have an offset, to place the arrows correctly, the arrow coordinates need to be corrected, relative to the SVG's position.
Essencialy, we have to substract the SVG offset, to get the arrow in the correct position. That's what I mean by "relative positioning" (related to the SVG position).

On the other hand, my purpose in using the library is to create the relationships in roadmap https://www.luisllamas.es/en/development-roadmap/
As you can see, I can't use the library because the diagram is not positioned in 0,0 (it's inside a container), and most of the boxes are outside 100% and 100% with. So, instead, I've ultimately solved it by creating my own Astro component for the arrows.

The equations I include in the PR are the ones I've made for the Astro component, and I include as a proposal to the library in order to contribute to its development, because I think it has great potential

@ChrisShank
Copy link
Collaborator

This way, now the SVG has a size that is given by the target and source, and also has an offset from top left.

Since the SVG now have an offset, to place the arrows correctly, the arrow coordinates need to be corrected, relative to the SVG's position.
Essentially, we have to substract the SVG offset, to get the arrow in the correct position. That's what I mean by "relative positioning" (related to the SVG position).

Yup I’m think I’m following you!

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.

2 participants