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

Edit money curves to appear to go 'up' #112

Merged
merged 2 commits into from
Apr 17, 2023
Merged

Conversation

travissouthard
Copy link
Contributor

Overview

These changes check if the offsetX value in the getBezierOffsetLatLng function in the useCreateArcPath hook is negative (i.e. the end of the arc is West of DC), then to set the thetaOffset value to be on the other side of the polar coordinate "line" between our points' offset values. Without doing this all the curves will make a hurricane-like pattern and not quite look right.

I got to this from looking at the blog post that I believe this team forked the original implementation from.

Closes #88

Demo

Before:
image

After:
image

Testing Instructions

  • In this branch, run ./scripts/server and navigate to the page
  • Play the animated map and ensure:
    • The arcs all curve up (both going east and west (there are only a few east-bound examples))
    • The curves are mostly distinguishable from each other (sorry Midwest)

Checklist

  • fixup! commits have been squashed
  • CHANGELOG.md updated with summary of features or fixes, following Keep a Changelog guidelines
  • README.md updated if necessary to reflect the changes
  • CI passes after rebase

@github-actions
Copy link

github-actions bot commented Apr 14, 2023

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ ACTION actionlint 1 0 0.02s
✅ BASH shfmt 11 0 0.01s
✅ JAVASCRIPT eslint 45 0 4.73s
✅ JAVASCRIPT prettier 45 0 3.15s
✅ JSON eslint-plugin-jsonc 4 0 1.7s
✅ JSON jsonlint 4 0 0.18s
✅ SPELL misspell 90 0 0.12s
✅ TERRAFORM terraform-fmt 5 0 0.21s
✅ YAML yamllint 5 0 0.27s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by OX Security

Copy link
Contributor

@jacobtylerwalls jacobtylerwalls left a comment

Choose a reason for hiding this comment

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

Looking great. At your option, you could consider factoring out the Math.PI / 10 so that the conditional just sets the -1 or 1 sign.

@travissouthard
Copy link
Contributor Author

Looking great. At your option, you could consider factoring out the Math.PI / 10 so that the conditional just sets the -1 or 1 sign.

Yeah, I went back and forth on that. But after a weekend of distance, I think factoring that out is the way.

@travissouthard travissouthard merged commit 37666d0 into develop Apr 17, 2023
3 checks passed
@travissouthard travissouthard deleted the ts/reverse-the-curves branch April 17, 2023 16:26
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.

Adjust animated arcs to mimic curve along great circle route
2 participants