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

[Dependency Updates] Update coilComposeVersion to 2.2.2 #18326

Merged

Conversation

ParaskP7
Copy link
Contributor

@ParaskP7 ParaskP7 commented Apr 26, 2023

Parent #17563
Batch Branch: deps/main-batch-androidx-compose-kotlin

This PR updates coilComposeVersion to 2.2.2.


Note that the latest 2.3.0 update of coilComposeVersion is transitively updating Kotlin to 1.8.10 and as such will be done at a later point of time, after the Compose Complier is updated to target this newer Kotlin version.


PS: @ovitrif I added you as the main reviewer, but not so randomly (context), since I just wanted someone from the WordPress team to be aware of and sign-off on that change for WPAndroid. I also added the @wordpress-mobile/apps-infrastructure team, but this in done only for monitoring purposes, as such, I am not expecting any active review from that team. Thus, feel free to merge this PR if you deem so.


Fix List:

  1. Remove unsupported placeholders and errors from coil's async image

FYI: @ovitrif please take a close look at this commit's description and let me know your thoughts. I tried finding a solution to overcome this limitation but couldn't (I timeboxed my efforts). I then decided to remove both, the placeholder and error, to progress with this update and give you a heads-up on it in case you are able to think of a way forward. Below are a couple of my suggestion to move forward (post merging this PR):

  • We ask help from design to draw us a VectorDrawable alternative of bg_rectangle_placeholder_globe and when that is done, we'll create another PR to add this functionality back before merging the parent branch to trunk, or 🎨
  • We ignore this functionality altogether (for the time being), with the premise that it is just a nice to have but wouldn't break our flows, the UI would just show an empty image when the image can't be fetched from the API. 🤔

PS: The AsyncImage official documentation can be found here: https://coil-kt.github.io/coil/compose/#asyncimage

Let me know your thoughts? 🙏

Refactor List:

  1. Optimize imports on user avatar image

To test:

  1. See the dependency tree diff result and verify correctness.
  2. Thoroughly smoke test any Coil enhanced Compose related screen, on both, the WordPress and Jetpack apps, and see if everything is working as expected.
  3. In addition to the above smoke test, you can expand the below and follow the inner and more explicitly test steps within:
1. Jetpack Migration Flow [JetpackMigrationFragment.kt + SiteList.kt + UserAvatarImage.kt]

ℹ️ This test applies to the Jetpack app.

  • Install both apps.
  • Clear cache/data of the Jetpack app and restart it.
  • The migration flow should appear, verify the list of sites it is shown, including the user avatar
    and any site image, and that everything is functioning as expected.
2. Blaze Screen [BlazeOverlayFragment.kt]

ℹ️ This test applies to the Jetpack app.

  • Go to Posts (or Pages) screen.
  • Find a couple of posts from the list of posts, one with and without a featured image,
    click on the More options and then on its Promote with Blaze option.
  • Verify that the Blaze screen is shown, that it includes the post's (or page's) details,
    with or without a featured image (depending on the post you selected), that the details
    of this post (or page) are above the Blaze this Post (or Page) button, and that everything is
    functioning as expected.

Regression Notes

  1. Potential unintended areas of impact

    • Potential breakage or misbehaviour on any or all Coil enhanced Compose related screens, like the Jetpack Migration Flow screen and the Blaze green.
  2. What I did to test those areas of impact (or what existing automated tests I relied on)

    • See To test section above.
  3. What automated tests I added (or what prevented me from doing so)

    • N/A

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

UI Changes testing checklist:

  • Portrait and landscape orientations.
  • Light and dark modes.
  • Fonts: Larger, smaller and bold text.
  • High contrast.
  • Talkback.
  • Languages with large words or with letters/accents not frequently used in English.
  • Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
  • Large and small screen sizes. (Tablet and smaller phones)
  • Multi-tasking: Split screen and Pop-up view. (Android 10 or higher)

Release Notes: https://github.com/coil-kt/coil/releases/tag/2.0.0

------------------------------------------------------------------------

