-
Notifications
You must be signed in to change notification settings - Fork 663
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
Upgrade to compose 1.7 #9894
base: master
Are you sure you want to change the base?
Upgrade to compose 1.7 #9894
Conversation
3 builds increased size
Stripe Identity Example 21.5.1-theme1 (20)
|
Item | Install Size Change | Download Size Change |
---|---|---|
🗑 android.support.v4.media.session.MediaSessionCompat | ⬇️ -50.8 kB | ⬇️ -24.4 kB |
kotlin.sequences.SequencesKt__SequencesJVMKt | ⬆️ 42.0 kB | ⬆️ 20.2 kB |
📝 androidx.compose.ui.focus.FocusInteropUtils | ⬆️ 27.5 kB | ⬆️ 13.2 kB |
kotlin.collections.SetsKt | ⬆️ 26.2 kB | ⬆️ 12.6 kB |
📝 androidx.recyclerview.widget.ChildHelper | ⬆️ 25.1 kB | ⬆️ 12.0 kB |
Financial Connections Example 21.5.1 (210501)
com.stripe.android.financialconnections.example
⚖️ Compare build
⏱️ Analyze build performance
Total install size change: ⬆️ 246.3 kB (2.26%)
Total download size change: ⬆️ 115.1 kB (2.07%)
Largest size changes
Item | Install Size Change | Download Size Change |
---|---|---|
📝 androidx.fragment.app.FragmentContainer | ⬆️ 89.2 kB | ⬆️ 42.0 kB |
📝 androidx.compose.ui.util.ListUtilsKt | ⬆️ 72.5 kB | ⬆️ 34.1 kB |
📝 androidx.fragment.app.SpecialEffectsController$Operation$State$Co... | ⬆️ 54.2 kB | ⬆️ 25.5 kB |
🗑 com.stripe.android.financialconnections.features.error.ErrorScree... | ⬇️ -53.1 kB | ⬇️ -25.0 kB |
🗑 com.stripe.android.paymentsheet.state.PaymentElementLoaderKt | ⬇️ -51.5 kB | ⬇️ -24.3 kB |
PaymentSheet Example 21.5.1 (11)
com.stripe.android.paymentsheet.example
⚖️ Compare build
⏱️ Analyze build performance
Total install size change: ⬆️ 9.0 kB (0.06%)
Total download size change: ⬆️ 7.3 kB (0.09%)
Largest size changes
Item | Install Size Change | Download Size Change |
---|---|---|
🗑 androidx.compose.material.icons.filled.CloseKt | ⬇️ -67.3 kB | ⬇️ -30.5 kB |
📝 com.google.android.gms.common.api.internal.TaskUtil | ⬆️ 63.7 kB | ⬆️ 28.8 kB |
📝 androidx.compose.ui.text.android.TextLayoutGetRangeForRectExtensi... | ⬆️ 61.1 kB | ⬆️ 27.6 kB |
🗑 androidx.compose.material.icons.filled.ClearKt | ⬇️ -42.1 kB | ⬇️ -19.0 kB |
🗑 com.stripe.android.stripecardscan.camera | ⬇️ -40.1 kB | ⬇️ -18.1 kB |
🛸 Powered by Emerge Tools
Comment trigger: Size diff threshold of 100.00kB exceeded
Diffuse output:
APK
DEX
ARSC
|
bdba073
to
70eb92a
Compare
4fe9ae2
to
3a35e9d
Compare
77361ee
to
bd8f43c
Compare
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.
This is bad! Need to figure out how to fix!
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.
Same!
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.
Cursors went missing in these screenshots, but still exist in production!
bd8f43c
to
d427efa
Compare
@@ -1,5 +1,5 @@ | |||
ext.versions = [ | |||
accompanist : '0.34.0', | |||
accompanist : '0.36.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.
Should be 0.37.2
@@ -374,6 +375,7 @@ internal class PaymentMethodEndToEndTest { | |||
.isEqualTo(PaymentMethod.Type.WeChatPay) | |||
} | |||
|
|||
@Ignore |
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.
Please revert this change
@@ -110,6 +111,13 @@ internal class NavigationHandler<T : Any>( | |||
} | |||
|
|||
private fun navigateWithDelay(action: () -> Unit) { | |||
if (Build.FINGERPRINT == "robolectric") { |
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.
This seems like something we shouldn't do. Please revert this.
@@ -330,7 +332,7 @@ private fun TitleContent(title: String, subtitle: String?, isEnabled: Boolean, c | |||
if (contentDescription != null) { | |||
this.contentDescription = contentDescription | |||
} | |||
} | |||
}.focusable(), |
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.
Why does this need to be focusable?
.padding(4.dp) | ||
.fillMaxHeight() | ||
) { | ||
Text( | ||
stringResource(id = R.string.stripe_view_more), | ||
modifier = Modifier | ||
.clickable { |
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.
Why are we adding a second click listener here? The one on the row should be enough.
@@ -68,6 +69,7 @@ internal fun SavedPaymentMethodRowButton( | |||
promoText = null, | |||
onClick = onClick, | |||
modifier = modifier | |||
.focusable() |
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.
I think this is duplicated, given we also set it where we pass the modifier to.
@@ -38,10 +38,19 @@ internal data class CardPaymentMethodDetails( | |||
|
|||
internal data class UsBankPaymentMethodDetails( | |||
override val id: String, | |||
val last4: String = "6789", |
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.
Why is this change in this PR?
@@ -74,14 +74,15 @@ internal class ManageActivityTest { | |||
@Test | |||
fun `selecting a payment method returns updated selection`() = launch { | |||
managePage.assertLpmIsNotSelected(PaymentMethodFixtures.CARD_ID) | |||
managePage.selectPaymentMethod(PaymentMethodFixtures.CARD_ID) | |||
managePage.selectPaymentMethodWithLast4("4242") |
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.
These changes should be reverted.
@@ -318,13 +321,13 @@ internal class CustomerSessionPaymentSheetActivityTest { | |||
verticalModePage.assertPrimaryButton(isEnabled()) | |||
|
|||
verticalModePage.clickViewMore() | |||
managePage.waitUntilVisible() | |||
managePage.waitUntilVisible(customLast4 = cards[0].card!!.last4) |
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.
Please revert these changes.
import com.stripe.android.uicore.elements.DROPDOWN_MENU_CLICKABLE_TEST_TAG | ||
import com.stripe.android.uicore.elements.TEST_TAG_DROP_DOWN_CHOICE | ||
|
||
internal class EditPage( | ||
private val composeTestRule: ComposeTestRule | ||
) { | ||
fun waitUntilVisible() { | ||
// composeTestRule.waitUntil { |
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.
Please revert these changes.
} | ||
// composeTestRule | ||
// .onNodeWithTag(UPDATE_PM_SCREEN_TEST_TAG) | ||
// .assertExists() |
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.
Please revert these changes.
@@ -98,12 +107,14 @@ internal class EditPage( | |||
} | |||
|
|||
fun onRemoveButton(): SemanticsNodeInteraction { | |||
return composeTestRule.onNodeWithTag(UPDATE_PM_REMOVE_BUTTON_TEST_TAG) | |||
return composeTestRule.onNodeWithText("Remove") // .performClick() |
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.
Please revert these changes.
b8b64ee
to
bbd2310
Compare
Summary
I found a workaround for our upgrade issue here: https://issuetracker.google.com/issues/372512084#comment2