-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Enable crosswiki notifications and minor UI fixes in displaying notif… #1540
Conversation
8256e15
to
fea2288
Compare
Codecov Report
@@ Coverage Diff @@
## master #1540 +/- ##
=========================================
- Coverage 3.13% 3.11% -0.02%
=========================================
Files 136 140 +4
Lines 7397 7437 +40
Branches 713 714 +1
=========================================
Hits 232 232
- Misses 7150 7190 +40
Partials 15 15
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for submitting this PR @maskaravivek , this enhancement would be very useful indeed! Happy to merge pending successful tests and separation of Gradle changes.
android.useDeprecatedNdk=true | ||
BUTTERKNIFE_VERSION=8.6.0 | ||
org.gradle.jvmargs=-Xmx1536M | ||
buildToolsVersion=26.0.2 | ||
targetSdkVersion=25 | ||
buildToolsVersion=27.0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to change this in this PR? Might be worth putting Gradle changes in a separate one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gradle changes were unfortunately required as the new library Glide
was having dependency conflicts with
com.android.support:design:26.0.2
The only way I was able to fix it was by upgrading the build tools version.
Moreover, adding Glide
was necessary for handling .svg
images(notification icons on commons are hosted in .svg
). The other image library, Fresco
that the app already uses, can't handle .svg
images.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah okay, thanks for the clarification!
Works for me @maskaravivek thanks! |
This PR enables crosswiki notifications so that some of the wikidata and wikipedia notifications(currently thank you edits) can be seen in the app.
Moreover, it does quite a few UI fixes:
Notifications page now looks much better :)