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

AdyenComponent Composable disappears on recomposition #1399

Closed
ronaldvanduren opened this issue Nov 24, 2023 · 5 comments · Fixed by #1403
Closed

AdyenComponent Composable disappears on recomposition #1399

ronaldvanduren opened this issue Nov 24, 2023 · 5 comments · Fixed by #1403
Labels
Confirmed bug Indicates that issue has been confirmed to be a bug by an Adyen developer

Comments

@ronaldvanduren
Copy link

Describe the bug
After migrating to 5.0.1 I notice that the AdyenComponent is not shown after the initial composition of the screen (using Compose). It works fine if the AdyenComponent is added and not removed from the view. But if the AdyenComponent is removed and added again, the second time it's not shown. Also, if it's used inside a HorizontalPager it's not shown at all.

In 4.13.3 I implemented the CardView in Compose without this issue, the view remains always visible.

This works(v4.13.3)

@Composable
private fun PaymentMethods(
    .......
    pagerState: PagerState,
    viewState: ......
    onCreditCardInputChanged: (CardComponentState) -> Unit
) {
    
    HorizontalPager(state = pagerState) { currentPage ->
        viewState.paymentMethods.elementAt(currentPage).let { paymentMethod ->
            when (paymentMethod) {
                is PaymentMethod.Twint -> TwintContent()
                is PaymentMethod.CreditCard -> CreditCardContent(
                    cardComponent,
                    onCreditCardInputChanged
                )
            }
        }
    }
}

@Composable
internal fun CreditCardContent(
    cardComponent: CardComponent,
    onCreditCardInputChanged: (CardComponentState) -> Unit = { }
) {
    Column(
        Modifier.height(PAYMENT_CONTENT_HEIGHT.dp)
    ) {
        
        val lifecycleOwner = LocalLifecycleOwner.current

        DisposableEffect(cardComponent) {

            cardComponent.observe(lifecycleOwner) { paymentComponentState ->
                onCreditCardInputChanged.invoke(paymentComponentState)
            }

            onDispose {
                cardComponent.removeObservers(lifecycleOwner)
            }
        }

        AndroidView(factory = { context ->
            CardView(context).apply {
                attach(cardComponent, lifecycleOwner)
            }
        }, modifier = Modifier.fillMaxWidth()
        )
    }
}

This doesn't work (v5.01)

@Composable
private fun PaymentMethods(
    ........
    pagerState: PagerState,
    viewState: ....
    onCreditCardInputChanged: (CardComponentState) -> Unit
) {
    ......
    HorizontalPager(state = pagerState) { currentPage ->
        viewState.paymentMethods.elementAt(currentPage).let { paymentMethod ->
            when (paymentMethod) {
                is PaymentMethod.Twint -> TwintContent()
                is PaymentMethod.CreditCard -> CreditCardContent(cardComponent)
            }
        }
    }
}

@Composable
internal fun CreditCardContent(
    cardComponent: CardComponent
) {
    Column(
        Modifier.height(PAYMENT_CONTENT_HEIGHT.dp)
    ) {
        
        AdyenComponent(
            component = cardComponent,
            modifier = Modifier.fillMaxWidth()
        )
    }
}

This does work (v5.01)

@Composable
private fun PaymentMethods(
    ........
    pagerState: PagerState,
    viewState: ....
    onCreditCardInputChanged: (CardComponentState) -> Unit
) {
        CreditCardContent(cardComponent)
    ......
}

@Composable
internal fun CreditCardContent(
    cardComponent: CardComponent
) {
    Column(
        Modifier.height(PAYMENT_CONTENT_HEIGHT.dp)
    ) {
        
        AdyenComponent(
            component = cardComponent,
            modifier = Modifier.fillMaxWidth()
        )
    }
}

Expected behavior
Whenever the AdyenComponent is added dynamically to the view tree (e.g. via HorizontalPager), then the AdyenComponent should be visible.

Smartphone:

  • Device: Pixel 6
  • OS: Android 14
@ronaldvanduren ronaldvanduren changed the title AdyenComponent Compable disappears on recomposition AdyenComponent Composable disappears on recomposition Nov 27, 2023
@araratthehero
Copy link
Contributor

araratthehero commented Nov 28, 2023

Hi @ronaldvanduren, thank you for reaching out and for the detailed explanation.

We are currently investigating and will issue a fix as soon as we find the cause.
Meanwhile I did a quick investigation and have a suggestion which might help you solve the issue for the time being.

What you could do is to set the beyondBoundsPageCount to a value bigger than 0 (More information can be found in the documentation of Pager here). The beyondBoundsPageCount field can be adjusted based on how many payment methods you have in the HorizontalPager.

Here is the modified code for your case:

@Composable
private fun PaymentMethods(
    ........
    pagerState: PagerState,
    viewState: ....
    onCreditCardInputChanged: (CardComponentState) -> Unit
) {
    ......
    HorizontalPager(
        state = pagerState,
        beyondBoundsPageCount = 1 // Any value should work here, based on the amount of payment methods
    ) { currentPage ->
        viewState.paymentMethods.elementAt(currentPage).let { paymentMethod ->
            when (paymentMethod) {
                is PaymentMethod.Twint -> TwintContent()
                is PaymentMethod.CreditCard -> CreditCardContent(cardComponent)
            }
        }
    }
}

For the case of using only two payment methods, setting contentPadding to any value bigger than 0 also helps, since the view stays visible on the screen. Here is the example:

@Composable
private fun PaymentMethods(
    ........
    pagerState: PagerState,
    viewState: ....
    onCreditCardInputChanged: (CardComponentState) -> Unit
) {
    ......
    HorizontalPager(
        state = pagerState,
        contentPadding = PaddingValues(horizontal = 32.dp) // Adds padding around the whole content
    ) { currentPage ->
        viewState.paymentMethods.elementAt(currentPage).let { paymentMethod ->
            when (paymentMethod) {
                is PaymentMethod.Twint -> TwintContent()
                is PaymentMethod.CreditCard -> CreditCardContent(cardComponent)
            }
        }
    }
}

I know that if you have more payment methods, using the beyondBoundsPageCount field might compromise on the performance of the pager, but this will solve your issue while we work on a more permanent fix.

Could you please try that and let us know if it works for you?

@araratthehero araratthehero added the Confirmed bug Indicates that issue has been confirmed to be a bug by an Adyen developer label Nov 28, 2023
@ronaldvanduren
Copy link
Author

Thanks for the reply! Unfortunately, setting beyondBoundsPageCount does not work consistently. It "sometimes" works when the debugger is attached, this with the fact that in 5.x.x the components make use of flows to initialise the views makes me think it might be a concurrency issue.

The same behaviour is observed when setting the contentPadding, when the debugger or the layout inspector is attached, then the component shows more often.

I do see however that the AdyenComponentView is inflated and that the CardView is attached, it's just that the CardView is not properly inflated (it seems).

AdyenComponentView
Screenshot 2023-12-01 at 11 47 30

CardView
Screenshot 2023-12-01 at 11 47 44

@araratthehero araratthehero linked a pull request Dec 5, 2023 that will close this issue
3 tasks
@araratthehero araratthehero added the Pending release Indicates issue is pending a release to be solved label Dec 6, 2023
@araratthehero
Copy link
Contributor

Hi @ronaldvanduren, the fix for this bug has been merged and will be available in our next release.
We will keep you updated when it goes live.

Thanks again for reporting it!

@araratthehero
Copy link
Contributor

Hi @ronaldvanduren, we just released 5.1.0 which includes a fix for this issue, let us know if you still face any problems with it.

@ronaldvanduren
Copy link
Author

Sorry for the late reply, it's a different sprint for us and this task was pushed back to the backlog. However, I checked today and the issue is resolved 🎉!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Confirmed bug Indicates that issue has been confirmed to be a bug by an Adyen developer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants