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

Android: cleanup implementation (remove deprecated async task, share code between old/new architecture, prevent crashes on curson management) #752

Closed
EnricoMazzu opened this issue Sep 26, 2024 · 5 comments

Comments

@EnricoMazzu
Copy link
Contributor

EnricoMazzu commented Sep 26, 2024

Step of rewark:

  1. Remove usage of deprecated AsyncTask
  2. add check to prevent crashes in cursor read (invalid indexing or other similar condition)
  3. Check if create a Impl class for prevent code duplication between old and new architecture

NB: work on implementation , not on apis contract ->No breaking changes

I will submit a pull request in the next days

@EnricoMazzu EnricoMazzu changed the title Android Implementation: Remove usage of Async Task, add checks to prevent crashes on cursor read (ex invalid indexes) Android: cleanup implementation (remove deprecated async task, share code between old/new architecture, prevent crashes on curson managenet Sep 26, 2024
@EnricoMazzu EnricoMazzu changed the title Android: cleanup implementation (remove deprecated async task, share code between old/new architecture, prevent crashes on curson managenet Android: cleanup implementation (remove deprecated async task, share code between old/new architecture, prevent crashes on curson management) Sep 26, 2024
@EnricoMazzu
Copy link
Contributor Author

EnricoMazzu commented Oct 21, 2024

@morenoh149 : in my refactoring I've seen that on android side there is a partial management of Android permission.
I can complete this work and make contacts plugin not dependent from external permission package also in android.
Is there some specific reason for depend on external android permission package for this check?

@morenoh149
Copy link
Owner

@EnricoMazzu coud you share a link to the lines in question? If we can remove a dependency that would be a win

morenoh149 added a commit that referenced this issue Oct 24, 2024
@EnricoMazzu
Copy link
Contributor Author

EnricoMazzu commented Oct 26, 2024

Hi @morenoh149

[ContactsManager.java] (https://github.com/morenoh149/react-native-contacts/blob/v8.0.3/android/src/newarch/com/rt2zz/reactnativecontacts/ContactsManager.java)

methods:
void requestReadContactsPermission()
protected static void onRequestPermissionsResult(...)

I can work to handle permission directly in the module code (without delegate this task for android to the app and AndroidPermission module)

@morenoh149
Copy link
Owner

@EnricoMazzu sounds good

Copy link

This issue is stale, please provide more information about the status

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants