Close #170 - Go to parent tab if one exists on back press

[jonalmeida]

Pull Request checklist

• Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features

[rahulsainani]

❓ Mostly for my understanding, I was checking the RemoveTabUsecase (reducer), it seems the tab.parentId != null check is done there, so do we need to pass hasParent as the second param or just true would suffice.

[jonalmeida]

Good point, we don't have to do the extra check for the parentId. When this code was originally written, the use case was not doing this check, so we don't need the refactor to include it as well.

Since we're also not in the SessionState.Source.External case, then the tab is TabSessionState is also redundant as we're not a CustomTabSessionState then.

[rahulsainani]

Since we're also not in the SessionState.Source.External case, then the tab is TabSessionState is also redundant as we're not a CustomTabSessionState then.

CustomTabSessionState could still have source as SessionState.Source.Internal.CustomTab right? So just the case that the source is not external is not proof enough that the tab is TabSessionState right?

[jonalmeida]

Interesting point! If a custom tab is internal then we probably do want to finish the activity too because we would be in a context of a separate activity (external app activity).

The secondary value of the tab is TabSessionState && tab.parentId != null check is for the return value to tell our other features that we've handled the back press in this feature.

So maybe the if-statement above should be:

val isExternalOrCustomTab = tab.source is SessionState.Source.External ||
    tab.source is SessionState.Source.Internal.CustomTab

return if (isExternalOrCustomTab && !tab.restored) {
    activity.finish()
    removeTabUseCase(tab.id)
    true
} else {
    val hasParent = tab is TabSessionState && tab.parentId != null
    removeTabUseCase(tab.id, selectParentIfExists = hasParent) // Alternatively, set this to always true.
    // We want to return to home if this session didn't have a parent session to select.
    hasParent
}

What do you think?

[rahulsainani]

Looks good and works well! 🚀 Had one question.