feat: add onComplete param to popWhile function in RouterExt.kt

[Tommyten]

Closes #98

[arkivanov]

Ahh, and could you please run ./gradlew apiDump and commit the changes?

[Tommyten]

Thank you for the suggestions, I've updated my branch

[arkivanov]

Sorry, the sample master-detail doesn't compile. It's better to also allow the most common use case without onComplete callback: router.popWhile {}.

Perhaps we need two variants - the existing one as it is now, and the new one with all required arguments:

router.popWhile(predicate) and router.popWhile(predicate, onComplete).

Do you want to update? Or alternatively I can do it in your branch. If you will update, please don't forget to rerun apiDump task at the end.

[arkivanov]

So the existing function will delegate to the new one, similar to Router.navigate -

Decompose/decompose/src/commonMain/kotlin/com/arkivanov/decompose/router/RouterExt.kt

Line 9 in 96d08a3

navigate(transformer = transformer, onComplete = { _, _ -> })

[Tommyten]

I will do it as soon as I get to my PC.
Sorry for the hassle 😅
I'm still getting used to the project but hoping to be involved more soon!

[arkivanov]

Thanks for your contributions!

Sorry for the hassle

I should have noticed this at the very beginning. :-)

[Tommyten]

How about we just swap the parameters to be

inline fun <C : Any> Router<C, *>.popWhile(
    crossinline onComplete: (isSuccess: Boolean) -> Unit = {},
    crossinline predicate: (C) -> Boolean
) {
    navigate(
        transformer = { stack -> stack.dropLastWhile(predicate) },
        onComplete = { newStack, oldStack -> onComplete(newStack.size < oldStack.size) }
    )
}

This way the API would be backwards compatible. Although having the first parameter being optional is not always best practice, we could prevent having to overload popWhile. You call.

[arkivanov]

How about we just swap the parameters to be

inline fun <C : Any> Router<C, *>.popWhile(
    crossinline onComplete: (isSuccess: Boolean) -> Unit = {},
    crossinline predicate: (C) -> Boolean
) {
    navigate(
        transformer = { stack -> stack.dropLastWhile(predicate) },
        onComplete = { newStack, oldStack -> onComplete(newStack.size < oldStack.size) }
    )
}

This way the API would be backwards compatible. Although having the first parameter being optional is not always best practice, we could prevent having to overload popWhile. You call.

There are a few points:

• I believe this change is still binary incompatible, it's only source compatible
• When using both arguments, it's looks intuitive to handle completion in the lambda outside parentheses
• There is already Navigator.navigate(transformer, onComplete) method, and an extension Navigator.navigate(transformer). This was done like this for the reason mentioned above. Would be good to keep popWhile aligned.

[arkivanov]

There is a build error:

API check failed for project decompose.

Could you please run ./gradlew :decompose:apiDump and commit the changes?

[arkivanov]

Thanks for the contribution!