From b42a512b871d40cfe3f43731be76ca96948f3f71 Mon Sep 17 00:00:00 2001 From: Colin Lee Date: Sat, 16 Feb 2019 20:55:49 -0600 Subject: [PATCH] Fixes #557: Selected tab should appear with selected theme --- .../org/mozilla/fenix/home/HomeFragment.kt | 60 ++++++++++++++----- .../mozilla/fenix/home/tabs/TabsAdapter.kt | 48 +++++++++++---- .../mozilla/fenix/home/tabs/TabsComponent.kt | 13 ++-- app/src/main/res/layout/tab_list_row.xml | 1 + 4 files changed, 93 insertions(+), 29 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt index ab4c718fab..86fc2d833b 100644 --- a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt @@ -29,6 +29,7 @@ import org.mozilla.fenix.home.tabs.TabsAction import org.mozilla.fenix.home.tabs.TabsChange import org.mozilla.fenix.home.tabs.TabsComponent import org.mozilla.fenix.home.tabs.TabsState +import org.mozilla.fenix.home.tabs.toSessionViewState import org.mozilla.fenix.isPrivate import org.mozilla.fenix.mvi.ActionBusFactory import org.mozilla.fenix.mvi.getAutoDisposeObservable @@ -46,8 +47,10 @@ class HomeFragment : Fragment() { savedInstanceState: Bundle? ): View? { val view = inflater.inflate(R.layout.fragment_home, container, false) + val sessionManager = requireComponents.core.sessionManager TabsComponent(view.homeLayout, bus, (activity as HomeActivity).browsingModeManager.isPrivate, - TabsState(requireComponents.core.sessionManager.sessions)) + TabsState(sessionManager.sessions.map { it.toSessionViewState(it == sessionManager.selectedSession) }) + ) SessionsComponent(view.homeLayout, bus, (activity as HomeActivity).browsingModeManager.isPrivate) layoutComponents(view) ActionBusFactory.get(this).logMergedObservables() @@ -66,13 +69,17 @@ class HomeFragment : Fragment() { .subscribe { when (it) { is TabsAction.Select -> { - requireComponents.core.sessionManager.select(it.session) - val directions = HomeFragmentDirections.actionHomeFragmentToBrowserFragment(it.session.id, + val session = requireComponents.core.sessionManager.findSessionById(it.sessionId) + requireComponents.core.sessionManager.select(session!!) + val directions = HomeFragmentDirections.actionHomeFragmentToBrowserFragment( + it.sessionId, (activity as HomeActivity).browsingModeManager.isPrivate) Navigation.findNavController(view).navigate(directions) } is TabsAction.Close -> { - requireComponents.core.sessionManager.remove(it.session) + requireComponents.core.sessionManager.findSessionById(it.sessionId)?.let { session -> + requireComponents.core.sessionManager.remove(session) + } } } } @@ -167,37 +174,62 @@ class HomeFragment : Fragment() { val observer = object : SessionManager.Observer { override fun onSessionAdded(session: Session) { super.onSessionAdded(session) + val sessionManager = requireComponents.core.sessionManager getManagedEmitter().onNext( - TabsChange.Changed(requireComponents.core.sessionManager.sessions - .filter { (activity as HomeActivity).browsingModeManager.isPrivate == it.private })) + TabsChange.Changed( + sessionManager.sessions + .filter { (activity as HomeActivity).browsingModeManager.isPrivate == it.private } + .map { it.toSessionViewState(it == sessionManager.selectedSession) } + ) + ) } override fun onSessionRemoved(session: Session) { super.onSessionRemoved(session) + val sessionManager = requireComponents.core.sessionManager getManagedEmitter().onNext( - TabsChange.Changed(requireComponents.core.sessionManager.sessions - .filter { (activity as HomeActivity).browsingModeManager.isPrivate == it.private })) + TabsChange.Changed( + sessionManager.sessions + .filter { (activity as HomeActivity).browsingModeManager.isPrivate == it.private } + .map { it.toSessionViewState(it == sessionManager.selectedSession) } + ) + ) } override fun onSessionSelected(session: Session) { super.onSessionSelected(session) + val sessionManager = requireComponents.core.sessionManager getManagedEmitter().onNext( - TabsChange.Changed(requireComponents.core.sessionManager.sessions - .filter { (activity as HomeActivity).browsingModeManager.isPrivate == it.private })) + TabsChange.Changed( + sessionManager.sessions + .filter { (activity as HomeActivity).browsingModeManager.isPrivate == it.private } + .map { it.toSessionViewState(it == sessionManager.selectedSession) } + ) + ) } override fun onSessionsRestored() { super.onSessionsRestored() + val sessionManager = requireComponents.core.sessionManager getManagedEmitter().onNext( - TabsChange.Changed(requireComponents.core.sessionManager.sessions - .filter { (activity as HomeActivity).browsingModeManager.isPrivate == it.private })) + TabsChange.Changed( + sessionManager.sessions + .filter { (activity as HomeActivity).browsingModeManager.isPrivate == it.private } + .map { it.toSessionViewState(it == sessionManager.selectedSession) } + ) + ) } override fun onAllSessionsRemoved() { super.onAllSessionsRemoved() + val sessionManager = requireComponents.core.sessionManager getManagedEmitter().onNext( - TabsChange.Changed(requireComponents.core.sessionManager.sessions - .filter { (activity as HomeActivity).browsingModeManager.isPrivate == it.private })) + TabsChange.Changed( + sessionManager.sessions + .filter { (activity as HomeActivity).browsingModeManager.isPrivate == it.private } + .map { it.toSessionViewState(it == sessionManager.selectedSession) } + ) + ) } } requireComponents.core.sessionManager.register(observer) diff --git a/app/src/main/java/org/mozilla/fenix/home/tabs/TabsAdapter.kt b/app/src/main/java/org/mozilla/fenix/home/tabs/TabsAdapter.kt index a040fcf3fb..3bb9982570 100644 --- a/app/src/main/java/org/mozilla/fenix/home/tabs/TabsAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/home/tabs/TabsAdapter.kt @@ -13,14 +13,13 @@ import androidx.recyclerview.widget.RecyclerView import io.reactivex.Observer import kotlinx.android.extensions.LayoutContainer import kotlinx.android.synthetic.main.tab_list_row.* -import mozilla.components.browser.session.Session import org.mozilla.fenix.R import org.mozilla.fenix.ext.increaseTapArea class TabsAdapter(private val actionEmitter: Observer) : RecyclerView.Adapter() { - var sessions = listOf() + var sessions = listOf() set(value) { val diffResult = DiffUtil.calculateDiff(TabsDiffCallback(field, value), true) field = value @@ -44,31 +43,50 @@ class TabsAdapter(private val actionEmitter: Observer) : } } + override fun onBindViewHolder(holder: RecyclerView.ViewHolder, position: Int, payloads: MutableList) { + if (payloads.isEmpty()) onBindViewHolder(holder, position) + else if (holder is TabViewHolder) { + val bundle = payloads[0] as Bundle + bundle.getString(tab_url)?.apply(holder::updateUrl) + bundle.getBoolean(tab_selected).apply(holder::updateSelected) + } + } + private class TabViewHolder( - view: View, + val view: View, actionEmitter: Observer, override val containerView: View? = view ) : RecyclerView.ViewHolder(view), LayoutContainer { - var session: Session? = null + var session: SessionViewState? = null init { item_tab.setOnClickListener { - actionEmitter.onNext(TabsAction.Select(session!!)) + actionEmitter.onNext(TabsAction.Select(session?.id!!)) } close_tab_button?.run { increaseTapArea(closeButtonIncreaseDps) setOnClickListener { - actionEmitter.onNext(TabsAction.Close(session!!)) + actionEmitter.onNext(TabsAction.Close(session?.id!!)) } } } - fun bindSession(session: Session) { + fun bindSession(session: SessionViewState) { this.session = session - text_url.text = session.url + updateUrl(session.url) + updateSelected(session.selected) + } + + fun updateUrl(url: String) { + text_url.text = url + } + + fun updateSelected(selected: Boolean) { + item_tab.background = if (selected) + view.context.getDrawable(R.drawable.session_border) else null } companion object { @@ -76,11 +94,16 @@ class TabsAdapter(private val actionEmitter: Observer) : const val LAYOUT_ID = R.layout.tab_list_row } } + + companion object { + const val tab_url = "url" + const val tab_selected = "selected" + } } class TabsDiffCallback( - private val oldList: List, - private val newList: List + private val oldList: List, + private val newList: List ) : DiffUtil.Callback() { override fun getOldListSize(): Int = oldList.size @@ -99,7 +122,10 @@ class TabsDiffCallback( val newSession = newList[newItemPosition] val diffBundle = Bundle() if (oldSession.url != newSession.url) { - diffBundle.putString("url", newSession.url) + diffBundle.putString(TabsAdapter.tab_url, newSession.url) + } + if (oldSession.selected != newSession.selected) { + diffBundle.putBoolean(TabsAdapter.tab_selected, newSession.selected) } return if (diffBundle.size() == 0) null else diffBundle } diff --git a/app/src/main/java/org/mozilla/fenix/home/tabs/TabsComponent.kt b/app/src/main/java/org/mozilla/fenix/home/tabs/TabsComponent.kt index bb11d25d19..878093a43a 100644 --- a/app/src/main/java/org/mozilla/fenix/home/tabs/TabsComponent.kt +++ b/app/src/main/java/org/mozilla/fenix/home/tabs/TabsComponent.kt @@ -36,13 +36,18 @@ class TabsComponent( } } -data class TabsState(val sessions: List) : ViewState +data class TabsState(val sessions: List) : ViewState +data class SessionViewState(val id: String, val url: String, val selected: Boolean) + +fun Session.toSessionViewState(selected: Boolean): SessionViewState { + return SessionViewState(this.id, this.url, selected) +} sealed class TabsAction : Action { - data class Select(val session: Session) : TabsAction() - data class Close(val session: Session) : TabsAction() + data class Select(val sessionId: String) : TabsAction() + data class Close(val sessionId: String) : TabsAction() } sealed class TabsChange : Change { - data class Changed(val sessions: List) : TabsChange() + data class Changed(val sessions: List) : TabsChange() } diff --git a/app/src/main/res/layout/tab_list_row.xml b/app/src/main/res/layout/tab_list_row.xml index 44a5131971..c9347e3e8e 100644 --- a/app/src/main/res/layout/tab_list_row.xml +++ b/app/src/main/res/layout/tab_list_row.xml @@ -25,6 +25,7 @@ android:tint="@android:color/black" android:paddingStart="10dp" android:paddingEnd="10dp" + android:paddingTop="4dp" app:layout_constraintStart_toStartOf="parent" app:layout_constraintTop_toTopOf="parent" android:contentDescription="@string/favicon_content_description"/>