Weird behaviour when navigating to new key of same class fragment about simple-stack HOT 9 CLOSED

Hey hey,

System identity hash code is definitely not a good choice, it's not exact across process death. You'll end up with duplicate fragments.

If you need 2 different fragment instances, especially based on the incoming arguments, I recommend making the key be a data class, and override getFragmentTag with toString().

I typically use toString() but you need to ensure that it's implemented in a stable way, like how data class does it, so using the class FQN as the default makes more sense for fragment tag from the library perspective (specifically because the default implementation of toString adds the hash code, which differs after process death).

If there is no differing argument, you can also create UUID as part of the key.

@Parcelize
data class MyKey(private val keyUuid: String = UUID().toString()): DefaultFragmentKey() {
    override fun getFragmentTag(): String = toString() 

    ... 
}

from simple-stack.

Hey, thanks for lightening fast response @Zhuinden

Seems like i have some complications here

I have a base class

@kotlinx.android.parcel.Parcelize
open class BaseKey(val creator : ()->Fragment): DefaultFragmentKey() {
    override fun instantiateFragment(): Fragment  = creator()

    @Nonnull
    override fun getFragmentTag() = this::class.qualifiedName + ":" + System.identityHashCode(this)
}

and a sample key

class PersonDetailKey(val personId: String) : BaseKey({
        PersonDetailFragment()
    })

Would it be fine if I do following modifications

@kotlinx.android.parcel.Parcelize
open class BaseKey(val creator : ()->Fragment): DefaultFragmentKey() {
    override fun instantiateFragment(): Fragment  = creator()

    @Nonnull
    override fun getFragmentTag() = toString() // < maybe this wont work n i have to shift this to all subclasses?

data class PersonDetailKey(val personId: String) : BaseKey({    //< made it a data class
        PersonDetailFragment()
    })

from simple-stack.

fun main() {
    val c = C("somearg")
    println(c.toString())
    println(c.getTag())

}

open class P():I{
    override fun getTag() = toString()
}

data class C(val p :String):P()

interface I{
    fun getTag():String
}

prints

C(p=somearg)
C(p=somearg)

So i guess it'd work 🤫

from simple-stack.

@Zhuinden I see one more problem with your solution of using UUID and toString of data class
Let's say I have a key data class ShowInputArgsScreenKey(val input : String)
It creates a fragment whose edittext shows input value and clicking on button below it goes to new Key with current edittext input

As this key has a parameter, adding uuid param is not required but i think there will be issues if user just keeps clicking the go button, as two keys are not different objects but their toString values are same (passing unedited input)

Then I have 2 questions

1. Would that be issue?
2. If that'd be an issue then can I use nanoid as field in base class and override getFragmentTag as

@kotlinx.android.parcel.Parcelize
open class BaseKey(val creator : ()->Fragment): DefaultFragmentKey() {
   val uuid = UUIDUtils.createNewUUID()
    override fun instantiateFragment(): Fragment  = creator()

    @Nonnull
    override fun getFragmentTag() = toString()+uuid
    }

from simple-stack.

1.) definitely don't use System.identityHashCode(this) as part of the fragment tag

2.) if you use UUID for a key to ensure each key is unique, I'd definitely make sure that gets parcelized, so the UUID string has to be part of the primary constructor.

Normally I use toString() alone, and ensure that each key is a data class (so that hashcode isn't part of toString).

from simple-stack.

I don't get how System.identityHashCode(this) will cause issues.. can you explain

from simple-stack.

If you test for process death https://stackoverflow.com/a/49107399/2413303 you'll have fragments recreated by the system that you won't be able to remove as you navigate, so they'll overlap each other. The default implementation of DefaultFragmentKey uses getClass().getName() specifically to avoid usage of hasbCode() as part of the fragment tag.

from simple-stack.

So after process death the fragments are recreated with the old tags but as new keys are also recreated their getFragmentTag() will return a different string because of hashcode change, so the DefaultFragmentStateChanger won't be able to find and remove the old fragments when navigating and instead create new fragments with instantiateFragment()
This results in old fragments instantiated by the os again to remain in fragment backstack maybe even visible to user..
So to avoid this, the keys which should differ based on input args and hence return toString for getFragmentTag() must also retain those input args across process death
Got it 👍

I wonder if simple-stack's default behaviour to use same fragment for different key instances differs from android (new activity/fragments (screen) are created when user navigates with new data (bundles/args) unless singleInstance ofc but that's explicit declaration. I'm amazed how nobody saw that as a problem before)

from simple-stack.

I think they didn't see it as an issue because changing the fragment tag is fairly simple.

The default Fragment behaviors would allow you to stack the same fragment type 3 times on itself but if it has the same tag it'd only find the last one at a time, a bit surprising really. Finding the fragment by its tag being reliable is something people generally like as a feature.

from simple-stack.