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

DialogFragment and BottomSheetDialogFragment as part of navigation stack #250

Open
uvaysss opened this issue Nov 15, 2021 · 6 comments
Open

Comments

@uvaysss
Copy link

uvaysss commented Nov 15, 2021

I really like this libary and was wondering how to implement navigation for dialog and bottomsheet fragments.

Their are 2 reasons for this:

  • consistent navigation also for dialog and bottomsheet;
  • possibility to show another fullscreen fragment on top of a dialog or bottomsheet, because the usual one is shown with it's own window.

I was thinking about a own backstack for each fragment, but with a custom StateChanger where instead of attach/detach or show/hide would be used add/remove so that the fragments that are underneath could be visible.

Maybe you could give a hint about how you would do this kind of navigation or maybe why I should not do this, maybe there are some downsides to this.

@Zhuinden
Copy link
Owner

Heyo!

Historically, DefaultFragmentStateChanger contains what we've needed for full-screen-based fragment navigation, where each Fragment is its own unique screen.

The reason why dialogs never made it into it is because that suddenly introduces the concept of "destination parent-child relationships", namely that a DialogFragment does not exist alone: it is in fact automatically recreated by the fragmentManager. So it means there is a separation between how these fragments are managed, and suddenly some fragments are connected to others. You need to consider that "if a DialogFragmentKey is showing, then the KEY UNDER IT is the ACTUAL top key, and the dialog on top is a tracking of an event".

Then we must consider that that DialogFragment should track that when it is dismiss()ed, then it is removed from the backstack. And if it weren't removed on dismiss, then you'd end up with inconsistency.

As such, it was easier to say that if something is a dialog, then the fragment itself starts it.

You could definitely say that "but Jetpack Navigation has its own support for DialogFragments, surely it's a solveable problem" yes, and they do the tricks I mentioned above for tracking whether a DialogFragment should still be tracked as a destination, and they do this via LifecycleOwner, and at the time this did not exist nor would it have been actually worth it in comparison to just creating the DialogFragment via dialogFragment.show(parentFragmentManager) in our case. 🤔

@uvaysss
Copy link
Author

uvaysss commented Nov 16, 2021

Thank you for your quick response!

Well, I was thinking to treat the DialogFragment and BottomSheetDialogFragment as a usual Fragment by implementing them myself. Usual Fragment just with some animation, size difference and that their will be shown with a visible dimmed background. In this scenario I will not have this kind of issues, right?

@Zhuinden
Copy link
Owner

Zhuinden commented Nov 16, 2021

Well technically in that case they aren't really DialogFragment/BottomSheetDialogFragment, right? 😅

Something that has been historically tricky with fragments is to use an animation that opens them from below and be shown on top of the previous fragment, theoretically they created FragmentContainerView to "fix" that but that only works with addToBackStack() which I don't use.

I have previously used the following code in a real project for such customization:

