`AppNavigator#setupFragmentTransaction` public signature compatibility

[v3nko]

It's very nice to see the library fully rewritten in a pure Kotlin. I understand the need of reviewing some internal solutions in favor of better approaches, etc. However, I believe that public API should not be changed due to internal rewrite until there are specific technical reasons.

Currently, I'm talking about AppNavigator#setupFragmentTransaction which now has a reduced number of arguments in a signature. In most of the projects with Cicerone I've used this callback to setup generic animations along with shared transitions and achieved this by extending the commands with transition arguments. The new implementation of a navigator does not have this argument. Yes, the transitions/animations are a separate topic to discuss, but still, this is not about animations but transaction options in general. With the new callback signature, there is no good way to pass such options into a transaction. Fragment arguments may look like workaround but they are not supposed to store transaction options.

You may suggest overriding commitNewFragmentScreen. This technically means copy-paste of this function with a slight change just for transaction options callback. I consider this as not a good solution eighter as instead of extending you are copying functionality and creating one more place to check on each library update (it may receive updates and fixes and you need to check this part manually).

I'd like to suggest adding a command argument to the setupFragmentTransaction like in previous versions.

[terrakok]

This is good point. Sorry for changing this external API. But let me give you another variant for animation setup: what do you think about adding animation parameters to screen instance and taking them in setupFragmentTransaction:

class AnimatedScreen(
    val animation: Any
) : FragmentScreen(
    key = "AnimatedScreen",
    fragmentCreator = { AnimatedScreen() }
)

override fun setupFragmentTransaction(
    screen: FragmentScreen,
    fragmentTransaction: FragmentTransaction,
    currentFragment: Fragment?,
    nextFragment: Fragment
) {
    if (screen is Screens.AnimatedScreen) {
        val animationData = screen.animation
        // some animation setup
    }
}

[terrakok]

At the moment I'm going to move add/replace flag from command parameter to FragmentScreen property

[terrakok]

I've done it:

• daef586461e8c0c4f03d8251641bec51392e8a27
• 4fc5b26980b270c02913da9d9bb034b1b6c59b4f

[terrakok]

I think it's more right way cause FragmentScreen is class for description all parameters for opening new fragment: key, arguments, transaction type and fragment factory

[terrakok]

New version 7.0 has been published