Rating model implementation for third-party stores - #680
Conversation
|
Thanks so much for this, @ashb155! We'll try to review it soon :) |
angrezichatterbox
left a comment
angrezichatterbox
left a comment
There was a problem hiding this comment.
Hey @ashb155
Thanks for the PR.
Instead of the inbuild review flow fail leading to a toast could the user be instead be redirected to the playstore page. This would be better for the user experience.
|
will work on this @angrezichatterbox! |
|
Let us know when this is ready for further review, @ashb155 :) |
|
should be good to go @andrewtavis! i guess tests are passing now |
|
Great to hear, @ashb155! Let us know how this is looking, @angrezichatterbox :) |
angrezichatterbox
left a comment
angrezichatterbox
left a comment
There was a problem hiding this comment.
Thanks for the quick changes @ashb155. I have a small nitpick suggestion to add on once thats done this PR would be good to go.
|
hi @angrezichatterbox , just made the changes required, do let me know if its good to go |
|
@ashb155 GetStoreDesc returns hardcoded English strings instead of stringResource like the rest of AboutUtil.kt, and given Scribe's i18n focus, these should probably be proper string resources |
|
@Roniscend, as discussed with @andrewtavis in the related issue, plan is to hardcode them first and then get them over to scribe i18n later |
|
Thanks all for the conversation here 💙 @ashb155, do you want to open a PR in Scribe-i18n with the needed strings and we'll bring that in and then into this PR? I guess it would make sense if we finalize the strings with the merge here. |
|
sounds good @andrewtavis, will have the i18n PR ready as well in a bit |
angrezichatterbox
left a comment
angrezichatterbox
left a comment
There was a problem hiding this comment.
Thanks for the changes @ashb155
Everything looks good now : )
|
Thanks for the review here, @angrezichatterbox and @Roniscend! Let's wait for the Scribe-i18n PR and then bring the changes in here so we're finalizing the whole process in one PR. Good to know we can merge once the Scribe-i18n changes are brought in though! |
|
have linked the i18n PR @andrewtavis, do have a look and let me know if any changes! |
|
Hey @ashb155! Already has been merged! Feel free to update the PR here with the latest Scribe-i18n and we can merge! note: I updated your keys to be |
|
great to hear @andrewtavis, will keep naming conventions as consistent as it can be for any future changes |
|
looks like tests are passing @andrewtavis , do let me know if any more changes or if this is good to go! |
andrewtavis
left a comment
andrewtavis
left a comment
There was a problem hiding this comment.
praise: All's looking and working really well, @ashb155! Great that this is coming in so quickly! 😊 Looking forward to updating this with all the proper URLs and new stores post launch 🚀
| try { | ||
| context.startActivity(intent) | ||
| } catch (e: ActivityNotFoundException) { | ||
| Toast.makeText(context, "No browser found to open $storeName page", Toast.LENGTH_SHORT).show() |
There was a problem hiding this comment.
note: This Toast would also ideally be localized, but maybe we can do an audit of the toasts to see if there are others that aren't localized? Do you want to look into that, @ashb155?
There was a problem hiding this comment.
Will create an issue for this and work on complete audit and i18n integration @andrewtavis
There was a problem hiding this comment.
Great! Please reference this PR and ping me in there :) And feel free to assign yourself!
yea @andrewtavis , registering the app with the package name should be all we need right now since the links are generated dynamically, if anything changes, we still have the flexibility to update the configuration post-release |
4 participants
Contributor checklist
./gradlew lintKotlin detekt testcommand as directed in the testing section of the contributing guideDescription
This PR expands the "Rate Us" functionality on the About Page by dynamically detecting the installation source (added
PackageManager.getInstallSourceInfomethod for Android 11+ along with existinggetInstallerPackageName()) and rendering a contextual subtitle and intent link (Google Play, F-Droid, Amazon, and Galaxy Store).Previously, the rating item only showed a generic "Rate Scribe" and lacked support for alternative app stores like Amazon and Samsung. Now, it accurately reflects where the user downloaded the app and routes them there directly.
The external store URLs are constructed dynamically using the app's package name (e.g., https://f-droid.org/packages/${context.packageName}, https://www.amazon.com/gp/mas/dl/android?p=${context.packageName}) as dummy links, which can be modified later as required.
For other third party stores, we default to Google Play Store, which again, can be modified as required.
Related issue