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

Add a new straight line option for drawing lines #135

Merged
merged 1 commit into from
Apr 22, 2021

Conversation

oskari
Copy link
Contributor

@oskari oskari commented Apr 9, 2021

Controlled by a prop lineStyle while still being backwards-compatible with the noCurves prop.
Maybe in a some major release we can drop the noCurves prop in favor of the lineStyle.

example:
image

@oskari
Copy link
Contributor Author

oskari commented Apr 9, 2021

I actually just noticed that this might be what PR #120 was trying to achieve, thus fulfilling issue #119

@pierpo
Copy link
Owner

pierpo commented Apr 14, 2021

... this is amazing!

Thank you so much for the contribution. I will have a deeper look, but what I've seen so far is just fantastic. The API makes totally makes sense, too!

Same as the other PR, I'll have a deeper look in the following days, then merge & publish 😊

Copy link
Owner

@pierpo pierpo left a comment

Choose a reason for hiding this comment

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

I'm really amazed that you pulled this off so easily. I thought it would be way more complex! Thank you for your help, this is amazing!

However, there are merge conflicts now that I merged your other PR. Could you resolve them? 😊

Controlled by a prop `lineStyle` while still being backwards-compatible
with the `noCurves` prop.
@oskari
Copy link
Contributor Author

oskari commented Apr 19, 2021

I'm really amazed that you pulled this off so easily. I thought it would be way more complex! Thank you for your help, this is amazing!

However, there are merge conflicts now that I merged your other PR. Could you resolve them? 😊

Can't take much credit for it. Spend few hours trying to figure out how it should be calculated when I realized that the offset calculation was already doing it, so had to just adapt it a bit.

Merge conflicts should be resolved now. I added the startMarker to eighth example as I've been using that to verify everything looks like they're supposed to do.

@pierpo
Copy link
Owner

pierpo commented Apr 22, 2021

Well, that's fantastic. Truly amazing job, thank you so much for the contribution!

@pierpo pierpo merged commit a4bafd3 into pierpo:master Apr 22, 2021
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.

None yet

2 participants