From 291f21aa8e903faa453535afea9df777cd9db0a3 Mon Sep 17 00:00:00 2001 From: Colin Lee Date: Mon, 18 Feb 2019 15:18:55 -0600 Subject: [PATCH] Handle listener state bugs as onViewCreated isn't always called --- .../mozilla/fenix/browser/BrowserFragment.kt | 29 +++-- .../org/mozilla/fenix/home/HomeFragment.kt | 103 +++++++----------- .../fenix/library/history/HistoryFragment.kt | 21 ++-- .../mozilla/fenix/search/SearchFragment.kt | 3 + 4 files changed, 73 insertions(+), 83 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt index 811163303..a8f30351a 100644 --- a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt @@ -103,18 +103,6 @@ class BrowserFragment : Fragment(), BackHandler { override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) - getAutoDisposeObservable() - .subscribe { - when (it) { - is SearchAction.ToolbarTapped -> Navigation.findNavController(toolbar) - .navigate(BrowserFragmentDirections.actionBrowserFragmentToSearchFragment( - requireComponents.core.sessionManager.selectedSession?.id, - (activity as HomeActivity).browsingModeManager.isPrivate - )) - is SearchAction.ToolbarMenuItemTapped -> handleToolbarItemInteraction(it) - } - } - sessionId = BrowserFragmentArgs.fromBundle(arguments!!).sessionId (activity as AppCompatActivity).supportActionBar?.hide() @@ -184,6 +172,23 @@ class BrowserFragment : Fragment(), BackHandler { view = view) } + override fun onStart() { + super.onStart() + getAutoDisposeObservable() + .subscribe { + when (it) { + is SearchAction.ToolbarTapped -> Navigation.findNavController(toolbar) + .navigate( + BrowserFragmentDirections.actionBrowserFragmentToSearchFragment( + requireComponents.core.sessionManager.selectedSession?.id, + (activity as HomeActivity).browsingModeManager.isPrivate + ) + ) + is SearchAction.ToolbarMenuItemTapped -> handleToolbarItemInteraction(it) + } + } + } + @SuppressWarnings("ReturnCount") override fun onBackPressed(): Boolean { if (findInPageIntegration.onBackPressed()) return true 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 86fc2d833..a4ab0e5ca 100644 --- a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt @@ -65,25 +65,6 @@ class HomeFragment : Fragment() { (activity as AppCompatActivity).supportActionBar?.hide() setupHomeMenu() - getAutoDisposeObservable() - .subscribe { - when (it) { - is TabsAction.Select -> { - 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.findSessionById(it.sessionId)?.let { session -> - requireComponents.core.sessionManager.remove(session) - } - } - } - } - val searchIcon = requireComponents.search.searchEngineManager.getDefaultSearchEngine(requireContext()).let { BitmapDrawable(resources, it.icon) } @@ -97,7 +78,7 @@ class HomeFragment : Fragment() { view.toolbar.setCompoundDrawablesWithIntrinsicBounds(searchIcon, null, null, null) val roundToInt = (toolbarPaddingDp * Resources.getSystem().displayMetrics.density).roundToInt() view.toolbar.compoundDrawablePadding = roundToInt - view.toolbar.setOnClickListener { it -> + view.toolbar.setOnClickListener { val directions = HomeFragmentDirections.actionHomeFragmentToSearchFragment(null, (activity as HomeActivity).browsingModeManager.isPrivate) Navigation.findNavController(it).navigate(directions) @@ -145,8 +126,30 @@ class HomeFragment : Fragment() { } } - override fun onResume() { - super.onResume() + override fun onStart() { + super.onStart() + if (isAdded) { + getAutoDisposeObservable() + .subscribe { + when (it) { + is TabsAction.Select -> { + 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.findSessionById(it.sessionId)?.let { session -> + requireComponents.core.sessionManager.remove(session) + } + } + } + } + } + sessionObserver = subscribeToSessions() sessionObserver?.onSessionsRestored() } @@ -174,68 +177,44 @@ 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( - sessionManager.sessions - .filter { (activity as HomeActivity).browsingModeManager.isPrivate == it.private } - .map { it.toSessionViewState(it == sessionManager.selectedSession) } - ) - ) + emitSessionChanges() } override fun onSessionRemoved(session: Session) { super.onSessionRemoved(session) - val sessionManager = requireComponents.core.sessionManager - getManagedEmitter().onNext( - TabsChange.Changed( - sessionManager.sessions - .filter { (activity as HomeActivity).browsingModeManager.isPrivate == it.private } - .map { it.toSessionViewState(it == sessionManager.selectedSession) } - ) - ) + emitSessionChanges() } override fun onSessionSelected(session: Session) { super.onSessionSelected(session) - val sessionManager = requireComponents.core.sessionManager - getManagedEmitter().onNext( - TabsChange.Changed( - sessionManager.sessions - .filter { (activity as HomeActivity).browsingModeManager.isPrivate == it.private } - .map { it.toSessionViewState(it == sessionManager.selectedSession) } - ) - ) + emitSessionChanges() } override fun onSessionsRestored() { super.onSessionsRestored() - val sessionManager = requireComponents.core.sessionManager - getManagedEmitter().onNext( - TabsChange.Changed( - sessionManager.sessions - .filter { (activity as HomeActivity).browsingModeManager.isPrivate == it.private } - .map { it.toSessionViewState(it == sessionManager.selectedSession) } - ) - ) + emitSessionChanges() } override fun onAllSessionsRemoved() { super.onAllSessionsRemoved() - val sessionManager = requireComponents.core.sessionManager - getManagedEmitter().onNext( - TabsChange.Changed( - sessionManager.sessions - .filter { (activity as HomeActivity).browsingModeManager.isPrivate == it.private } - .map { it.toSessionViewState(it == sessionManager.selectedSession) } - ) - ) + emitSessionChanges() } } requireComponents.core.sessionManager.register(observer) return observer } + private fun emitSessionChanges() { + val sessionManager = requireComponents.core.sessionManager + getManagedEmitter().onNext( + TabsChange.Changed( + sessionManager.sessions + .filter { (activity as HomeActivity).browsingModeManager.isPrivate == it.private } + .map { it.toSessionViewState(it == sessionManager.selectedSession) } + ) + ) + } + companion object { const val addTabButtonIncreaseDps = 8 const val overflowButtonIncreaseDps = 8 diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt index d8ce40e3a..3a84fd4c5 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt @@ -79,6 +79,18 @@ class HistoryFragment : Fragment(), CoroutineScope, BackHandler { override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) + launch(Dispatchers.IO) { + val items = requireComponents.core.historyStorage.getVisited() + .mapIndexed { id, item -> HistoryItem(id, item) } + + launch(Dispatchers.Main) { + getManagedEmitter().onNext(HistoryChange.Change(items)) + } + } + } + + override fun onStart() { + super.onStart() getAutoDisposeObservable() .subscribe { when (it) { @@ -93,15 +105,6 @@ class HistoryFragment : Fragment(), CoroutineScope, BackHandler { .onNext(HistoryChange.ExitEditMode) } } - - launch(Dispatchers.IO) { - val items = requireComponents.core.historyStorage.getVisited() - .mapIndexed { id, item -> HistoryItem(id, item) } - - launch(Dispatchers.Main) { - getManagedEmitter().onNext(HistoryChange.Change(items)) - } - } } override fun onOptionsItemSelected(item: MenuItem): Boolean { diff --git a/app/src/main/java/org/mozilla/fenix/search/SearchFragment.kt b/app/src/main/java/org/mozilla/fenix/search/SearchFragment.kt index 818751f81..e5c8f892b 100644 --- a/app/src/main/java/org/mozilla/fenix/search/SearchFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/search/SearchFragment.kt @@ -63,7 +63,10 @@ class SearchFragment : Fragment() { lifecycle.addObserver((toolbarComponent.uiView as ToolbarUIView).toolbarIntegration) view.toolbar_wrapper.clipToOutline = false + } + override fun onStart() { + super.onStart() getAutoDisposeObservable() .subscribe { when (it) {