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

Feature implementation Rotation along path and path context with toPoints and toKeyPoints #566

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

noobd3v
Copy link

@noobd3v noobd3v commented Apr 10, 2023

Rotation along the path of layout

@noobd3v
Copy link
Author

noobd3v commented Apr 10, 2023

@microsoft-github-policy-service agree

@danmarshall
Copy link
Contributor

Thanks for the contribution! Can I ask that you:

  • Remove changes to package-lock.json files
  • Remove changes to .js files

In this case, let's only make changes to .ts files

@noobd3v
Copy link
Author

noobd3v commented Apr 11, 2023

Thanks for the contribution! Can I ask that you:

  • Remove changes to package-lock.json files
  • Remove changes to .js files

In this case, let's only make changes to .ts files

Reverted other changes.

@noobd3v
Copy link
Author

noobd3v commented Apr 17, 2023

can we resolve the comments or changes needed?

@noobd3v noobd3v changed the title Feature implementation Rotation along path Feature implementation Rotation along path and path context with toPoints and toKeyPoints Apr 19, 2023
@noobd3v
Copy link
Author

noobd3v commented Apr 25, 2023

Hi @danmarshall I have made changes as per the comments please check and let me know if all looks good and we can merge and close the pr.

(chainPoints)=> {
for(let i = 0; i < chainPoints.length; i++)
{
alongPathAngles.push(angle.ofPointInDegrees(chainPoints[i].link.endPoints[0], chainPoints[i].link.endPoints[1]));
Copy link
Contributor

Choose a reason for hiding this comment

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

Here is where we need to look at the angle at ratio r (if it is an arc)

@danmarshall
Copy link
Contributor

Hi @noobd3v , thanks for the contribution and your patience. As we go through this implementation, we are uncovering many ideas that we need to think through carefully. Also, I think we will need to add a couple of small unit tests which exercise the new feature.

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