This is a major update and with it, it brings a basic API change, the
introduction of 'AsyncImage', see below:

https://github.com/coil-kt/coil/blob/main/CHANGELOG.md#200---may-10-2022

As such, this update is also migrating the old 'Image + painter' way of
doing things into the new Coil's 'AsyncImage + ImageRequest' API.
Unfortunately, Compose's 'Painter' and 'PainterResources' APIs do not
support '<layer-list/>', or anything other than a 'VectorDrawable' or a
rasterized asset type (PNG, JPG, etc).

As such, when one tried to do use '<layer-list/>' the below exception is
thrown:

------------------------------------------------------------------------
E  FATAL EXCEPTION: main
   Process: com.jetpack.android.beta, PID: 14827
   java.lang.IllegalArgumentException: Only VectorDrawables and
    rasterized asset types are supported ex. PNG, JPG
   	 at androidx.compose.ui.res.PainterResources_androidKt
   	  .loadVectorResource(PainterResources.android.kt:94)
   	 at androidx.compose.ui.res.PainterResources_androidKt
   	 .painterResource(PainterResources.android.kt:65)
------------------------------------------------------------------------

As such, in order to support placeholders and errors for 'AsyncImage',
both, the 'bg_rectangle_placeholder_globe_margin_8dp' and
'bg_rectangle_placeholder_globe_32dp' drawables, need to be converted to
a 'VectorDrawable' or a rasterized asset type.

In the meanwhile, those placeholders and errors are removed in order to
progress with this Coil update. Later on, with some design help, both,
the placeholders and errors can get added back.

------------------------------------------------------------------------

PS: I also tried to convert the '<layer-list/>' to a Compose image, but
this is not working with 'Painter' and 'PainterResources', which is the
API that 'AsyncImage' uses, see below:

Image(
    modifier = Modifier
        .padding(8.dp) // For bg_rectangle_placeholder_globe_margin_8dp
        .clip(shape = RectangleShape)
        .background(color = colorResource(id = R.color.placeholder)),
    painter = painterResource(id = R.drawable.ic_globe_white_24dp),
    contentDescription = "...",
)
@ParaskP7 ParaskP7 added this to the Future milestone Apr 26, 2023
@ParaskP7 ParaskP7 requested review from ovitrif and a team April 26, 2023 13:06
@ParaskP7 ParaskP7 self-assigned this Apr 26, 2023
@wpmobilebot
Copy link
Contributor

Found 1 violations:

The PR caused the following dependency changes:

 +--- project :libs:analytics
 |    +--- com.automattic:Automattic-Tracks-Android:2.2.0
 |    |    +--- io.sentry:sentry-android -> 5.4.3
 |    |    |    \--- io.sentry:sentry-android-core:5.4.3
 |    |    |         \--- androidx.core:core:1.3.2 -> 1.8.0
-|    |    |              +--- androidx.collection:collection:1.0.0 -> 1.1.0
+|    |    |              +--- androidx.collection:collection:1.0.0 -> 1.2.0
 |    |    |              \--- androidx.versionedparcelable:versionedparcelable:1.1.1
-|    |    |                   \--- androidx.collection:collection:1.0.0 -> 1.1.0 (*)
+|    |    |                   \--- androidx.collection:collection:1.0.0 -> 1.2.0 (*)
 |    |    +--- io.sentry:sentry-android-okhttp -> 5.4.3
 |    |    |    +--- com.squareup.okhttp3:okhttp-bom:4.9.2
-|    |    |    |    \--- com.squareup.okhttp3:okhttp:4.9.2 (c)
+|    |    |    |    \--- com.squareup.okhttp3:okhttp:4.9.2 -> 4.10.0 (c)
-|    |    |    \--- com.squareup.okhttp3:okhttp -> 4.9.2
-|    |    |         +--- com.squareup.okio:okio:2.8.0 -> 2.10.0
-|    |    |         |    +--- org.jetbrains.kotlin:kotlin-stdlib:1.4.20 -> 1.7.20 (*)
-|    |    |         |    \--- org.jetbrains.kotlin:kotlin-stdlib-common:1.4.20 -> 1.7.20
-|    |    |         \--- org.jetbrains.kotlin:kotlin-stdlib:1.4.10 -> 1.7.20 (*)
+|    |    |    \--- com.squareup.okhttp3:okhttp -> 4.10.0
+|    |    |         +--- com.squareup.okio:okio:3.0.0 -> 3.2.0
+|    |    |         |    \--- com.squareup.okio:okio-jvm:3.2.0
+|    |    |         |         +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.20 -> 1.7.20 (*)
+|    |    |         |         \--- org.jetbrains.kotlin:kotlin-stdlib-common:1.6.20 -> 1.7.20
+|    |    |         \--- org.jetbrains.kotlin:kotlin-stdlib:1.6.20 -> 1.7.20 (*)
-|    |    \--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.9.2 (*)
+|    |    \--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.10.0 (*)
 |    \--- androidx.preference:preference:1.2.0
 |         +--- androidx.appcompat:appcompat:1.1.0 -> 1.4.2
 |         |    +--- androidx.activity:activity:1.2.4 -> 1.5.1
-|         |    |    \--- androidx.collection:collection:1.0.0 -> 1.1.0 (*)
+|         |    |    \--- androidx.collection:collection:1.0.0 -> 1.2.0 (*)
 |         |    +--- androidx.fragment:fragment:1.3.6 -> 1.5.5
-|         |    |    +--- androidx.collection:collection:1.1.0 (*)
+|         |    |    +--- androidx.collection:collection:1.1.0 -> 1.2.0 (*)
 |         |    |    \--- androidx.viewpager:viewpager:1.0.0
 |         |    |         \--- androidx.customview:customview:1.0.0 -> 1.1.0
-|         |    |              \--- androidx.collection:collection:1.1.0 (*)
+|         |    |              \--- androidx.collection:collection:1.1.0 -> 1.2.0 (*)
 |         |    +--- androidx.appcompat:appcompat-resources:1.4.2
-|         |    |    +--- androidx.collection:collection:1.0.0 -> 1.1.0 (*)
+|         |    |    +--- androidx.collection:collection:1.0.0 -> 1.2.0 (*)
 |         |    |    +--- androidx.vectordrawable:vectordrawable:1.1.0
-|         |    |    |    \--- androidx.collection:collection:1.1.0 (*)
+|         |    |    |    \--- androidx.collection:collection:1.1.0 -> 1.2.0 (*)
 |         |    |    \--- androidx.vectordrawable:vectordrawable-animated:1.1.0
-|         |    |         \--- androidx.collection:collection:1.1.0 (*)
+|         |    |         \--- androidx.collection:collection:1.1.0 -> 1.2.0 (*)
 |         |    +--- androidx.emoji2:emoji2:1.0.0
-|         |    |    \--- androidx.collection:collection:1.1.0 (*)
+|         |    |    \--- androidx.collection:collection:1.1.0 -> 1.2.0 (*)
 |         |    +--- androidx.emoji2:emoji2-views-helper:1.0.0
-|         |    |    \--- androidx.collection:collection:1.1.0 (*)
+|         |    |    \--- androidx.collection:collection:1.1.0 -> 1.2.0 (*)
-|         |    \--- androidx.collection:collection:1.0.0 -> 1.1.0 (*)
+|         |    \--- androidx.collection:collection:1.0.0 -> 1.2.0 (*)
 |         +--- androidx.fragment:fragment-ktx:1.3.6 -> 1.5.5
 |         |    \--- androidx.collection:collection-ktx:1.1.0
-|         |         \--- androidx.collection:collection:1.1.0 (*)
+|         |         \--- androidx.collection:collection:1.1.0 -> 1.2.0 (*)
 |         +--- androidx.recyclerview:recyclerview:1.0.0 -> 1.2.1
-|         |    \--- androidx.collection:collection:1.0.0 -> 1.1.0 (*)
+|         |    \--- androidx.collection:collection:1.0.0 -> 1.2.0 (*)
 |         +--- androidx.slidingpanelayout:slidingpanelayout:1.2.0
 |         |    +--- androidx.window:window:1.0.0
-|         |    |    \--- androidx.collection:collection:1.1.0 (*)
+|         |    |    \--- androidx.collection:collection:1.1.0 -> 1.2.0 (*)
 |         |    \--- androidx.transition:transition:1.4.1
-|         |         \--- androidx.collection:collection:1.1.0 (*)
+|         |         \--- androidx.collection:collection:1.1.0 -> 1.2.0 (*)
-|         \--- androidx.collection:collection:1.0.0 -> 1.1.0 (*)
+|         \--- androidx.collection:collection:1.0.0 -> 1.2.0 (*)
 +--- project :libs:image-editor
 |    +--- androidx.viewpager2:viewpager2:1.0.0
-|    |    \--- androidx.collection:collection:1.1.0 (*)
+|    |    \--- androidx.collection:collection:1.1.0 -> 1.2.0 (*)
 |    +--- com.google.android.material:material:1.2.1 -> 1.6.0-alpha01
 |    |    +--- androidx.coordinatorlayout:coordinatorlayout:1.1.0
-|    |    |    \--- androidx.collection:collection:1.0.0 -> 1.1.0 (*)
+|    |    |    \--- androidx.collection:collection:1.0.0 -> 1.2.0 (*)
 |    |    \--- androidx.dynamicanimation:dynamicanimation:1.0.0
-|    |         \--- androidx.collection:collection:1.0.0 -> 1.1.0 (*)
+|    |         \--- androidx.collection:collection:1.0.0 -> 1.2.0 (*)
 |    \--- com.github.yalantis:ucrop:2.2.8
-|         \--- com.squareup.okhttp3:okhttp:3.12.13 -> 4.9.2 (*)
+|         \--- com.squareup.okhttp3:okhttp:3.12.13 -> 4.10.0 (*)
 +--- project :libs:editor
 |    +--- org.wordpress:aztec:{strictly v1.6.3} -> v1.6.3
 |    |    \--- androidx.legacy:legacy-support-v4:1.0.0
 |    |         \--- androidx.media:media:1.0.0 -> 1.2.1
-|    |              \--- androidx.collection:collection:1.1.0 (*)
+|    |              \--- androidx.collection:collection:1.1.0 -> 1.2.0 (*)
 |    +--- com.facebook.fresco:imagepipeline-okhttp3:2.5.0
-|    |    \--- com.squareup.okhttp3:okhttp:3.12.1 -> 4.9.2 (*)
+|    |    \--- com.squareup.okhttp3:okhttp:3.12.1 -> 4.10.0 (*)
 |    \--- org.wordpress-mobile.gutenberg-mobile:react-native-gutenberg-bridge:v1.94.0-alpha1
 |         +--- com.github.wordpress-mobile:react-native-video:5.2.0-wp-5
 |         |    +--- com.google.android.exoplayer:extension-okhttp:2.13.3
-|         |    |    \--- com.squareup.okhttp3:okhttp:3.12.11 -> 4.9.2 (*)
+|         |    |    \--- com.squareup.okhttp3:okhttp:3.12.11 -> 4.10.0 (*)
-|         |    \--- com.squareup.okhttp3:okhttp:${OKHTTP_VERSION} -> 4.9.2 (*)
+|         |    \--- com.squareup.okhttp3:okhttp:${OKHTTP_VERSION} -> 4.10.0 (*)
 |         \--- org.wordpress-mobile.react-native-libraries.v1:react-native-fast-image:8.5.11
 |              \--- com.github.bumptech.glide:okhttp3-integration:4.12.0
-|                   \--- com.squareup.okhttp3:okhttp:3.9.1 -> 4.9.2 (*)
+|                   \--- com.squareup.okhttp3:okhttp:3.9.1 -> 4.10.0 (*)
 +--- org.wordpress:fluxc:{strictly 2.25.0} -> 2.25.0
-|    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.9.2 (*)
+|    +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.10.0 (*)
 |    \--- com.squareup.okhttp3:okhttp-urlconnection:4.9.0 -> 4.9.2
-|         \--- com.squareup.okhttp3:okhttp:4.9.2 (*)
+|         \--- com.squareup.okhttp3:okhttp:4.9.2 -> 4.10.0 (*)
 +--- org.wordpress:login:1.3.0
 |    \--- com.google.android.gms:play-services-auth:18.1.0 -> 20.4.1
 |         +--- com.google.android.gms:play-services-auth-api-phone:18.0.1
 |         |    \--- com.google.android.gms:play-services-base:18.0.1 -> 18.1.0
-|         |         +--- androidx.collection:collection:1.0.0 -> 1.1.0 (*)
+|         |         +--- androidx.collection:collection:1.0.0 -> 1.2.0 (*)
 |         |         \--- com.google.android.gms:play-services-basement:18.1.0
-|         |              \--- androidx.collection:collection:1.0.0 -> 1.1.0 (*)
+|         |              \--- androidx.collection:collection:1.0.0 -> 1.2.0 (*)
 |         \--- com.google.android.gms:play-services-auth-base:18.0.4
-|              \--- androidx.collection:collection:1.0.0 -> 1.1.0 (*)
+|              \--- androidx.collection:collection:1.0.0 -> 1.2.0 (*)
 +--- com.automattic:about:1.1.0
 |    +--- androidx.compose.ui:ui:1.0.5 -> 1.3.3
-|    |    +--- androidx.collection:collection:1.0.0 -> 1.1.0 (*)
+|    |    +--- androidx.collection:collection:1.0.0 -> 1.2.0 (*)
 |    |    \--- androidx.compose.ui:ui-text:1.3.3
-|    |         \--- androidx.collection:collection:1.0.0 -> 1.1.0 (*)
+|    |         \--- androidx.collection:collection:1.0.0 -> 1.2.0 (*)
-|    \--- com.google.accompanist:accompanist-drawablepainter:0.20.2
-|         +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.5.31 -> 1.7.20 (*)
-|         \--- androidx.compose.ui:ui:1.0.5 -> 1.3.3 (*)
+|    \--- com.google.accompanist:accompanist-drawablepainter:0.20.2 -> 0.25.1
+|         \--- androidx.compose.ui:ui:1.2.1 -> 1.3.3 (*)
 +--- com.google.firebase:firebase-messaging:21.1.0
-|    +--- androidx.collection:collection:1.0.0 -> 1.1.0 (*)
+|    +--- androidx.collection:collection:1.0.0 -> 1.2.0 (*)
 |    \--- com.google.firebase:firebase-iid:21.1.0
-|         \--- androidx.collection:collection:1.0.0 -> 1.1.0 (*)
+|         \--- androidx.collection:collection:1.0.0 -> 1.2.0 (*)
-+--- com.squareup.okio:okio:2.8.0 -> 2.10.0 (*)
++--- com.squareup.okio:okio:2.8.0 -> 3.2.0 (*)
 +--- com.squareup.retrofit2:retrofit:2.9.0
-|    \--- com.squareup.okhttp3:okhttp:3.14.9 -> 4.9.2 (*)
+|    \--- com.squareup.okhttp3:okhttp:3.14.9 -> 4.10.0 (*)
 +--- com.airbnb.android:lottie:5.2.0
-|    \--- com.squareup.okio:okio:1.17.4 -> 2.10.0 (*)
+|    \--- com.squareup.okio:okio:1.17.4 -> 3.2.0 (*)
 +--- com.zendesk:support:5.1.1
 |    +--- com.zendesk:support-providers:5.1.1
 |    |    \--- com.zendesk:core:4.0.9
 |    |         +--- com.squareup.okhttp3:logging-interceptor:4.9.2
