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

Replace material-dialogs with AppCompat AlertDialog #32

Merged
merged 2 commits into from May 29, 2021
Merged

Replace material-dialogs with AppCompat AlertDialog #32

merged 2 commits into from May 29, 2021

Conversation

msfjarvis
Copy link
Contributor

@msfjarvis msfjarvis commented May 29, 2021

This PR removes the material-dialogs dependency in the library by switching its uses to the AppCompat AlertDialog.

This has multiple benefits:

  • Prevents classpath pollution for users of this library who do not use material-dialogs
  • Reduces APK size for the abovementioned users
  • Allows the dialogs shown by the library to follow the theming set up by the apps using it

Fixes #33

Screenshots

API 29:

screenshot-20210529-152938

@anggrayudi
Copy link
Owner

Great work! 👍
BTW, I have the following requests:

  1. Change the target branch to release/0.7.0
  2. Please put test results (screenshots) into the PR description on API 19, 21 and 29.

@msfjarvis msfjarvis changed the base branch from master to release/0.7.0 May 29, 2021 09:29
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis
Copy link
Contributor Author

The API 19 emulator was prohibitively slow on my machine so I could not manage to install the sample on it, and on API 21 the app crashed on launch immediately:

Stacktrace
AndroidRuntime  D  Shutting down VM
                E  FATAL EXCEPTION: main
                E  Process: com.anggrayudi.storage.sample, PID: 3513
                E  java.lang.RuntimeException: Unable to start activity ComponentInfo{com.anggrayudi.storage.sample/com.anggrayudi.storage.sample.activity.MainA
                   ctivity}: java.lang.NullPointerException: Attempt to invoke virtual method 'java.lang.String java.io.File.getPath()' on a null object referen
                   ce
                E      at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2298)
                E      at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2360)
                E      at android.app.ActivityThread.access$800(ActivityThread.java:144)
                E      at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1278)
                E      at android.os.Handler.dispatchMessage(Handler.java:102)
                E      at android.os.Looper.loop(Looper.java:135)
                E      at android.app.ActivityThread.main(ActivityThread.java:5221)
                E      at java.lang.reflect.Method.invoke(Native Method)
                E      at java.lang.reflect.Method.invoke(Method.java:372)
                E      at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:899)
                E      at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:694)
                E  Caused by: java.lang.NullPointerException: Attempt to invoke virtual method 'java.lang.String java.io.File.getPath()' on a null object refere
                   nce
                E      at com.anggrayudi.storage.file.DocumentFileCompat.getStorageIds(DocumentFileCompat.kt:457)
                E      at com.anggrayudi.storage.sample.StorageInfoAdapter.<init>(StorageInfoAdapter.kt:32)
                E      at com.anggrayudi.storage.sample.activity.MainActivity.onCreate(MainActivity.kt:65)
                E      at android.app.Activity.performCreate(Activity.java:5937)
                E      at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1105)
                E      at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2251)
                E      ... 10 more
                Process  I  Sending signal. PID: 3513 SIG: 9

I've attached the screenshot for API 29.

@anggrayudi
Copy link
Owner

Can you try to add filterNotNull() into DocumentFileCompat at line 456? Let's see whether it could fix the issue.

Screen Shot 2021-05-29 at 17 08 16

@msfjarvis
Copy link
Contributor Author

It does fix the crash, but I can't attach a screenshot for it since storage access is automatically granted on API 21 so the code path for the dialogs is never hit.

Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@anggrayudi anggrayudi merged commit 479ad11 into anggrayudi:release/0.7.0 May 29, 2021
@msfjarvis msfjarvis deleted the remove-material-dialogs branch May 29, 2021 10:48
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.

Remove material-dialogs from library dependencies
2 participants