[fenix] Issue https://github.com/mozilla-mobile/fenix/issues/20054: Update recent tab on title or icon changes

pull/600/head
Jonathan Almeida 3 years ago committed by mergify[bot]
parent 35cd30fbe1
commit 3e2f501795

@ -9,6 +9,7 @@ import mozilla.components.browser.state.state.TabSessionState
import mozilla.components.feature.tab.collections.TabCollection import mozilla.components.feature.tab.collections.TabCollection
import mozilla.components.feature.top.sites.TopSite import mozilla.components.feature.top.sites.TopSite
import mozilla.components.lib.state.Action import mozilla.components.lib.state.Action
import mozilla.components.lib.state.Middleware
import mozilla.components.lib.state.State import mozilla.components.lib.state.State
import mozilla.components.lib.state.Store import mozilla.components.lib.state.Store
import org.mozilla.fenix.components.tips.Tip import org.mozilla.fenix.components.tips.Tip
@ -17,9 +18,10 @@ import org.mozilla.fenix.components.tips.Tip
* The [Store] for holding the [HomeFragmentState] and applying [HomeFragmentAction]s. * The [Store] for holding the [HomeFragmentState] and applying [HomeFragmentAction]s.
*/ */
class HomeFragmentStore( class HomeFragmentStore(
initialState: HomeFragmentState = HomeFragmentState() initialState: HomeFragmentState = HomeFragmentState(),
middlewares: List<Middleware<HomeFragmentState, HomeFragmentAction>> = emptyList()
) : Store<HomeFragmentState, HomeFragmentAction>( ) : Store<HomeFragmentState, HomeFragmentAction>(
initialState, ::homeFragmentStateReducer initialState, ::homeFragmentStateReducer, middlewares
) )
data class Tab( data class Tab(

@ -8,11 +8,11 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.collect import kotlinx.coroutines.flow.collect
import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.map
import mozilla.components.browser.state.selector.normalTabs import mozilla.components.browser.state.selector.selectedTab
import mozilla.components.browser.state.state.BrowserState import mozilla.components.browser.state.state.BrowserState
import mozilla.components.browser.state.store.BrowserStore import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.lib.state.helpers.AbstractBinding import mozilla.components.lib.state.helpers.AbstractBinding
import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifChanged import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifAnyChanged
import org.mozilla.fenix.home.HomeFragmentAction import org.mozilla.fenix.home.HomeFragmentAction
import org.mozilla.fenix.home.HomeFragmentStore import org.mozilla.fenix.home.HomeFragmentStore
@ -22,20 +22,17 @@ import org.mozilla.fenix.home.HomeFragmentStore
*/ */
@OptIn(ExperimentalCoroutinesApi::class) @OptIn(ExperimentalCoroutinesApi::class)
class RecentTabsListFeature( class RecentTabsListFeature(
private val browserStore: BrowserStore, browserStore: BrowserStore,
private val homeStore: HomeFragmentStore private val homeStore: HomeFragmentStore
) : AbstractBinding<BrowserState>(browserStore) { ) : AbstractBinding<BrowserState>(browserStore) {
override suspend fun onState(flow: Flow<BrowserState>) { override suspend fun onState(flow: Flow<BrowserState>) {
flow.map { it.selectedTabId } flow.map { it.selectedTab }
.ifChanged() .ifAnyChanged { arrayOf(it?.id, it?.content?.title, it?.content?.icon) }
.collect { selectedTabId -> .collect { selectedTab ->
// Attempt to get the selected normal tab since here may not be a selected tab or // Attempt to get the selected normal tab since here may not be a selected tab or
// the selected tab may be a private tab. // the selected tab may be a private tab.
val selectedTab = browserStore.state.normalTabs.firstOrNull { val recentTabsList = if (selectedTab != null && selectedTab.content.private.not()) {
it.id == selectedTabId
}
val recentTabsList = if (selectedTab != null) {
listOf(selectedTab) listOf(selectedTab)
} else { } else {
emptyList() emptyList()

@ -7,6 +7,7 @@ package org.mozilla.fenix.home.recenttabs.view
import android.view.View import android.view.View
import kotlinx.android.synthetic.main.recent_tabs_list_row.* import kotlinx.android.synthetic.main.recent_tabs_list_row.*
import mozilla.components.browser.icons.BrowserIcons import mozilla.components.browser.icons.BrowserIcons
import mozilla.components.browser.state.state.ContentState
import mozilla.components.browser.state.state.TabSessionState import mozilla.components.browser.state.state.TabSessionState
import org.mozilla.fenix.R import org.mozilla.fenix.R
import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.components
@ -18,7 +19,8 @@ import org.mozilla.fenix.utils.view.ViewHolder
* View holder for a recent tab item. * View holder for a recent tab item.
* *
* @param interactor [RecentTabInteractor] which will have delegated to all user interactions. * @param interactor [RecentTabInteractor] which will have delegated to all user interactions.
* @param icons * @param icons an instance of [BrowserIcons] for rendering the sites icon if one isn't found
* in [ContentState.icon].
*/ */
class RecentTabViewHolder( class RecentTabViewHolder(
view: View, view: View,
@ -27,13 +29,19 @@ class RecentTabViewHolder(
) : ViewHolder(view) { ) : ViewHolder(view) {
fun bindTab(tab: TabSessionState) { fun bindTab(tab: TabSessionState) {
recent_tab_title.text = tab.content.title // A page may take a while to retrieve a title, so let's show the url until we get one.
recent_tab_title.text = if (tab.content.title.isNotEmpty()) {
tab.content.title
} else {
tab.content.url
}
if (tab.content.icon != null) { if (tab.content.icon != null) {
recent_tab_icon.setImageBitmap(tab.content.icon) recent_tab_icon.setImageBitmap(tab.content.icon)
} else { } else {
icons.loadIntoView(recent_tab_icon, tab.content.url) icons.loadIntoView(recent_tab_icon, tab.content.url)
} }
recent_tab_icon.setImageBitmap(tab.content.icon)
itemView.setOnClickListener { itemView.setOnClickListener {
interactor.onRecentTabClicked(tab.id) interactor.onRecentTabClicked(tab.id)

@ -137,6 +137,14 @@ sealed class AdapterItem(@LayoutRes val viewType: Int) {
object RecentTabsHeader : AdapterItem(RecentTabsHeaderViewHolder.LAYOUT_ID) object RecentTabsHeader : AdapterItem(RecentTabsHeaderViewHolder.LAYOUT_ID)
data class RecentTabItem(val tab: TabSessionState) : AdapterItem(RecentTabViewHolder.LAYOUT_ID) { data class RecentTabItem(val tab: TabSessionState) : AdapterItem(RecentTabViewHolder.LAYOUT_ID) {
override fun sameAs(other: AdapterItem) = other is RecentTabItem && tab.id == other.tab.id override fun sameAs(other: AdapterItem) = other is RecentTabItem && tab.id == other.tab.id
override fun contentsSameAs(other: AdapterItem): Boolean {
val otherItem = other as RecentTabItem
// We only care about updating if the title and icon have changed because that is
// all we show today. This should be updated if we want to show updates for more.
return tab.content.title == otherItem.tab.content.title &&
tab.content.icon == otherItem.tab.content.icon
}
} }
/** /**

@ -4,26 +4,50 @@
package org.mozilla.fenix.home package org.mozilla.fenix.home
import io.mockk.mockk
import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.TestCoroutineDispatcher import kotlinx.coroutines.test.TestCoroutineDispatcher
import mozilla.components.browser.state.action.ContentAction.UpdateIconAction
import mozilla.components.browser.state.action.ContentAction.UpdateTitleAction
import mozilla.components.browser.state.action.TabListAction import mozilla.components.browser.state.action.TabListAction
import mozilla.components.browser.state.state.BrowserState import mozilla.components.browser.state.state.BrowserState
import mozilla.components.browser.state.state.createTab import mozilla.components.browser.state.state.createTab
import mozilla.components.browser.state.store.BrowserStore import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.support.test.ext.joinBlocking import mozilla.components.support.test.ext.joinBlocking
import mozilla.components.support.test.libstate.ext.waitUntilIdle import mozilla.components.support.test.libstate.ext.waitUntilIdle
import mozilla.components.support.test.middleware.CaptureActionsMiddleware
import mozilla.components.support.test.rule.MainCoroutineRule import mozilla.components.support.test.rule.MainCoroutineRule
import org.junit.After
import org.junit.Assert.assertEquals import org.junit.Assert.assertEquals
import org.junit.Assert.assertNotNull
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Rule import org.junit.Rule
import org.junit.Test import org.junit.Test
import org.mozilla.fenix.home.HomeFragmentAction.RecentTabsChange
import org.mozilla.fenix.home.recenttabs.RecentTabsListFeature import org.mozilla.fenix.home.recenttabs.RecentTabsListFeature
class RecentTabsListFeatureTest { class RecentTabsListFeatureTest {
private lateinit var homeStore: HomeFragmentStore
private lateinit var middleware: CaptureActionsMiddleware<HomeFragmentState, HomeFragmentAction>
@OptIn(ExperimentalCoroutinesApi::class) @OptIn(ExperimentalCoroutinesApi::class)
@get:Rule @get:Rule
val coroutinesTestRule = MainCoroutineRule(TestCoroutineDispatcher()) val coroutinesTestRule = MainCoroutineRule(TestCoroutineDispatcher())
@Before
fun setup() {
middleware = CaptureActionsMiddleware()
homeStore = HomeFragmentStore(middlewares = listOf(middleware))
}
@After
fun teardown() {
middleware.reset()
}
@Test @Test
fun `GIVEN no selected tab WHEN the feature starts THEN dispatch an empty list`() { fun `GIVEN no selected tab WHEN the feature starts THEN dispatch an empty list`() {
val browserStore = BrowserStore() val browserStore = BrowserStore()
@ -53,7 +77,6 @@ class RecentTabsListFeatureTest {
selectedTabId = "1" selectedTabId = "1"
) )
) )
val homeStore = HomeFragmentStore()
val feature = RecentTabsListFeature( val feature = RecentTabsListFeature(
browserStore = browserStore, browserStore = browserStore,
homeStore = homeStore homeStore = homeStore
@ -83,7 +106,6 @@ class RecentTabsListFeatureTest {
selectedTabId = "1" selectedTabId = "1"
) )
) )
val homeStore = HomeFragmentStore()
val feature = RecentTabsListFeature( val feature = RecentTabsListFeature(
browserStore = browserStore, browserStore = browserStore,
homeStore = homeStore homeStore = homeStore
@ -122,7 +144,6 @@ class RecentTabsListFeatureTest {
selectedTabId = "1" selectedTabId = "1"
) )
) )
val homeStore = HomeFragmentStore()
val feature = RecentTabsListFeature( val feature = RecentTabsListFeature(
browserStore = browserStore, browserStore = browserStore,
homeStore = homeStore homeStore = homeStore
@ -141,4 +162,54 @@ class RecentTabsListFeatureTest {
assertEquals(0, homeStore.state.recentTabs.size) assertEquals(0, homeStore.state.recentTabs.size)
} }
@Test
fun `WHEN the selected tabs title or icon update THEN update the home store`() {
val browserStore = BrowserStore(
BrowserState(
tabs = listOf(
createTab(
url = "https://www.mozilla.org",
id = "1"
)
),
selectedTabId = "1"
)
)
val feature = RecentTabsListFeature(
browserStore = browserStore,
homeStore = homeStore
)
feature.start()
homeStore.waitUntilIdle()
middleware.assertLastAction(RecentTabsChange::class) {
val tab = it.recentTabs.first()
assertTrue(tab.content.title.isEmpty())
assertNull(tab.content.icon)
}
browserStore.dispatch(UpdateTitleAction("1", "test")).joinBlocking()
homeStore.waitUntilIdle()
middleware.assertLastAction(RecentTabsChange::class) {
val tab = it.recentTabs.first()
assertEquals("test", tab.content.title)
assertNull(tab.content.icon)
}
browserStore.dispatch(UpdateIconAction("1", "https://www.mozilla.org", mockk()))
.joinBlocking()
homeStore.waitUntilIdle()
middleware.assertLastAction(RecentTabsChange::class) {
val tab = it.recentTabs.first()
assertEquals("test", tab.content.title)
assertNotNull(tab.content.icon)
}
}
} }

@ -6,6 +6,7 @@ package org.mozilla.fenix.home.recenttabs.view
import android.view.LayoutInflater import android.view.LayoutInflater
import android.view.View import android.view.View
import androidx.core.graphics.drawable.toBitmap
import io.mockk.every import io.mockk.every
import io.mockk.mockk import io.mockk.mockk
import io.mockk.verify import io.mockk.verify
@ -16,9 +17,12 @@ import mozilla.components.browser.icons.IconRequest
import mozilla.components.browser.state.state.createTab import mozilla.components.browser.state.state.createTab
import mozilla.components.support.test.robolectric.testContext import mozilla.components.support.test.robolectric.testContext
import org.junit.Assert.assertEquals import org.junit.Assert.assertEquals
import org.junit.Assert.assertNotNull
import org.junit.Assert.assertNull
import org.junit.Before import org.junit.Before
import org.junit.Test import org.junit.Test
import org.junit.runner.RunWith import org.junit.runner.RunWith
import org.mozilla.fenix.R
import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
import org.mozilla.fenix.home.sessioncontrol.SessionControlInteractor import org.mozilla.fenix.home.sessioncontrol.SessionControlInteractor
@ -60,4 +64,26 @@ class RecentTabViewHolderTest {
verify { interactor.onRecentTabClicked(tab.id) } verify { interactor.onRecentTabClicked(tab.id) }
} }
@Test
fun `WHEN a recent tab icon exists THEN load it`() {
val bitmap = testContext.getDrawable(R.drawable.ic_search)!!.toBitmap()
val tabWithIcon = tab.copy(content = tab.content.copy(icon = bitmap))
val viewHolder = RecentTabViewHolder(view, interactor, icons)
assertNull(view.recent_tab_icon.drawable)
viewHolder.bindTab(tabWithIcon)
assertNotNull(view.recent_tab_icon.drawable)
}
@Test
fun `WHEN a recent tab does not have a title THEN show the url`() {
val tabWithoutTitle = createTab(url = "https://mozilla.org")
RecentTabViewHolder(view, interactor, icons).bindTab(tabWithoutTitle)
assertEquals(tabWithoutTitle.content.url, view.recent_tab_title.text)
}
} }

Loading…
Cancel
Save