-|    |         |    \--- com.squareup.okhttp3:okhttp:4.9.2 (*)
+|    |         |    \--- com.squareup.okhttp3:okhttp:4.9.2 -> 4.10.0 (*)
-|    |         \--- com.squareup.okhttp3:okhttp:4.9.2 (*)
+|    |         \--- com.squareup.okhttp3:okhttp:4.9.2 -> 4.10.0 (*)
 |    \--- com.zendesk:guide:1.0.9
 |         +--- com.zendesk:messaging:5.2.5
 |         |    \--- com.zendesk:common-ui:4.0.5
 |         |         \--- com.zendesk.belvedere2:belvedere:3.0.5
 |         |              \--- com.squareup.picasso:picasso:2.8
-|         |                   \--- com.squareup.okhttp3:okhttp:3.10.0 -> 4.9.2 (*)
+|         |                   \--- com.squareup.okhttp3:okhttp:3.10.0 -> 4.10.0 (*)
-|         \--- com.squareup.okhttp3:okhttp:4.9.2 (*)
+|         \--- com.squareup.okhttp3:okhttp:4.9.2 -> 4.10.0 (*)
-\--- io.coil-kt:coil-compose:1.4.0
-     +--- io.coil-kt:coil:1.4.0
-     |    \--- io.coil-kt:coil-base:1.4.0
-     |         +--- org.jetbrains.kotlin:kotlin-parcelize-runtime:1.5.30 -> 1.7.20 (*)
-     |         +--- org.jetbrains.kotlin:kotlin-stdlib:1.5.30 -> 1.7.20 (*)
-     |         +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.5.2 -> 1.6.4 (*)
-     |         +--- androidx.lifecycle:lifecycle-common-java8:2.3.1 -> 2.5.1 (*)
-     |         +--- com.squareup.okhttp3:okhttp:3.12.13 -> 4.9.2 (*)
-     |         +--- com.squareup.okio:okio:2.10.0 (*)
-     |         +--- androidx.annotation:annotation:1.2.0 -> 1.5.0 (*)
-     |         +--- androidx.appcompat:appcompat-resources:1.3.1 -> 1.4.2 (*)
-     |         +--- androidx.collection:collection-ktx:1.1.0 (*)
-     |         +--- androidx.core:core-ktx:1.6.0 -> 1.8.0 (*)
-     |         +--- androidx.exifinterface:exifinterface:1.3.3 (*)
-     |         \--- androidx.lifecycle:lifecycle-runtime:2.3.1 -> 2.5.1 (*)
-     +--- io.coil-kt:coil-compose-base:1.4.0
-     |    +--- io.coil-kt:coil-base:1.4.0 (*)
-     |    +--- androidx.compose.ui:ui:1.0.3 -> 1.3.3 (*)
-     |    +--- androidx.core:core-ktx:1.6.0 -> 1.8.0 (*)
-     |    \--- com.google.accompanist:accompanist-drawablepainter:0.19.0 -> 0.20.2 (*)
-     +--- androidx.compose.ui:ui:1.0.3 -> 1.3.3 (*)
-     \--- androidx.core:core-ktx:1.6.0 -> 1.8.0 (*)
+\--- io.coil-kt:coil-compose:2.2.2
+     +--- io.coil-kt:coil-compose-base:2.2.2
+     |    +--- io.coil-kt:coil-base:2.2.2
+     |    |    +--- androidx.lifecycle:lifecycle-runtime:2.4.1 -> 2.5.1 (*)
+     |    |    +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.6.4 (*)
+     |    |    +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.7.10 -> 1.7.20 (*)
+     |    |    +--- com.squareup.okhttp3:okhttp:4.10.0 (*)
+     |    |    +--- com.squareup.okio:okio:3.2.0 (*)
+     |    |    +--- androidx.annotation:annotation:1.5.0 (*)
+     |    |    +--- androidx.appcompat:appcompat-resources:1.4.2 (*)
+     |    |    +--- androidx.collection:collection:1.2.0 (*)
+     |    |    +--- androidx.core:core-ktx:1.8.0 (*)
+     |    |    \--- androidx.exifinterface:exifinterface:1.3.3 (*)
+     |    +--- androidx.compose.foundation:foundation:1.2.1 -> 1.3.1 (*)
+     |    +--- androidx.core:core-ktx:1.8.0 (*)
+     |    \--- com.google.accompanist:accompanist-drawablepainter:0.25.1 (*)
+     \--- io.coil-kt:coil:2.2.2
+          \--- io.coil-kt:coil-base:2.2.2 (*)

