Convert Plaid to use AndroidX #524
Conversation
| android:layout_height="match_parent" | ||
| android:clipToPadding="false" | ||
| tools:context=".ui.HomeActivity"> | ||
| <androidx.drawerlayout.widget.DrawerLayout xmlns:android="http://schemas.android.com/apk/res/android" |
There was a problem hiding this comment.
Can we revert this weird formatting?
There was a problem hiding this comment.
Not sure why GitHub isn't showing the updated whitespace changes. I'll check this again before merging.
build.gradle
Outdated
| 'ktlint' : '0.24.0', | ||
| 'lifecycle' : '1.1.1', | ||
| 'legacy_support' : '1.0.0', |
There was a problem hiding this comment.
Split into a few different versions - androidx, legacyCoreUtils, and material. androidx feels generic though - suggestions for something better?
build.gradle
Outdated
| 'mockito' : '2.23.0', | ||
| 'mockito_kotlin' : '2.0.0-RC3', | ||
| 'okhttp' : '3.10.0', | ||
| 'retrofit' : '2.4.0', | ||
| 'retrofitCoroutines': '0.9.2', | ||
| 'room' : '1.1.1', | ||
| 'room' : '2.1.0-alpha01', | ||
| 'supportLibrary' : '27.1.1', |
core/build.gradle
Outdated
| api "com.android.support:support-dynamic-animation:${versions.supportLibrary}" | ||
| api "androidx.constraintlayout:constraintlayout:${versions.constraintLayout}" | ||
| api "androidx.browser:browser:${versions.legacy_support}" | ||
| api "com.google.android.material:material:${versions.legacy_support}" |
There was a problem hiding this comment.
Move down to maintain alphabetic order?
There was a problem hiding this comment.
Moved it down, though I wanted to avoid alphabetizing the list to limit the PR size.
| @@ -17,8 +17,10 @@ | |||
|
|
|||
| package io.plaidapp.core.ui.recyclerview; | |||
|
|
|||
| import androidx.recyclerview.widget.RecyclerView; | |||
There was a problem hiding this comment.
Shouldn't we avoid imports just for doc links?
designernews/build.gradle
Outdated
|
|
||
| } | ||
|
|
||
| dataBinding { | ||
| enabled true | ||
| } | ||
|
|
||
| dependencies { |
There was a problem hiding this comment.
Shouldn't this be in the other deps block below?
| distributionBase=GRADLE_USER_HOME | ||
| distributionPath=wrapper/dists | ||
| zipStoreBase=GRADLE_USER_HOME | ||
| zipStorePath=wrapper/dists | ||
| distributionUrl=https\://services.gradle.org/distributions/gradle-4.9-bin.zip | ||
| distributionUrl=https\://services.gradle.org/distributions/gradle-4.10.1-all.zip |
There was a problem hiding this comment.
nit: can we stick to the bin distribution?
| @@ -62,20 +61,20 @@ internal class AboutViewModel @Inject constructor( | |||
| ) | |||
|
|
|||
| val about1 = SpannableString(resources.getString(R.string.about_plaid_1)) | |||
| about1 += AlignmentSpan.Standard(Layout.Alignment.ALIGN_CENTER) | |||
| // about1 += AlignmentSpan.Standard(Layout.Alignment.ALIGN_CENTER) | |||
There was a problem hiding this comment.
looks like the plus overload was removed in 1.0.0.
If I remember correctly, they removed it because of a low API usability. you couldn't tell what that plus would really do.
We should create an extension function to Spannable, doing the functionality that used to be here
There was a problem hiding this comment.
Ported over plusAssign extension function from android-ktx
|
What's missing to remove the [WIP] label? Looks like things are failing due to CI issues with instrumented tests but the code seems to be all good. |
tiembo
changed the title
|
@florina-muntenescu fixed the instrumentation tests and removed [WIP] from the title. Ready for final review; feel free to merge if everything looks ok. |
No description provided.