class FragmentStateChanger(
    fragmentManager: FragmentManager,
    @IdRes containerId: Int
) : DefaultFragmentStateChanger(fragmentManager, containerId) {
    override fun onForwardNavigation(
        fragmentTransaction: FragmentTransaction,
        stateChange: StateChange
    ) {
        val topNewKey = stateChange.topNewKey<FragmentKey>()
        //val topPreviousKey = stateChange.topPreviousKey<FragmentKey>()

        if (topNewKey.shouldAnimateInFromBelow) {
            fragmentTransaction.setCustomAnimations(
                R.anim.slide_in_to_center,
                R.anim.animate_do_nothing,
                R.anim.slide_in_to_center,
                R.anim.animate_do_nothing
            )
        } else {
            super.onForwardNavigation(fragmentTransaction, stateChange)
        }
    }

    override fun onBackwardNavigation(
        fragmentTransaction: FragmentTransaction,
        stateChange: StateChange
    ) {
        val topNewKey = stateChange.topNewKey<FragmentKey>()
        val topPreviousKey = stateChange.topPreviousKey<FragmentKey>()

        if (topPreviousKey?.shouldAnimateInFromBelow == true) {
            fragmentTransaction.setCustomAnimations(
                R.anim.animate_do_nothing,
                R.anim.slide_out_from_center,
                R.anim.animate_do_nothing,
                R.anim.slide_out_from_center
            )
        } else {
            super.onBackwardNavigation(fragmentTransaction, stateChange)
        }
    }

    override fun onReplaceNavigation(
        fragmentTransaction: FragmentTransaction,
        stateChange: StateChange
    ) {
        val newKey = stateChange.topNewKey<FragmentKey>()

        if (newKey.shouldDisableAnimationOnReplace) {
            // do nothing
        } else {
            super.onReplaceNavigation(fragmentTransaction, stateChange)
        }
    }

Where slide_in_to_center is

<?xml version="1.0" encoding="utf-8"?>
<set>
    <translate xmlns:android="http://schemas.android.com/apk/res/android"
        android:fromYDelta="100%"
        android:toYDelta="0"
        android:interpolator="@android:anim/decelerate_interpolator"
        android:duration="250" />
</set>

slide_out_from_center is

<?xml version="1.0" encoding="utf-8"?>
<set>
    <translate xmlns:android="http://schemas.android.com/apk/res/android"
        android:fromYDelta="0%"
        android:toYDelta="100%"
        android:interpolator="@android:anim/decelerate_interpolator"
        android:duration="250" />
</set>

With a key like

abstract class FragmentKey: DefaultFragmentKey(), DefaultServiceProvider.HasServices {
    // ...
    open val shouldAnimateInFromBelow: Boolean = false

    open val shouldDisableAnimationOnReplace: Boolean = false
    // ...
}

Along with this hacky thing that allowed for the same hack that FragmentContainerView uses, but with the ability to configure whether it should be applied or not https://gist.github.com/Zhuinden/6ccd443d09c49f43f13b6169ae3d7966

As mentioned in this issue Zhuinden/simple-stack-extensions#5 (comment)


I am 100% guaranteed this looks daunting, but thankfully that part is a solved problem. Fragments have a lot of benefits, but their animation support kinda sucks, and has historically always sucked. Which is why these hoops exist at all. 😅

Anyway, as for the dimmed background, the trick is that you would want the previous fragment to show underneath, correct? If this is your plan, you might need to define parent-child relations to make sure that the "child" is added and the "parent" also stays added. 🤔

This requires a custom FragmentStateChanger implementation, although this is totally okay, the library itself does not enforce you to use DefaultFragmentStateChanger as you can provide any state changer implementation you need, but you need to know how Fragments, add/remove/attach/detach work.


And this is why we typically just did BottomSheetDialogFragment with .show(fragmentManager). 🤨

(And technically, setTargetFragment(this) before Googlers decided that a perfectly well-working type-safe synchronous API is too good so it should be deprecated in favor of string keys, see FragmentResultListener 🙄 )

Putting dialogs in the backstack creates a lot of edge-cases, I was very surprised when Google invented "dialog destinations" back when. That's my take on it anyway

@uvaysss
Copy link
Author

uvaysss commented Nov 16, 2021

Well technically in that case they aren't really DialogFragment/BottomSheetDialogFragment, right? 😅

Technically no, I was just implying that I'm aiming for this kind of behavior, sorry 😬

I am 100% guaranteed this looks daunting, but thankfully that part is a solved problem. Fragments have a lot of benefits, but their animation support kinda sucks, and has historically always sucked. Which is why these hoops exist at all. 😅

Well I'm not suprised that this the solution when working with fragments.

Anyway, as for the dimmed background, the trick is that you would want the previous fragment to show underneath, correct? If this is your plan, you might need to define parent-child relations to make sure that the "child" is added and the "parent" also stays added. 🤔

This requires a custom FragmentStateChanger implementation, although this is totally okay, the library itself does not enforce you to use DefaultFragmentStateChanger as you can provide any state changer implementation you need, but you need to know how Fragments, add/remove/attach/detach work.

Yes, this is the tricky part.
At first I was thinking to make for each fullscreen fragment a local stack with a custom state changer that would add/remove, but know I'm not so sure about this. Well I will take my time and think this through, сonsidering all your tips about this topic.

Thank you very much for your help!

@Zhuinden
Copy link
Owner

Zhuinden commented Nov 21, 2021

You're welcome. Technically I've been thinking about this since, and I've remembered that it seems to be a common requirement to have smaller translucent fragments without it being dialogs. This would require to add the last fragment, but not detach the one above it.

Technically I could theoretically add support for this in a non-breaking manner via a custom interface, if it is supported by DefaultFragmentStateChanger. I'll take it into consideration.

Till then though, I'd still advise using DialogFragment, it's really nice. 😅

@Zhuinden
Copy link
Owner

image

Even Jetpack Navigation has trouble making this work 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants