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

Feat: Indicator #21

Merged
merged 7 commits into from
Sep 8, 2022
Merged

Feat: Indicator #21

merged 7 commits into from
Sep 8, 2022

Conversation

chrispader
Copy link
Member

Allow the user to show an indicator at the end of the graph. This indicator gets hidden once the hold/pan gesture gets started.

@chrispader
Copy link
Member Author

@mrousavy Ready for review! This PR is based on #17 and #18.

@gtokman
Copy link
Contributor

gtokman commented Jul 2, 2022

@chrispader do you have a video example of what the indicator looks like? Wondering what the use case would be!

@chrispader
Copy link
Member Author

This is how it looks in the app i originally had the need for this feature.

This includes the basic indicator and also a "live" pulse animation.

Upload.from.GitHub.for.iOS.MOV

@chrispader
Copy link
Member Author

Not completely finished yet though 😬

@gtokman
Copy link
Contributor

gtokman commented Jul 4, 2022

@chrispader the live pulse looks super nice!

@gtokman
Copy link
Contributor

gtokman commented Jul 4, 2022

Have you tried integrating a gradient?

Saw some discussion here #11 (comment).

@chrispader
Copy link
Member Author

Not yet! Implemented a lot of features for use cases in one of our own projects at Margelo. I'll talk to @mrousavy on this. If i find some time, i may be implementing this for general use.

@Leoputera2407
Copy link

Hey Chris Pader, I was playing around with the Spline interpolation method you integration in this PR. I think it might not be implemented correctly, as there were a few of my datapoints with higher Y positions, got rendered lower

@chrispader
Copy link
Member Author

chrispader commented Aug 31, 2022

Hey Chris Pader, I was playing around with the Spline interpolation method you integration in this PR. I think it might not be implemented correctly, as there were a few of my datapoints with higher Y positions, got rendered lower

Hi! Thanks for your input.

I already made some changes in a project for one of our clients, that address these issues..

I'll update this PR soon, so everhing works.

Sorry for the delay, kinda busy right now 😅😬

@Leoputera2407
Copy link

Leoputera2407 commented Aug 31, 2022

No worries, thanks for working on this! I have been trying to fix the interpolation issue as well. The current bezier curves only results in such blocky graphs when data is sparse.

@Leoputera2407
Copy link

@chrispader, are there any updates on this?

@chrispader
Copy link
Member Author

Not supposed to be closed, just an issue with squashing the commits. To be continued

@mrousavy mrousavy reopened this Sep 6, 2022
@chrispader chrispader force-pushed the feat/indicator branch 2 times, most recently from b27bbf9 to 9f99135 Compare September 6, 2022 13:52
Copy link
Member

@mrousavy mrousavy left a comment

Choose a reason for hiding this comment

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

Looks good and runs very smoothly - nice work!

Left just a few code org comments to make it cleaner and easy to read, then we're good to go!

@chrispader chrispader requested a review from mrousavy September 8, 2022 10:03
Christoph Pader and others added 2 commits September 8, 2022 12:18
Co-authored-by: Marc Rousavy <marcrousavy@hotmail.com>
@mrousavy
Copy link
Member

mrousavy commented Sep 8, 2022

LGTM! 🚀 nice work chris!

@mrousavy mrousavy merged commit cce1ed2 into margelo:main Sep 8, 2022
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.

4 participants