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

Full overhaul of the repository #52

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

peterhav
Copy link

To the developer of this library,

I have been using this library in one of my Android applications and really appreciate it. Recently I have migrated all my sources to Android X and found this library to be the last dependency that was not yet using AndroidX.
After looking at the sources; I've decided to make several more improvements and ended up with the list below:

Updates

  • Material Components (removes material search view dependency that is no longer required)
  • Material styling/theming (end users only need to supply their colors.xml (custom_colors.xml in the sample) and themes.xml, if not provided default library colors are used
  • Kotlin coroutines/Flow instead of RxLibrary (removing dependency to io.reactivex.rxjava2)
  • Navigation (instead of Builder) mainly for safe arguments (end users can integrate the fragment using a navgraph)
  • Kotlin instead of Java
  • AndroidX (automatic migration, jetifier disabled since all dependencies use androidx)
  • Introduction of viewmodels (state changes and sharing of result)
  • Single Fragment (in line with the single activity recommendation and for easy integration)
  • Databinding for the UIs
  • Timber dependency added for logging
  • Permission requester integration
  • Hilt dependency injection is used for easy viewmodel injection (note that the sharedviewmodel works at activity level, which makes the picked contacts result communication
  • Strings are now arguments using resource identifiers n order to facilitate users to add customization and internationalization of any text used
  • Sample app added, that shows how the contactspicker can be used

I've tried to keep the original code and interface in place where possible. Happy for you to decide if you want to use this or not.
Kind regards,
Peter

Introduction Android Material Components
Migration to Kotlin
Migration to AndroidX
Android Architecture Navigation: mainly for Safe Arguments
Switch to Fragment & Fragment ViewModel instead of single Activity
Use of databinding
Read Contacts Permission requester integrated
Introduction Timber for logging
Hilt dependency injection (for viewmodel)
Sample App added to showcase the library
Configure strings added to arguments
Latest version of Android build tools and other dependencies
@broakenmedia
Copy link
Owner

Excellent stuff Peter... thank you. I sadly have not had the time to work on this anymore and my primary development platform is no longer Android which makes it even harder! I've invited you as a collaborator on the project, please feel free to integrate this changes as you see fit :)

@peterhav
Copy link
Author

peterhav commented Aug 29, 2020 via email

dest.writeTypedList(mPhoneNumbers)
}

companion object {
Copy link

Choose a reason for hiding this comment

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

You can use @parcelize to avoid creating all this manually

dest.writeString(number)
}

companion object {
Copy link

Choose a reason for hiding this comment

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

Same here

import kotlin.properties.ReadOnlyProperty
import kotlin.reflect.KProperty

fun <T> Fragment.viewLifecycleLazy(initialise: () -> T): ReadOnlyProperty<Fragment, T> = object : ReadOnlyProperty<Fragment, T>, DefaultLifecycleObserver {
Copy link

Choose a reason for hiding this comment

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

Typo in initialise

@@ -0,0 +1,67 @@
<?xml version="1.0" encoding="utf-8"?>
<navigation xmlns:android="http://schemas.android.com/apk/res/android"
Copy link

Choose a reason for hiding this comment

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

👍

Copy link

@ispam ispam 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, just minor comments, not really a blocker apart from the conflicts and should be G2G

@ispam ispam mentioned this pull request Feb 7, 2021
@oscarito9410
Copy link

Guys, I'm interested on contributing in this repo. @peterhav may I take this branch and solved conflicts?
CC: @broakenmedia

@broakenmedia
Copy link
Owner

Guys, I'm interested on contributing in this repo. @peterhav may I take this branch and solved conflicts?
CC: @broakenmedia

Hello, you're welcome to contribute if still interested? Thanks.

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