britannio / in_app_review

A Flutter plugin for showing the In-App Review/System Rating pop up on Android, IOS, and MacOS. It makes it easy for users to rate your app.
294 stars 76 forks source link

Update README.md to add Android setup #100

Open mathrocco opened 10 months ago

mathrocco commented 10 months ago

Add Android setup instructions to solve issues for package users https://docs.flutter.dev/add-to-app/android/plugin-setup#b-plugins-needing-project-edits

britannio commented 10 months ago

Hey, thanks for the PR. If this step can be handled by the package then it'd save some time for developers. Is this line not already sufficient? https://github.com/britannio/in_app_review/blob/master/in_app_review/android/build.gradle#L48

mathrocco commented 10 months ago

Hey, thanks for the PR. If this step can be handled by the package then it'd save some time for developers. Is this line not already sufficient? https://github.com/britannio/in_app_review/blob/master/in_app_review/android/build.gradle#L48

Thanks for the fast response @britannio. These changes need to be done in the app build.gradle file, otherwise the dependency won't be imported, like it says here: https://docs.flutter.dev/add-to-app/android/plugin-setup#b-plugins-needing-project-edits

Btw, I think this Gradle file you mentioned isn't even being used by the package, so I suggest trying to remove it https://docs.flutter.dev/packages-and-plugins/developing-packages

mathrocco commented 10 months ago

@britannio about making setup easier, I noticed that Firebase for instance uses its CLI to help with that. But they have a wide variety of packages. In the case of one single package, I think it may be easier to perform it manually instead. Take a look on https://pub.dev/packages/espresso for instance

britannio commented 10 months ago

Btw, I think this Gradle file you mentioned isn't even being used by the package, so I suggest trying to remove it https://docs.flutter.dev/packages-and-plugins/developing-packages

It has an example of firebase_crashlytics that points to this line: https://github.com/firebase/flutterfire/blob/bdb95fcacf7cf077d162d2f267eee54a8b0be3bc/packages/firebase_crashlytics/android/build.gradle#L40

Then again, crashlytics also requires users to modify android/build.gradle but with a separate dependency.

britannio commented 10 months ago

Now I'm curious as to why build.gradle must be modified for Crashlytics but not other Firebase libraries.

https://firebase.google.com/docs/auth/android/start#add_to_your_app

mathrocco commented 10 months ago

Now I'm curious as to why build.gradle must be modified for Crashlytics but not other Firebase libraries.

https://firebase.google.com/docs/auth/android/start#add_to_your_app

There's other plugin from firebase that requires these changes, but according to this doc this is handled by the flutterfire CLI

Screenshot 2023-09-15 at 17 25 00
britannio commented 10 months ago

There's other plugin from firebase that requires these changes, but according to this doc this is handled by the flutterfire CLI

Screenshot 2023-09-15 at 17 25 00

But why not Firebase auth? Developers of a native app need to add it but unless I'm mistaken, it isn't automatically added by the flutterfire cli.

I had a look at similar packages for other hybrid app frameworks and while they could be making the same mistake, the main difference is the additional dependency on com.google.android.gms:play-services-base so maybe this is the fix?

https://github.com/expo/expo/blob/ec9fe241beb15ca1bf923f9db9cab733b66549b6/packages/expo-store-review/android/build.gradle#L89-L91

https://github.com/MinaSamir11/react-native-in-app-review/blob/master/android/build.gradle#L74-L75

davidmartos96 commented 10 months ago

@mathrocco The link you posted is documentation about app-to-app configurations, which is when you have an existing Android native project and you want to use Flutter for a specific part in the project.

Normally the only required changes in the android configuration outside the plugin are the Gradle plugins, not Gradle dependencies, which are automatically included.

In the case of crashlytics they have a Gradle plugin and a Gradle dependency. When using the package, the user only needs to include the Gradle plugin explicitly.

I don't think this change is needed at all, although I might be mistaken. Just wanted to share my take on what the link refers to 🙂

Screenshot_2023-09-16-19-48-45-62_40deb401b9ffe8e1df2f1cc5ba480b12.jpg

Screenshot_2023-09-16-19-48-29-60_40deb401b9ffe8e1df2f1cc5ba480b12.jpg

britannio commented 10 months ago

I have added com.google.android.gms:play-services-base as a gradle dependency because the in-app-review packages for other hybrid frameworks do the same. Hopefully, this fixes the issue @mathrocco was facing. If not, it should not break anything.

britannio commented 10 months ago

I have added com.google.android.gms:play-services-base as a gradle dependency because the in-app-review packages for other hybrid frameworks do the same. Hopefully, this fixes the issue @mathrocco was facing. If not, it should not break anything.

Based on https://github.com/britannio/in_app_review/commit/d258bcba8f2f1e453d52c71451ec9f84b327fb37#commitcomment-127538410, the additional dependency is unlikely to be the fix.

britannio commented 10 months ago

@mathrocco Maybe you could test out the package again without your change and then with it afterwards to rule out any other effect. There's actually a long list of reasons why requestReview() may not work on Android: https://developer.android.com/guide/playcore/in-app-review/test#troubleshooting.

mathrocco commented 10 months ago

@mathrocco Maybe you could test out the package again without your change and then with it afterwards to rule out any other effect. There's actually a long list of reasons why requestReview() may not work on Android: https://developer.android.com/guide/playcore/in-app-review/test#troubleshooting.

@britannio I have tested these changes I proposed and it only worked after I added the dependencies. I did double check it re-installing a version without build.gradle changes after I made it work and the in-app-review banner didn't appear anymore.

I can test it again with this change you've introduced here to see if this is going to work and let you know.

Also, later next week I'll be able to see the real impact in production of my changes and I let you know 👍🏼

MattyBoy4444 commented 7 months ago

@mathrocco what did you find out?

murock commented 5 months ago

I'm having possibly related issues with a dependency mismatch, have you guys seen this one before?

Duplicate class com.google.android.play.core.review.ReviewException found in modules jetified-core-1.10.3-runtime (com.google.android.play:core:1.10.3) and jetified-review-2.0.1-runtime (com.google.android.play:review:2.0.1)

MattyBoy4444 commented 3 months ago

Any update on this? Is this still needed?