Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Commit

Permalink
For #22078 Selecting search group in Jump back in switches active tab
Browse files Browse the repository at this point in the history
  • Loading branch information
Amejia481 authored and mergify[bot] committed Nov 3, 2021
1 parent 1809bea commit cff78af
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,12 @@ class DefaultRecentTabsController(
}

override fun handleRecentSearchGroupClicked(tabId: String) {
selectTabUseCase.invoke(tabId)
metrics.track(Event.JumpBackInGroupTapped)
navController.navigate(HomeFragmentDirections.actionGlobalTabsTrayFragment())
navController.navigate(
HomeFragmentDirections.actionGlobalTabsTrayFragment(
focusGroupTabId = tabId
)
)
}

@VisibleForTesting(otherwise = PRIVATE)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,8 @@ class TabsTrayFragment : AppCompatDialogFragment() {
tabsTrayStore = StoreProvider.get(this) {
TabsTrayStore(
initialState = TabsTrayState(
mode = initialMode
mode = initialMode,
focusGroupTabId = args.focusGroupTabId
)
)
}
Expand Down
11 changes: 10 additions & 1 deletion app/src/main/java/org/mozilla/fenix/tabstray/TabsTrayStore.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@ import mozilla.components.lib.state.Store
* currently selected tabs.
* @property syncing Whether the Synced Tabs feature should fetch the latest tabs from paired
* devices.
* @property focusGroupTabId The search group tab id to focus. Defaults to null.
*/
data class TabsTrayState(
val selectedPage: Page = Page.NormalTabs,
val mode: Mode = Mode.Normal,
val syncing: Boolean = false
val syncing: Boolean = false,
val focusGroupTabId: String? = null
) : State {

/**
Expand Down Expand Up @@ -119,6 +121,11 @@ sealed class TabsTrayAction : Action {
* no sync action was able to be performed.
*/
object SyncCompleted : TabsTrayAction()

/**
* Removes the [TabsTrayState.focusGroupTabId] of the [TabsTrayState].
*/
object ConsumeFocusGroupTabIdAction : TabsTrayAction()
}

/**
Expand Down Expand Up @@ -149,6 +156,8 @@ internal object TabsTrayReducer {
state.copy(syncing = true)
is TabsTrayAction.SyncCompleted ->
state.copy(syncing = false)
is TabsTrayAction.ConsumeFocusGroupTabIdAction ->
state.copy(focusGroupTabId = null)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import org.mozilla.fenix.components.appstate.AppAction
import org.mozilla.fenix.components.AppStore
import org.mozilla.fenix.ext.settings
import org.mozilla.fenix.selection.SelectionHolder
import org.mozilla.fenix.tabstray.TabsTrayAction
import org.mozilla.fenix.tabstray.TabsTrayInteractor
import org.mozilla.fenix.tabstray.TabsTrayStore
import org.mozilla.fenix.tabstray.browser.containsTabId
Expand Down Expand Up @@ -84,6 +85,10 @@ class NormalBrowserPageViewHolder(
val searchTermTabGroupsAreEnabled = containerView.context.settings().searchTermTabGroupsAreEnabled

val selectedTab = browserStore.state.selectedNormalTab ?: return
// It's safe to read the state directly (i.e. won't cause bugs because of the store actions
// processed on a separate thread) instead of observing it because this value is only set during
// the initialState of the TabsTrayStore being created.
val focusGroupTabId = tabsTrayStore.state.focusGroupTabId

// Update tabs into the inactive adapter.
if (inactiveTabsAreEnabled && selectedTab.isNormalTabInactive(maxActiveTime)) {
Expand All @@ -106,7 +111,13 @@ class NormalBrowserPageViewHolder(
}

// Updates tabs into the search term group adapter.
if (searchTermTabGroupsAreEnabled && selectedTab.isNormalTabActiveWithSearchTerm(maxActiveTime)) {
if (searchTermTabGroupsAreEnabled && (
!focusGroupTabId.isNullOrEmpty() ||
selectedTab.isNormalTabActiveWithSearchTerm(maxActiveTime)
)
) {
val tabId = focusGroupTabId ?: selectedTab.id

tabGroupAdapter.observeFirstInsert {
// With a grouping, we need to use the list of the adapter that is already grouped
// together for the UI, so we know the final index of the grouping to scroll to.
Expand All @@ -117,35 +128,40 @@ class NormalBrowserPageViewHolder(
// [DiffUtil.calculateDiff] directly to submit a changed list which evades the `ListAdapter` from being
// notified of updates, so it therefore returns an empty list.
tabGroupAdapter.currentList.forEachIndexed { groupIndex, group ->
if (group.containsTabId(selectedTab.id)) {
if (group.containsTabId(tabId)) {

// Index is based on tabs above (inactive) with our calculated index.
val indexToScrollTo = inactiveTabAdapter.itemCount + groupIndex
layoutManager.scrollToPosition(indexToScrollTo)

if (focusGroupTabId != null) {
tabsTrayStore.dispatch(TabsTrayAction.ConsumeFocusGroupTabIdAction)
}
return@observeFirstInsert
}
}
}
}

// Updates tabs into the normal browser tabs adapter.
browserAdapter.observeFirstInsert {
val activeTabsList = browserStore.state.getNormalTrayTabs(
searchTermTabGroupsAreEnabled,
inactiveTabsAreEnabled
)
activeTabsList.forEachIndexed { tabIndex, trayTab ->
if (trayTab.id == selectedTab.id) {

// Index is based on tabs above (inactive + groups + header) with our calculated index.
val indexToScrollTo = inactiveTabAdapter.itemCount +
tabGroupAdapter.itemCount +
headerAdapter.itemCount + tabIndex
if (focusGroupTabId.isNullOrEmpty()) {
// Updates tabs into the normal browser tabs adapter.
browserAdapter.observeFirstInsert {
val activeTabsList = browserStore.state.getNormalTrayTabs(
searchTermTabGroupsAreEnabled,
inactiveTabsAreEnabled
)
activeTabsList.forEachIndexed { tabIndex, trayTab ->
if (trayTab.id == selectedTab.id) {

// Index is based on tabs above (inactive + groups + header) with our calculated index.
val indexToScrollTo = inactiveTabAdapter.itemCount +
tabGroupAdapter.itemCount +
headerAdapter.itemCount + tabIndex

layoutManager.scrollToPosition(indexToScrollTo)
layoutManager.scrollToPosition(indexToScrollTo)

return@observeFirstInsert
return@observeFirstInsert
}
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions app/src/main/res/navigation/nav_graph.xml
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,11 @@
android:name="enterMultiselect"
android:defaultValue="false"
app:argType="boolean" />
<argument
android:name="focusGroupTabId"
app:nullable="true"
android:defaultValue="@null"
app:argType="string" />
</dialog>

<fragment
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

package org.mozilla.fenix.tabstray

import org.junit.Assert.assertEquals
import org.junit.Test

class TabsTrayStoreReducerTest {
@Test
fun `GIVEN focusGroupTabId WHEN ConsumeFocusGroupTabIdAction THEN focusGroupTabId must be consumed`() {
val initialState = TabsTrayState(focusGroupTabId = "id")
val expectedState = initialState.copy(focusGroupTabId = null)

val resultState = TabsTrayReducer.reduce(
initialState,
TabsTrayAction.ConsumeFocusGroupTabIdAction
)

assertEquals(expectedState, resultState)
}
}

0 comments on commit cff78af

Please sign in to comment.