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

Draw a fuller arrow #143

Merged
merged 2 commits into from
Dec 30, 2024
Merged

Draw a fuller arrow #143

merged 2 commits into from
Dec 30, 2024

Conversation

bobvanderlinden
Copy link
Contributor

@bobvanderlinden bobvanderlinden commented Dec 28, 2024

The current arrow is composed of 3 thin lines. It would be nice to have a more fatter arrow, so that the individual lines of the arrow aren't as prominent.

The arrow looked like:

image

(from small to large, unfilled first, then filled)

Now they look like:

image

This is to represent Angle and translate between radians, degrees and
Vec2D.
@gabm
Copy link
Owner

gabm commented Dec 29, 2024

I like the looks imho, code looks good to me! Would be a nice addition.

Currently I see these things

  • fixing the bug you discovered
  • can you post a before -after image?
  • doc:
    • looking this PR
    • adding an ASCII drawing as glossary (nice to have)

This changes the arrow with filled style from being 3 lines to a fuller
arrow. The non-filled style will stay using 3 lines.
@bobvanderlinden
Copy link
Contributor Author

I've updated the PR:

  • Updated the description with before and after screenshots.
  • Added better description in the comments with an attempt at ASCII-art representation 😅
  • Made sure the styles are being used by the arrow drawing. Filled style is the new fat arrow. The unfilled style uses the separate lines approach. The sizes for the two styles are being reused.

This does change the sizes of the unfilled style a bit compared to what was used previously, but it is consistent with filled style this way. Let me know what you think!

@gabm
Copy link
Owner

gabm commented Dec 30, 2024

I like the result, looks good to me!

If u feel that it's ready, please mark as ready and I'll merge.

@bobvanderlinden bobvanderlinden marked this pull request as ready for review December 30, 2024 15:39
@bobvanderlinden
Copy link
Contributor Author

Awesome 😄 Sorry, forgot this was still in draft 😅 It's ready now!

@gabm gabm merged commit da0616c into gabm:main Dec 30, 2024
3 checks passed
@bobvanderlinden bobvanderlinden deleted the fuller-arrow branch December 30, 2024 18:05
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