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

Chart interactions #72

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

Conversation

prithsharma
Copy link
Collaborator

Work in progress

Since I am not very familiar with both Objective-C and Swift, bootstrapping PR for early comments. Apart from general comments, looking for suggestions on the question below:

  • Since the implementation for the delegate would mostly remain same for all chart types, how can I write the implementation once instead of copying it over to class definitions of all charts?

@prithsharma
Copy link
Collaborator Author

I tried to add an onSelect property to RNLineChart and use it in the chartValueSelected delegate implementation. Unfortunately, I am facing a weird error that Value of type RNLineChart has no member 'onSelect'. Properties declared in Objective-C should be accessible in Swift class definitions, but I am most probably missing some basic concept.

Concerned commit - ffceb1f

@Jpadilla1 would you be able to help on this? The intermixing of Swift and Objective-C code is making it more confusing for a noob like me.

@prithsharma
Copy link
Collaborator Author

So I made a little progress. In the commit dd3fe04, I was able to get hold of the onSelect property using an instance variable and pass on an event to JS.
@Jpadilla1 please take a look and see if this looks OK to you. I can really use some Objective-C/iOS knowledge here. 😄

@prithsharma
Copy link
Collaborator Author

Selection/unselection selectors work for LineChart now.
@Jpadilla1 please review. If there is a neater way than copying the delegate implementation to all chart class definitions, I'll really appreciate the suggestion.

@prithsharma
Copy link
Collaborator Author

@Jpadilla1 did you get a chance to look at this?

@basequatro
Copy link

This a great PR, congrats.

@Jpadilla1
Copy link
Owner

Sorry for the late response @prithsharma, will take a look at it today.

@prithsharma
Copy link
Collaborator Author

@Jpadilla1 did you get a chance to look at this one?

@L-Jovi
Copy link

L-Jovi commented Jan 10, 2017

@prithsharma cool, and I notice the author don't have enough time to continue maintain the project from this issue :(

@wuxudong
Copy link

wuxudong commented Mar 9, 2017

@prithsharma chartValueSelected listener is also implemented at https://github.com/wuxudong/react-native-charts-wrapper . and it is shared among all chart type. may be you will want to take a look

@prithsharma prithsharma changed the title [WIP] Chart interactions Chart interactions May 19, 2017
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.

5 participants