Please review and act accordingly

@wpmobilebot
Copy link
Contributor

Jetpack📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack Jetpack
FlavorJalapeno
Build TypeDebug
Versionpr18326-93bd640
Commit93bd640
Direct Downloadjetpack-prototype-build-pr18326-93bd640.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

WordPress📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress WordPress
FlavorJalapeno
Build TypeDebug
Versionpr18326-93bd640
Commit93bd640
Direct Downloadwordpress-prototype-build-pr18326-93bd640.apk
Note: Google Login is not supported on these builds.

Copy link
Contributor

@ovitrif ovitrif left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @ParaskP7 👋🏻

Overal LGTM, top work on this, thanks a lot for finally making it possible to use AsyncImage and forget the dark past ❤️

I have a remark and I also have a code that might help with it…
Let me know how you think it's best to proceed.

I have local code to quickly add another placeholder for the sites without icons in the migration flow. It will look like this:

Previews

🌞 🌚

@ovitrif
Copy link
Contributor

ovitrif commented May 2, 2023

@ParaskP7 I pushed my work here: https://github.com/wordpress-mobile/WordPress-Android/compare/deps/update-coil-to-2.2.2-suggestion-1

Feel free to cherry pick the last commit of that branch and then merge this PR 🚀 :shipit:

@ovitrif
Copy link
Contributor

ovitrif commented May 2, 2023

@ParaskP7 I pushed my work here: https://github.com/wordpress-mobile/WordPress-Android/compare/deps/update-coil-to-2.2.2-suggestion-1

Feel free to cherry pick the last commit of that branch and then merge this PR 🚀 :shipit:

See this PR to make this easier:

@ParaskP7
Copy link
Contributor Author

ParaskP7 commented May 2, 2023

👋 @ovitrif and thanks so much for the review, testing and for actually providing a new set of icons!

Overal LGTM, top work on this, thanks a lot for finally making it possible to use AsyncImage and forget the dark past ❤️

...and into a bright future! ❤️

I have a remark and I also have a code that might help with it…
Let me know how you think it's best to proceed.

👍

I have local code to quickly add another icon for the sites without icons in the migration flow. It will look like this:

@ParaskP7 I pushed my work here: https://github.com/wordpress-mobile/WordPress-Android/compare/deps/update-coil-to-2.2.2-suggestion-1

Feel free to cherry pick the last commit of that branch and then merge this PR 🚀 :shipit:

Awesome, let me cherry-pick your change, apply it and it its magic work, then I'll push it before merging, thanks a ton Ovi! 🙇 ❤️ 🚀

@ParaskP7
Copy link
Contributor Author

ParaskP7 commented May 2, 2023

Works like a charm @ovitrif and I agree that there is no need for placeholder, having just an error and only for SiteIcon and UserAvatarImage should be enough (for now at least)... And again, thanks for wrangling the icons Ovi, you rock, ready for merge! 🥇

@ovitrif ovitrif merged commit b17a3c9 into deps/main-batch-androidx-compose-kotlin May 2, 2023
@ovitrif ovitrif deleted the deps/update-coil-to-2.2.2 branch May 2, 2023 12:15
@ovitrif
Copy link
Contributor

ovitrif commented May 2, 2023

Works like a charm @ovitrif and I agree that there is no need for placeholder, having just an error and only for SiteIcon and UserAvatarImage should be enough (for now at least)... And again, thanks for wrangling the icons Ovi, you rock, ready for merge! 🥇

Awesome! Thanks for double-checking ❤️

Ps: I merged deliberately before waiting for the checks, those will run hundreds of times on the build-up PRs of the main branch 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants