joshuapinter / react-native-unified-contacts

Your best friend when working with the latest and greatest Contacts Framework in iOS 9+ in React Native.
MIT License
158 stars 56 forks source link

Android support? #16

Closed vzhen closed 6 years ago

vzhen commented 7 years ago

When ?

joshuapinter commented 7 years ago

Working on it right now. Basic support by the end of the weekend, including, checking permissions, reading contacts, etc.

vzhen commented 7 years ago

I m ok with basic support as long as with read all contacts. Big thanks

vzhen commented 7 years ago

By the way i hope it support android 4 above.

joshuapinter commented 7 years ago

It's currently setup to target a minimum SDK version of 16, which is 4.1. And I'm testing it on a Google Pixel runnin Android 7.

Is the only thing you're going to be using it for is to fetch all Contacts? What else? Selecting a Contact? Etc.?

Let me know so I can consider it as I'm writing it.

joshuapinter commented 7 years ago

@vzhen Basic Android version nearly done. Feel like doing some testing?

joshuapinter commented 7 years ago

@vzhen Okay, Android version complete for getting Contacts and checking permissions. Just waiting for you now. Let me know when you're free and I'll give you the instructions.

vzhen commented 7 years ago

Sorry for the late reply. Yes i just need to fetch all contacts at the moment but may need add, update, delete in future. I will test and let you know within this week.

joshuapinter commented 7 years ago

Cool. I'm just at family dinner now but I'll add some instructions when I'm back for you.

joshuapinter commented 7 years ago

@vzhen Couple questions:

1) Are you searching for contacts by name or just getting ALL the contacts?

2) Do you need access to the Contact's full resolution picture or is their thumbnail image fine?

vzhen commented 7 years ago
  1. Just need to fetch all the contacts
  2. Don't need any picture or thumbnail But i'm not sure is that possible, i need to listening on new contact added. Which mean if there is a new contact then i will ONLY get the contact.

I'm ready to test it. Do you have android setup instruction?

joshuapinter commented 7 years ago
  1. No problem. That's ready now. See my next comment for instructions.

  2. Okay.

  3. Listening for new Contacts is interesting but will have to wait until later. I've never done that before. Maybe after you get it working, you can send a Pull Request to make the library handle it.

joshuapinter commented 7 years ago

Android Instructions

1) Add "react-native-unified-contacts": "joshuapinter/react-native-unified-contacts#android" to your package.json file and npm install.

2) In your console in your project root, run rnpm link react-native-unified-contacts. And if that doesn't work, run react-native link react-native-unified-contacts instead.

3) Add import Contacts from 'react-native-unified-contacts'; to your app where you want to use it.

4) Then get all the contacts with this:

Contacts.getContacts()
      .then( function(contacts) { console.log(contacts); } )
      .catch( function(error) { console.log(error); } );

Let me know how it goes.

Josh

vzhen commented 7 years ago

react-native link didn't include 'react-native-unified-contacts' into android\setting.gradle. I add it manually but then another error comes out * What went wrong: A problem occurred configuring project ':react-native-unified-contacts'. failed to find target with hash string 'android-25' in: D:\Development\Android\sdk

joshuapinter commented 7 years ago

@vzhen Working on this right now. When will you be able to test it again?

joshuapinter commented 7 years ago

@vzhen I know what that error message is saying. You need to install the latest Android SDK, which is API version 25 for Android 7.1.1. Do this through your SDK Manager in Android Studio. Then everything will work.

Try that and let me know how it goes.

the-habu commented 7 years ago

I'd also be very interested in Android support. If I can support you with testing, please let me know!

joshuapinter commented 7 years ago

@geekazoid-at You're in luck, it's almost ready. Can you test it out following the instructions here: https://github.com/joshuapinter/react-native-unified-contacts/issues/16#issuecomment-273193105

And let me know how it goes.

If you get the same error message as @vzhen, it's because you need to update your SDK Manager with Android 7.1.1 for it to compile properly.

Thanks!

joshuapinter commented 7 years ago

@geekazoid-at Have you had a chance to test it on Android yet?

joonhocho commented 7 years ago

@joshuapinter @geekazoid-at I have a fork joonhocho/react-native-contacts, if you want to refer how to get all contact information from Android. Note that I only use it to get all contact info. Not updates or add. My suggestion is that if you will be developing for Android, don't try to rename things on native side, but rather try to normalize it on js side. For example, Android uses emails rather than while iOS uses emailAddresses.

joshuapinter commented 7 years ago

@joonhocho Nice, thanks for posting that. I'll use it to help finish up the Android support thanks.

MattFoley commented 7 years ago

@joshuapinter Was any of the Android support ever merged in?

joshuapinter commented 7 years ago

@MattFoley Hey Matt. A lot of it has been done but it's sitting on the android branch.

I've been meaning to finish it up and merge it in but it won't be for a few weeks unless I get help with it. When do you need it by?

MattFoley commented 7 years ago

No real hurry, and I don't really have a problem just using your existing branch as is, thanks for the quick response! :)

joshuapinter commented 7 years ago

@MattFoley No worries. I'll let you know when it has feature parity. Let me know if you run into any trouble.

brettdonald commented 6 years ago

Poke. I am also waiting on Android support.

joshuapinter commented 6 years ago

@brettdonald Right on. That's good motivation to get this done. There's currently an android branch that's getting close to wrapped up. Any deadline on when you need this, Brett?

brettdonald commented 6 years ago

I need something today. And cross-platform support is more important to me than Unified, so I’ll be going with react-native-contacts. Your motivation should be to help the next developer who comes looking for a contacts library.

kafein commented 6 years ago

@joshuapinter I'm also waiting. I have about two weeks to finish my project. Is it possible?

Btw, Thanks for your efforts.

joshuapinter commented 6 years ago

@kafein You bet. Try installing the current android branch and see if you can get the basics working by following the instructions here: https://github.com/joshuapinter/react-native-unified-contacts/issues/16#issuecomment-273193105

So I can focus on the right stuff, what functions are you needing? e.g. Get all contacts. Lookup contact by email, Create contact, Delete contact, etc.

Eventually, we'll get everything in there but just figuring out where I should start so you can start using it in your project.

Thanks.

kafein commented 6 years ago

Thanks @joshuapinter

I'm going to try it. I'll report if i have problems.

Alver23 commented 6 years ago

Hi @joshuapinter, There is still no support for Android and ISO in the same branch?

joshuapinter commented 6 years ago

@Alver23 Check out the android branch.

Alver23 commented 6 years ago

@joshuapinter Would it work for ios and android, with branch #android?

joshuapinter commented 6 years ago

@Alver23 Yes. It adds support for Android on the existing iOS master branch. It's not complete but it does the basics.

Alver23 commented 6 years ago

@joshuapinter I understand, I can follow these instructions "https://github.com/joshuapinter/react-native-unified-contacts/issues/16#issuecomment-273193105" to work on both platforms

joshuapinter commented 6 years ago

@Alver23 That should work. Give it a try and post back here how it goes for you.

Alver23 commented 6 years ago

@joshuapinter Thanks

Alver23 commented 6 years ago

@joshuapinter I got this error when executing the command image

Alver23 commented 6 years ago

image

joshuapinter commented 6 years ago

@Alver23 Most likely needs to be updated to support the latest version of React Native that remove that method from the super class. If you want to make this update yourself and issue a Pull Request that would be great.

Otherwise, it'll have to wait until I get around to updating this myself.

Alver23 commented 6 years ago

@joshuapinter I do not have knowledge in java to make the correction. When you could solve it ?

joshuapinter commented 6 years ago

@Alver23 Probably not until later in March or maybe April, unfortunately.

I encourage you to try, though. If you're using React Native, learning the underlying languages of Java and Objective-C/Swift, is extremely helpful.

paintedbicycle commented 6 years ago

@joshuapinter What state is this in now? Where do you need it to be to merge it? What’s missing? Can you list out your ideal state for a PR so we can get this moving?

joshuapinter commented 6 years ago

@paintedbicycle Ideally we want feature parity (where applicable) with iOS with consistent interfaces between the two. We've been using the Android branch in beta for www.ntwrkapp.com and it's been working great but it's lacking features. Plus, both need to be updated to support the latest versions of iOS/Android/React Native.

paintedbicycle commented 6 years ago

@joshuapinter I'm willing to lend a hand, but since it's your repo it would be nice if you look the leadership position. In my opinion since this has been open for over a year, we need to get some momentum behind it by breaking it down into smaller pieces that people can chip away at. Perhaps you can open bug tickets on the Android branch for each thing you'd like to see with a short description? That way we can jump in an help. It would provide us with a roadmap and then we can have discussions on this thread on when it's ok to merge. Most competing libraries do not have full feature parity. I agree it's the goal, but at some point before then merging is likely possible. The opening of tickets and the attention on this branch will help us figure it all out.

paintedbicycle commented 6 years ago

An update on Android support.

Hello!

I've spent the past week or so trying to align the Android branch with the iOS branch and am starting to get there. My goal was to find out what state the Android branch was in and then to start using it as a unified branch where both iOS and Android apps could be run. I also wanted to get the ExampleApp updated. I have accomplished these things. Now that iOS and Android can be run from the same branch, we can start implementing each of the iOS methods in Android in order to achieve feature parity. We're not there yet (see list at the bottom) but this should be a lot easier now that the cobwebs have been cleaned off the branch.

I'm going to need help or this will move very slowly. If anyone out there can help with the Java side of things, I'd really appreciate it as I'm not a Java dev. Basically, we need to take a look at the iOS methods and recreate them all in Java. This has already begun - @joshuapinter got it started by implementing permissions and getContacts. But we need to continue. There really aren't that many and I think this is very achievable.

To anyone that was using the Android branch before (likely not too many people as it was a year old and not fully supported) - I've had to make a couple small (but breaking) changes to the Android files in order to get iOS and Android both working and returning the same data. (iOS remains exactly the same and will be used the guiding star in terms of API structure and naming).

Basically, before the iOS and the Android repos would both work separately if you installed them as two different apps, but if you tried to get them working at the same time with the same app or using ExampeApp, it did not work. This was because of API mismatches and the use of the JS Permissions API on Android in the index.js file. So, I have combined the repos on the Android branch (it's now as up-to-date as Master, but also has Android support) and I made some tweaks to get everything up and running.

The tweaks:

So, what this means is that anyone can now easily run the Android branch and get back the full iOS functionality of master, while also having very basic Android functionality. Just be careful as not all functions have been implemented in Android and calling them in your app will cause errors. In your app, you can use react-native "Platform" API to only run certain functions. For example, you can read and write contacts on iOS but only read on Android, so you might use Platform to not allow writing on Android for now.

Notes

HELP NEEDED

All of the below are for Android and in the Android branch

High Priority:

Medium Priority:

Lower priority (but still important):

Perhaps a good first PR for someone would be to add a compatability table to the readme in the Android branch that outlines the methods in this library and indicates if they are supported in iOS and/or Android. Perhaps a column for notes too. Notes could indicate if the format of the methods don't match or any other information.

In my opinion, for now the Android methods are ok to have breaking changes because Android was never released. What I mean is that we should do everything we can to align Android with the already up and running iOS API. So methods being renamed or their arguments and return values reformatted to match iOS is the priority. Any help is appreciated including testing, PRs or creating unit tests.

Thanks!

joshuapinter commented 6 years ago

Excellent work, @paintedbicycle!! Thank you for getting the android branch back up to speed and outlining everything that needs to be done to bring Android to a full parity release with iOS.

I'll be able to help with the high priority items as I'm quite familiar with Java and Android. Now that you have the ExampleApp working for both iOS and Android it should make it easier for other people to jump in and submit a PR as well.

One item I'd like to add to the To-Do list is a decent test suite. I'll take that on to kick off because it's a complicated decision and something that will affect long-term maintenance of the library. This will be critical to confidently accept PRs in the future and for keeping our sanity with our own development.

As per your permissions question, the inspiration of this library was built for the latest contacts framework by Apple that was released with iOS 9. There are other libraries that service the older versions of OS just fine. My vote is keeping this a more modern library. 9+ for iOS and 6+ for Android seem perfectly reasonable.

If others want to add support for older versions, they can submit a PR for us to consider.

My time is limited over the next couple of weeks with business travel and commitments but things should clear up after that.

However, I'm always available to answer questions and review Pull Requests, etc.

I'm excited about this moving forward and I want to thank you again @paintedbicycle for all of your hard work thus far. 🙏

paintedbicycle commented 6 years ago

Excellent @joshuapinter, thanks. Agreed, testing suite is important.

I've tried quite a few other react-native contacts libraries and while some of them have more complete Android support, there was always something missing from the API (either no postal addresses, or no ability to save contacts, not just read or no searching or the iOS stuff was messy). I think if we can get Android on feature parity with iOS in this repo it will be a very competitive react-native contacts repo and hopefully with that brings more attention and PRs from other devs adding new features.

I've now gone through and created open issues for most of the above with basic descriptions as I could think about them quickly. More thought will need to be put into place while developing, but hopefully that helps create a roadmap for people to follow.

Except for some smaller tweaks I'm going to step back for a bit now as I've done a big push and need to get back to my other work, but I'm still around and down to help. If we can get the high priority stuff addressed or even one or two more sets of eyes on the Android stuff I think we'll be in good shape.

Paul

joshuapinter commented 6 years ago

@paintedbicycle Excellent, thanks again for all your work on this! It'll help push things forward and get it in the spotlight a little more.

paintedbicycle commented 6 years ago

Ping @joshuapinter - are you able to get us over this next milestone? A bit of help getting permissions on android working properly might be all it takes. If you can help with a few hours here and there getting the high priority items done it will make a large difference.