From b5142bb93159f333f8dd9a50eebcf63b339f512a Mon Sep 17 00:00:00 2001 From: Simon Chae Date: Wed, 17 Apr 2019 11:51:35 -0400 Subject: [PATCH 01/27] Issue #1395: Introduce FocusRepo --- .../main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt diff --git a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt new file mode 100644 index 0000000000..df05eccc68 --- /dev/null +++ b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt @@ -0,0 +1,7 @@ +/* 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.tv.firefox.focus + +class FocusRepo() {} From 7e9568c3f67d99d79a718d8a33982673f5ff825b Mon Sep 17 00:00:00 2001 From: Simon Chae Date: Mon, 22 Apr 2019 14:28:39 -0400 Subject: [PATCH 02/27] Issue #1395: Add all required repos to FocusRepo --- .../org/mozilla/tv/firefox/focus/FocusRepo.kt | 39 ++++++++++++++++++- .../tv/firefox/pinnedtile/PinnedTileRepo.kt | 2 + 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt index df05eccc68..e5ca73ce9e 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt @@ -4,4 +4,41 @@ package org.mozilla.tv.firefox.focus -class FocusRepo() {} +import androidx.annotation.VisibleForTesting +import io.reactivex.rxkotlin.Observables +import org.mozilla.tv.firefox.ScreenController +import org.mozilla.tv.firefox.ScreenControllerStateMachine +import org.mozilla.tv.firefox.pinnedtile.PinnedTileRepo +import org.mozilla.tv.firefox.pocket.PocketVideoRepo +import org.mozilla.tv.firefox.session.SessionRepo + +class FocusRepo( + screenController: ScreenController, + sessionRepo: SessionRepo, + pinnedTileRepo: PinnedTileRepo, + pocketRepo: PocketVideoRepo) { + + private val _transition = Observables.combineLatest( + screenController.currentActiveScreen, + sessionRepo.state, + pinnedTileRepo.isEmpty, + pocketRepo.feedState) { activeScreen, sessionState, pinnedTilesIsEmpty, pocketState -> + dispatchFocusUpdates(activeScreen, sessionState, pinnedTilesIsEmpty, pocketState) + } + + @VisibleForTesting + private fun dispatchFocusUpdates( + activeScreen: ScreenControllerStateMachine.ActiveScreen, + sessionState: SessionRepo.State, + pinnedTilesIsEmpty: Boolean, + pocketState: PocketVideoRepo.FeedState) { + + when (activeScreen) { + ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY -> Unit + ScreenControllerStateMachine.ActiveScreen.WEB_RENDER -> Unit + ScreenControllerStateMachine.ActiveScreen.POCKET -> Unit + ScreenControllerStateMachine.ActiveScreen.SETTINGS -> Unit + } + + } +} diff --git a/app/src/main/java/org/mozilla/tv/firefox/pinnedtile/PinnedTileRepo.kt b/app/src/main/java/org/mozilla/tv/firefox/pinnedtile/PinnedTileRepo.kt index f3001cb55a..75b4332f6a 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/pinnedtile/PinnedTileRepo.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/pinnedtile/PinnedTileRepo.kt @@ -37,6 +37,8 @@ class PinnedTileRepo(private val applicationContext: Application) { BehaviorSubject.create() val pinnedTiles: Observable> = _pinnedTiles.hide() + val isEmpty: Observable = _pinnedTiles.map { it.size == 0 }.distinctUntilChanged() + @Deprecated(message = "Use PinnedTileRepo.pinnedTiles for new code") val legacyPinnedTiles = LiveDataReactiveStreams .fromPublisher(pinnedTiles.toFlowable(BackpressureStrategy.LATEST)) From 83834256bcabe8943dd0a379e0fbd25b96a70bc6 Mon Sep 17 00:00:00 2001 From: Simon Chae Date: Mon, 22 Apr 2019 15:44:44 -0400 Subject: [PATCH 03/27] Issue #1395: Add updateNavUrlInputFocus() --- .../org/mozilla/tv/firefox/focus/FocusRepo.kt | 64 +++++++++++++++++-- 1 file changed, 59 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt index e5ca73ce9e..7ad8ba0745 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt @@ -4,13 +4,17 @@ package org.mozilla.tv.firefox.focus +import android.view.View import androidx.annotation.VisibleForTesting import io.reactivex.rxkotlin.Observables +import io.reactivex.subjects.BehaviorSubject +import org.mozilla.tv.firefox.R import org.mozilla.tv.firefox.ScreenController import org.mozilla.tv.firefox.ScreenControllerStateMachine import org.mozilla.tv.firefox.pinnedtile.PinnedTileRepo import org.mozilla.tv.firefox.pocket.PocketVideoRepo import org.mozilla.tv.firefox.session.SessionRepo +import org.mozilla.tv.firefox.utils.URLs class FocusRepo( screenController: ScreenController, @@ -18,11 +22,13 @@ class FocusRepo( pinnedTileRepo: PinnedTileRepo, pocketRepo: PocketVideoRepo) { + val focusedView: BehaviorSubject = BehaviorSubject.create() // TODO: potential for telemetry? + private val _transition = Observables.combineLatest( - screenController.currentActiveScreen, - sessionRepo.state, - pinnedTileRepo.isEmpty, - pocketRepo.feedState) { activeScreen, sessionState, pinnedTilesIsEmpty, pocketState -> + screenController.currentActiveScreen, + sessionRepo.state, + pinnedTileRepo.isEmpty, + pocketRepo.feedState) { activeScreen, sessionState, pinnedTilesIsEmpty, pocketState -> dispatchFocusUpdates(activeScreen, sessionState, pinnedTilesIsEmpty, pocketState) } @@ -33,12 +39,60 @@ class FocusRepo( pinnedTilesIsEmpty: Boolean, pocketState: PocketVideoRepo.FeedState) { + if (!focusedView.hasValue()) { + // TODO: handle focus lost + } + + val currFocusedView: View = focusedView.value!! + when (activeScreen) { - ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY -> Unit + ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY -> { + when (currFocusedView.id) { + R.id.navUrlInput -> + updateNavUrlInputFocus(currFocusedView, sessionState, pinnedTilesIsEmpty, pocketState) + R.id.navButtonReload -> { + + } + R.id.navButtonForward -> { + + } + R.id.pocketVideoMegaTileView -> { + + } + } + } ScreenControllerStateMachine.ActiveScreen.WEB_RENDER -> Unit ScreenControllerStateMachine.ActiveScreen.POCKET -> Unit ScreenControllerStateMachine.ActiveScreen.SETTINGS -> Unit } } + + private fun updateNavUrlInputFocus( + focusedNavUrlInputView: View, + sessionState: SessionRepo.State, + pinnedTilesIsEmpty: Boolean, + pocketState: PocketVideoRepo.FeedState) { + + assert(focusedNavUrlInputView.id == R.id.navUrlInput) + + focusedNavUrlInputView.nextFocusDownId = when { + pocketState is PocketVideoRepo.FeedState.FetchFailed -> R.id.megaTileTryAgainButton + pocketState is PocketVideoRepo.FeedState.Inactive -> { + if (pinnedTilesIsEmpty) { + R.id.navUrlInput // if + } else { + R.id.tileContainer + } + } + else -> R.id.pocketVideoMegaTileView + } + + focusedNavUrlInputView.nextFocusUpId = when { + sessionState.backEnabled -> R.id.navButtonBack + sessionState.forwardEnabled -> R.id.navButtonForward + sessionState.currentUrl != URLs.APP_URL_HOME -> R.id.navButtonReload + else -> R.id.turboButton + } + } } From 48b31550f6222033267e7012002fa39a8708a227 Mon Sep 17 00:00:00 2001 From: Simon Chae Date: Mon, 22 Apr 2019 17:51:39 -0400 Subject: [PATCH 04/27] Issue #1395: Add GlobalFocusChangeListener --- .../org/mozilla/tv/firefox/MainActivity.kt | 7 ++- .../org/mozilla/tv/firefox/focus/FocusRepo.kt | 27 ++++++------ .../tv/firefox/utils/ServiceLocator.kt | 2 + app/src/main/res/layout/activity_main.xml | 43 ++++++++++--------- 4 files changed, 44 insertions(+), 35 deletions(-) diff --git a/app/src/main/java/org/mozilla/tv/firefox/MainActivity.kt b/app/src/main/java/org/mozilla/tv/firefox/MainActivity.kt index 8a675b20ba..5363c43794 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/MainActivity.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/MainActivity.kt @@ -13,9 +13,10 @@ import android.util.AttributeSet import android.util.Log import android.view.KeyEvent import android.view.View +import android.view.ViewTreeObserver import androidx.lifecycle.Observer import io.sentry.Sentry -import kotlinx.android.synthetic.main.activity_main.container_navigation_overlay +import kotlinx.android.synthetic.main.activity_main.* import kotlinx.android.synthetic.main.overlay_debug.debugLog import mozilla.components.browser.session.Session import mozilla.components.concept.engine.EngineView @@ -104,7 +105,7 @@ class MainActivity : LocaleAwareAppCompatActivity(), OnUrlEnteredListener, Media serviceLocator.intentLiveData.value = Consumable.from(intentData) // Debug logging display for non public users - // TODO: refactor out the debug variant visibility check in #1953 + // TODO: refactor out the debug variant visibility check in #1953\ BuildConstants.debugLogStr?.apply { debugLog.visibility = View.VISIBLE debugLog.text = this @@ -170,6 +171,7 @@ class MainActivity : LocaleAwareAppCompatActivity(), OnUrlEnteredListener, Media super.onStart() // TODO when MainActivity has a VM, route this call through it serviceLocator.pocketRepo.startBackgroundUpdates() + rootView.viewTreeObserver.addOnGlobalFocusChangeListener(serviceLocator.focusRepo) } override fun onStop() { @@ -177,6 +179,7 @@ class MainActivity : LocaleAwareAppCompatActivity(), OnUrlEnteredListener, Media // TODO when MainActivity has a VM, route this call through it serviceLocator.pocketRepo.stopBackgroundUpdates() TelemetryIntegration.INSTANCE.stopMainActivity() + rootView.viewTreeObserver.removeOnGlobalFocusChangeListener(serviceLocator.focusRepo) } override fun onDestroy() { diff --git a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt index 7ad8ba0745..918fb994c3 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt @@ -5,6 +5,7 @@ package org.mozilla.tv.firefox.focus import android.view.View +import android.view.ViewTreeObserver import androidx.annotation.VisibleForTesting import io.reactivex.rxkotlin.Observables import io.reactivex.subjects.BehaviorSubject @@ -20,36 +21,38 @@ class FocusRepo( screenController: ScreenController, sessionRepo: SessionRepo, pinnedTileRepo: PinnedTileRepo, - pocketRepo: PocketVideoRepo) { + pocketRepo: PocketVideoRepo): ViewTreeObserver.OnGlobalFocusChangeListener { - val focusedView: BehaviorSubject = BehaviorSubject.create() // TODO: potential for telemetry? + private var focusedView: BehaviorSubject = BehaviorSubject.create() // TODO: potential for telemetry? private val _transition = Observables.combineLatest( + focusedView, screenController.currentActiveScreen, sessionRepo.state, pinnedTileRepo.isEmpty, - pocketRepo.feedState) { activeScreen, sessionState, pinnedTilesIsEmpty, pocketState -> - dispatchFocusUpdates(activeScreen, sessionState, pinnedTilesIsEmpty, pocketState) + pocketRepo.feedState) { focusedView, activeScreen, sessionState, pinnedTilesIsEmpty, pocketState -> + dispatchFocusUpdates(focusedView, activeScreen, sessionState, pinnedTilesIsEmpty, pocketState) + } + + override fun onGlobalFocusChanged(oldFocus: View?, newFocus: View?) { + newFocus?.apply { + focusedView.onNext(this) + } } @VisibleForTesting private fun dispatchFocusUpdates( + focusedView: View, activeScreen: ScreenControllerStateMachine.ActiveScreen, sessionState: SessionRepo.State, pinnedTilesIsEmpty: Boolean, pocketState: PocketVideoRepo.FeedState) { - if (!focusedView.hasValue()) { - // TODO: handle focus lost - } - - val currFocusedView: View = focusedView.value!! - when (activeScreen) { ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY -> { - when (currFocusedView.id) { + when (focusedView.id) { R.id.navUrlInput -> - updateNavUrlInputFocus(currFocusedView, sessionState, pinnedTilesIsEmpty, pocketState) + updateNavUrlInputFocus(focusedView, sessionState, pinnedTilesIsEmpty, pocketState) R.id.navButtonReload -> { } diff --git a/app/src/main/java/org/mozilla/tv/firefox/utils/ServiceLocator.kt b/app/src/main/java/org/mozilla/tv/firefox/utils/ServiceLocator.kt index 623ed9e344..b7156e0aed 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/utils/ServiceLocator.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/utils/ServiceLocator.kt @@ -15,6 +15,7 @@ import org.mozilla.tv.firefox.experiments.ExperimentsProvider import org.mozilla.tv.firefox.experiments.FretboardProvider import org.mozilla.tv.firefox.ext.getAccessibilityManager import org.mozilla.tv.firefox.ext.webRenderComponents +import org.mozilla.tv.firefox.focus.FocusRepo import org.mozilla.tv.firefox.framework.FrameworkRepo import org.mozilla.tv.firefox.pinnedtile.PinnedTileRepo import org.mozilla.tv.firefox.pocket.PocketEndpoint @@ -77,6 +78,7 @@ open class ServiceLocator(val app: Application) { val sessionUseCases get() = app.webRenderComponents.sessionUseCases val searchEngineManager by lazy { SearchEngineManagerFactory.create(app) } val cursorEventRepo by lazy { CursorEventRepo(screenController) } + val focusRepo by lazy { FocusRepo(screenController, sessionRepo, pinnedTileRepo, pocketRepo) } open val frameworkRepo = FrameworkRepo.newInstanceAndInit(app.getAccessibilityManager()) open val pinnedTileRepo by lazy { PinnedTileRepo(app) } diff --git a/app/src/main/res/layout/activity_main.xml b/app/src/main/res/layout/activity_main.xml index adb0b335f3..b66cf8a7c9 100644 --- a/app/src/main/res/layout/activity_main.xml +++ b/app/src/main/res/layout/activity_main.xml @@ -2,35 +2,36 @@ - + + android:orientation="vertical" + android:id="@+id/container_web_render" + android:layout_width="match_parent" + android:layout_height="match_parent"/> + android:orientation="vertical" + android:id="@+id/container_navigation_overlay" + android:layout_width="match_parent" + android:layout_height="match_parent"/> + android:orientation="vertical" + android:id="@+id/container_pocket" + android:layout_width="match_parent" + android:layout_height="match_parent"/> + android:orientation="vertical" + android:id="@+id/container_settings" + android:layout_width="match_parent" + android:layout_height="match_parent"/> - + From 7bdddaca52187b946360231dedb23258b3943679 Mon Sep 17 00:00:00 2001 From: Simon Chae Date: Tue, 23 Apr 2019 11:33:10 -0400 Subject: [PATCH 05/27] Issue #1395: Introduce FocusRepo state --- .../org/mozilla/tv/firefox/focus/FocusRepo.kt | 47 ++++++++++++++++--- 1 file changed, 40 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt index 918fb994c3..e20110e35e 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt @@ -23,23 +23,56 @@ class FocusRepo( pinnedTileRepo: PinnedTileRepo, pocketRepo: PocketVideoRepo): ViewTreeObserver.OnGlobalFocusChangeListener { - private var focusedView: BehaviorSubject = BehaviorSubject.create() // TODO: potential for telemetry? + data class State( + val focusedView: View, + val defaultFocusMap: HashMap + ) - private val _transition = Observables.combineLatest( - focusedView, + private var _state: BehaviorSubject = BehaviorSubject.create() // TODO: potential for telemetry? + + // Keep track of prevScreen to identify screen transitions + private var prevScreen: ScreenControllerStateMachine.ActiveScreen = + ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY + + private val _focusUpdate = Observables.combineLatest( + _state, screenController.currentActiveScreen, sessionRepo.state, pinnedTileRepo.isEmpty, - pocketRepo.feedState) { focusedView, activeScreen, sessionState, pinnedTilesIsEmpty, pocketState -> - dispatchFocusUpdates(focusedView, activeScreen, sessionState, pinnedTilesIsEmpty, pocketState) + pocketRepo.feedState) { state, activeScreen, sessionState, pinnedTilesIsEmpty, pocketState -> + dispatchFocusUpdates(state.focusedView, activeScreen, sessionState, pinnedTilesIsEmpty, pocketState) + } + + val focusUpdate = _focusUpdate.hide() + + init { + initializeDefaultFocus() } override fun onGlobalFocusChanged(oldFocus: View?, newFocus: View?) { newFocus?.apply { - focusedView.onNext(this) + val newState = State( + focusedView = this, + defaultFocusMap = _state.value!!.defaultFocusMap + ) + _state.onNext(newState) } } + private fun initializeDefaultFocus() { + val focusMap = HashMap() + focusMap[ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY] = R.id.navUrlInput + focusMap[ScreenControllerStateMachine.ActiveScreen.WEB_RENDER] = R.id.engineView + focusMap[ScreenControllerStateMachine.ActiveScreen.POCKET] = R.id.videoFeed + + val newState = State( + focusedView = _state.value!!.focusedView, + defaultFocusMap = focusMap + ) + + _state.onNext(newState) + } + @VisibleForTesting private fun dispatchFocusUpdates( focusedView: View, @@ -68,7 +101,7 @@ class FocusRepo( ScreenControllerStateMachine.ActiveScreen.POCKET -> Unit ScreenControllerStateMachine.ActiveScreen.SETTINGS -> Unit } - + prevScreen = activeScreen } private fun updateNavUrlInputFocus( From d8b9607e3c18bece2bdcd92cdfed931f18c8011b Mon Sep 17 00:00:00 2001 From: Simon Chae Date: Tue, 23 Apr 2019 14:09:30 -0400 Subject: [PATCH 06/27] Issue #1395: Add FocusNode and remove android View references --- .../org/mozilla/tv/firefox/focus/FocusRepo.kt | 58 ++++++++++++++----- 1 file changed, 43 insertions(+), 15 deletions(-) diff --git a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt index e20110e35e..cb04b5db76 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt @@ -24,10 +24,30 @@ class FocusRepo( pocketRepo: PocketVideoRepo): ViewTreeObserver.OnGlobalFocusChangeListener { data class State( - val focusedView: View, + val focusNode: FocusNode, val defaultFocusMap: HashMap ) + /** + * FocusNode describes quasi-directional focusable paths given viewId + */ + data class FocusNode( + val viewId: Int, + val nextFocusUpId: Int? = null, + val nextFocusDownId: Int? = null, + val nextFocusLeftId: Int? = null, + val nextFocusRightId: Int? = null + ) { + fun updateViewNodeTree(view: View) { + assert(view.id == viewId) + + nextFocusUpId?.let { view.nextFocusUpId = it } + nextFocusDownId?.let { view.nextFocusDownId = it } + nextFocusLeftId?.let { view.nextFocusLeftId = it } + nextFocusRightId?.let { view.nextFocusRightId = it } + } + } + private var _state: BehaviorSubject = BehaviorSubject.create() // TODO: potential for telemetry? // Keep track of prevScreen to identify screen transitions @@ -40,7 +60,7 @@ class FocusRepo( sessionRepo.state, pinnedTileRepo.isEmpty, pocketRepo.feedState) { state, activeScreen, sessionState, pinnedTilesIsEmpty, pocketState -> - dispatchFocusUpdates(state.focusedView, activeScreen, sessionState, pinnedTilesIsEmpty, pocketState) + dispatchFocusUpdates(state.focusNode, activeScreen, sessionState, pinnedTilesIsEmpty, pocketState) } val focusUpdate = _focusUpdate.hide() @@ -52,9 +72,9 @@ class FocusRepo( override fun onGlobalFocusChanged(oldFocus: View?, newFocus: View?) { newFocus?.apply { val newState = State( - focusedView = this, - defaultFocusMap = _state.value!!.defaultFocusMap - ) + focusNode = FocusNode(newFocus.id), + defaultFocusMap = _state.value!!.defaultFocusMap) + _state.onNext(newState) } } @@ -66,16 +86,15 @@ class FocusRepo( focusMap[ScreenControllerStateMachine.ActiveScreen.POCKET] = R.id.videoFeed val newState = State( - focusedView = _state.value!!.focusedView, - defaultFocusMap = focusMap - ) + focusNode = FocusNode(R.id.navUrlInput), + defaultFocusMap = focusMap) _state.onNext(newState) } @VisibleForTesting private fun dispatchFocusUpdates( - focusedView: View, + focusNode: FocusNode, activeScreen: ScreenControllerStateMachine.ActiveScreen, sessionState: SessionRepo.State, pinnedTilesIsEmpty: Boolean, @@ -83,9 +102,9 @@ class FocusRepo( when (activeScreen) { ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY -> { - when (focusedView.id) { + when (focusNode.viewId) { R.id.navUrlInput -> - updateNavUrlInputFocus(focusedView, sessionState, pinnedTilesIsEmpty, pocketState) + updateNavUrlInputFocus(focusNode, sessionState, pinnedTilesIsEmpty, pocketState) R.id.navButtonReload -> { } @@ -105,14 +124,14 @@ class FocusRepo( } private fun updateNavUrlInputFocus( - focusedNavUrlInputView: View, + focusedNavUrlInputNode: FocusNode, sessionState: SessionRepo.State, pinnedTilesIsEmpty: Boolean, pocketState: PocketVideoRepo.FeedState) { - assert(focusedNavUrlInputView.id == R.id.navUrlInput) + assert(focusedNavUrlInputNode.viewId == R.id.navUrlInput) - focusedNavUrlInputView.nextFocusDownId = when { + val nextFocusDownId = when { pocketState is PocketVideoRepo.FeedState.FetchFailed -> R.id.megaTileTryAgainButton pocketState is PocketVideoRepo.FeedState.Inactive -> { if (pinnedTilesIsEmpty) { @@ -124,11 +143,20 @@ class FocusRepo( else -> R.id.pocketVideoMegaTileView } - focusedNavUrlInputView.nextFocusUpId = when { + val nextFocusUpId = when { sessionState.backEnabled -> R.id.navButtonBack sessionState.forwardEnabled -> R.id.navButtonForward sessionState.currentUrl != URLs.APP_URL_HOME -> R.id.navButtonReload else -> R.id.turboButton } + + val newState = State( + focusNode = FocusNode( + focusedNavUrlInputNode.viewId, + nextFocusUpId, + nextFocusDownId), + defaultFocusMap = _state.value!!.defaultFocusMap) + + _state.onNext(newState) } } From da3ac347853a23be5c505581c7c4ea1ab8ceb46a Mon Sep 17 00:00:00 2001 From: Simon Chae Date: Tue, 23 Apr 2019 14:24:50 -0400 Subject: [PATCH 07/27] Issue #1395: Implement the rest of NavigationOverlay focusTree updates --- .../org/mozilla/tv/firefox/focus/FocusRepo.kt | 71 +++++++++++++++++-- 1 file changed, 66 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt index cb04b5db76..893bf8c3c0 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt @@ -104,15 +104,15 @@ class FocusRepo( ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY -> { when (focusNode.viewId) { R.id.navUrlInput -> - updateNavUrlInputFocus(focusNode, sessionState, pinnedTilesIsEmpty, pocketState) + updateNavUrlInputFocusTree(focusNode, sessionState, pinnedTilesIsEmpty, pocketState) R.id.navButtonReload -> { - + updateReloadButtonFocusTree(focusNode, sessionState) } R.id.navButtonForward -> { - + updateForwardButtonFocusTree(focusNode, sessionState) } R.id.pocketVideoMegaTileView -> { - + updatePocketMegaTileFocusTree(focusNode, pinnedTilesIsEmpty) } } } @@ -123,7 +123,7 @@ class FocusRepo( prevScreen = activeScreen } - private fun updateNavUrlInputFocus( + private fun updateNavUrlInputFocusTree( focusedNavUrlInputNode: FocusNode, sessionState: SessionRepo.State, pinnedTilesIsEmpty: Boolean, @@ -159,4 +159,65 @@ class FocusRepo( _state.onNext(newState) } + + private fun updateReloadButtonFocusTree( + focusedReloadButtonNode: FocusNode, + sessionState: SessionRepo.State) { + + assert(focusedReloadButtonNode.viewId == R.id.navButtonReload) + + val nextFocusLeftId = when { + sessionState.forwardEnabled -> R.id.navButtonForward + sessionState.backEnabled -> R.id.navButtonBack + else -> R.id.navButtonReload + } + + val newState = State( + focusNode = FocusNode( + focusedReloadButtonNode.viewId, + nextFocusLeftId = nextFocusLeftId), + defaultFocusMap = _state.value!!.defaultFocusMap) + + _state.onNext(newState) + } + + private fun updateForwardButtonFocusTree( + focusedForwardButtonNode: FocusNode, + sessionState: SessionRepo.State) { + + assert(focusedForwardButtonNode.viewId == R.id.navButtonForward) + + val nextFocusLeftId = when { + sessionState.backEnabled -> R.id.navButtonBack + else -> R.id.navButtonForward + } + + val newState = State( + focusNode = FocusNode( + focusedForwardButtonNode.viewId, + nextFocusLeftId = nextFocusLeftId), + defaultFocusMap = _state.value!!.defaultFocusMap) + + _state.onNext(newState) + } + + private fun updatePocketMegaTileFocusTree( + focusedPocketMegatTileNode: FocusNode, + pinnedTilesIsEmpty: Boolean) { + + assert(focusedPocketMegatTileNode.viewId == R.id.pocketVideoMegaTileView) + + val nextFocusDownId = when { + pinnedTilesIsEmpty -> R.id.pocketVideoMegaTileView + else -> R.id.tileContainer + } + + val newState = State( + focusNode = FocusNode( + focusedPocketMegatTileNode.viewId, + nextFocusDownId = nextFocusDownId), + defaultFocusMap = _state.value!!.defaultFocusMap) + + _state.onNext(newState) + } } From 7cb550452185c25d5f981ff4e9b6a37700ba6db5 Mon Sep 17 00:00:00 2001 From: Simon Chae Date: Tue, 23 Apr 2019 22:49:13 -0400 Subject: [PATCH 08/27] Issue #1395: Add public focusUpdate Observable --- .../org/mozilla/tv/firefox/focus/FocusRepo.kt | 48 +++++++++---------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt index 893bf8c3c0..74630b2e00 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt @@ -7,6 +7,7 @@ package org.mozilla.tv.firefox.focus import android.view.View import android.view.ViewTreeObserver import androidx.annotation.VisibleForTesting +import io.reactivex.Observable import io.reactivex.rxkotlin.Observables import io.reactivex.subjects.BehaviorSubject import org.mozilla.tv.firefox.R @@ -48,8 +49,9 @@ class FocusRepo( } } - private var _state: BehaviorSubject = BehaviorSubject.create() // TODO: potential for telemetry? - + // TODO: potential for telemetry? + private val _state: BehaviorSubject = BehaviorSubject.create() + // Keep track of prevScreen to identify screen transitions private var prevScreen: ScreenControllerStateMachine.ActiveScreen = ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY @@ -63,16 +65,16 @@ class FocusRepo( dispatchFocusUpdates(state.focusNode, activeScreen, sessionState, pinnedTilesIsEmpty, pocketState) } - val focusUpdate = _focusUpdate.hide() + val focusUpdate: Observable = _focusUpdate.hide() init { initializeDefaultFocus() } override fun onGlobalFocusChanged(oldFocus: View?, newFocus: View?) { - newFocus?.apply { + newFocus?.let { val newState = State( - focusNode = FocusNode(newFocus.id), + focusNode = FocusNode(it.id), defaultFocusMap = _state.value!!.defaultFocusMap) _state.onNext(newState) @@ -98,21 +100,22 @@ class FocusRepo( activeScreen: ScreenControllerStateMachine.ActiveScreen, sessionState: SessionRepo.State, pinnedTilesIsEmpty: Boolean, - pocketState: PocketVideoRepo.FeedState) { + pocketState: PocketVideoRepo.FeedState): State { + var newState = _state.value!! when (activeScreen) { ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY -> { when (focusNode.viewId) { R.id.navUrlInput -> - updateNavUrlInputFocusTree(focusNode, sessionState, pinnedTilesIsEmpty, pocketState) + newState = updateNavUrlInputFocusTree(focusNode, sessionState, pinnedTilesIsEmpty, pocketState) R.id.navButtonReload -> { - updateReloadButtonFocusTree(focusNode, sessionState) + newState = updateReloadButtonFocusTree(focusNode, sessionState) } R.id.navButtonForward -> { - updateForwardButtonFocusTree(focusNode, sessionState) + newState = updateForwardButtonFocusTree(focusNode, sessionState) } R.id.pocketVideoMegaTileView -> { - updatePocketMegaTileFocusTree(focusNode, pinnedTilesIsEmpty) + newState = updatePocketMegaTileFocusTree(focusNode, pinnedTilesIsEmpty) } } } @@ -121,13 +124,14 @@ class FocusRepo( ScreenControllerStateMachine.ActiveScreen.SETTINGS -> Unit } prevScreen = activeScreen + return newState } private fun updateNavUrlInputFocusTree( focusedNavUrlInputNode: FocusNode, sessionState: SessionRepo.State, pinnedTilesIsEmpty: Boolean, - pocketState: PocketVideoRepo.FeedState) { + pocketState: PocketVideoRepo.FeedState): State { assert(focusedNavUrlInputNode.viewId == R.id.navUrlInput) @@ -150,19 +154,17 @@ class FocusRepo( else -> R.id.turboButton } - val newState = State( + return State( focusNode = FocusNode( focusedNavUrlInputNode.viewId, nextFocusUpId, nextFocusDownId), defaultFocusMap = _state.value!!.defaultFocusMap) - - _state.onNext(newState) } private fun updateReloadButtonFocusTree( focusedReloadButtonNode: FocusNode, - sessionState: SessionRepo.State) { + sessionState: SessionRepo.State): State { assert(focusedReloadButtonNode.viewId == R.id.navButtonReload) @@ -172,18 +174,16 @@ class FocusRepo( else -> R.id.navButtonReload } - val newState = State( + return State( focusNode = FocusNode( focusedReloadButtonNode.viewId, nextFocusLeftId = nextFocusLeftId), defaultFocusMap = _state.value!!.defaultFocusMap) - - _state.onNext(newState) } private fun updateForwardButtonFocusTree( focusedForwardButtonNode: FocusNode, - sessionState: SessionRepo.State) { + sessionState: SessionRepo.State): State { assert(focusedForwardButtonNode.viewId == R.id.navButtonForward) @@ -192,18 +192,16 @@ class FocusRepo( else -> R.id.navButtonForward } - val newState = State( + return State( focusNode = FocusNode( focusedForwardButtonNode.viewId, nextFocusLeftId = nextFocusLeftId), defaultFocusMap = _state.value!!.defaultFocusMap) - - _state.onNext(newState) } private fun updatePocketMegaTileFocusTree( focusedPocketMegatTileNode: FocusNode, - pinnedTilesIsEmpty: Boolean) { + pinnedTilesIsEmpty: Boolean): State { assert(focusedPocketMegatTileNode.viewId == R.id.pocketVideoMegaTileView) @@ -212,12 +210,10 @@ class FocusRepo( else -> R.id.tileContainer } - val newState = State( + return State( focusNode = FocusNode( focusedPocketMegatTileNode.viewId, nextFocusDownId = nextFocusDownId), defaultFocusMap = _state.value!!.defaultFocusMap) - - _state.onNext(newState) } } From 6553178528ecc9b86917aec94a0209ed416a6e81 Mon Sep 17 00:00:00 2001 From: Simon Chae Date: Wed, 24 Apr 2019 14:26:42 -0400 Subject: [PATCH 09/27] Issue #1395: Add appropriate observables to NavigationOverlayVM --- .../tv/firefox/architecture/ViewModelFactory.kt | 3 ++- .../navigationoverlay/NavigationOverlayFragment.kt | 13 +++++++++++++ .../navigationoverlay/NavigationOverlayViewModel.kt | 13 ++++++++++++- 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/org/mozilla/tv/firefox/architecture/ViewModelFactory.kt b/app/src/main/java/org/mozilla/tv/firefox/architecture/ViewModelFactory.kt index 8965df6d75..dfa8d953b9 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/architecture/ViewModelFactory.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/architecture/ViewModelFactory.kt @@ -62,7 +62,8 @@ class ViewModelFactory( ) as T NavigationOverlayViewModel::class.java -> NavigationOverlayViewModel( - serviceLocator.sessionRepo + serviceLocator.sessionRepo, + serviceLocator.focusRepo ) as T OverlayHintViewModel::class.java -> OverlayHintViewModel( diff --git a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt index 505613a925..e37da3e794 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt @@ -30,6 +30,7 @@ import androidx.recyclerview.widget.RecyclerView import io.reactivex.disposables.CompositeDisposable import io.reactivex.disposables.Disposable import io.reactivex.rxkotlin.addTo +import kotlinx.android.synthetic.main.activity_main.* import kotlinx.android.synthetic.main.fragment_navigation_overlay_orig.navUrlInput import kotlinx.android.synthetic.main.fragment_navigation_overlay_orig.pocketVideoMegaTileView import kotlinx.android.synthetic.main.fragment_navigation_overlay_orig.settingsTileContainer @@ -144,6 +145,7 @@ class NavigationOverlayFragment : Fragment() { get() = activity?.currentFocus private lateinit var serviceLocator: ServiceLocator + private lateinit var navigationOverlayViewModel: NavigationOverlayViewModel private lateinit var toolbarViewModel: ToolbarViewModel private lateinit var pinnedTileViewModel: PinnedTileViewModel private lateinit var pocketViewModel: PocketViewModel @@ -161,6 +163,7 @@ class NavigationOverlayFragment : Fragment() { serviceLocator = context!!.serviceLocator + navigationOverlayViewModel = FirefoxViewModelProviders.of(this).get(NavigationOverlayViewModel::class.java) toolbarViewModel = FirefoxViewModelProviders.of(this).get(ToolbarViewModel::class.java) pinnedTileViewModel = FirefoxViewModelProviders.of(this).get(PinnedTileViewModel::class.java) pocketViewModel = FirefoxViewModelProviders.of(this).get(PocketViewModel::class.java) @@ -222,6 +225,8 @@ class NavigationOverlayFragment : Fragment() { registerForContextMenu(tileContainer) updateFocusableViews() + observeFocusState() + .addTo(compositeDisposable) } override fun onStart() { @@ -246,6 +251,14 @@ class NavigationOverlayFragment : Fragment() { activity!!.moveTaskToBack(true) } + private fun observeFocusState(): Disposable { + return navigationOverlayViewModel.focusUpdate + .subscribe { focusNode -> + val focusedView = rootView.findViewById(focusNode.viewId) + focusNode.updateViewNodeTree(focusedView) + } + } + private fun observePocketState(): Disposable { return pocketViewModel.state .subscribe { state -> diff --git a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayViewModel.kt b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayViewModel.kt index daa3bad5cd..cad757d28d 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayViewModel.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayViewModel.kt @@ -6,11 +6,22 @@ package org.mozilla.tv.firefox.navigationoverlay import androidx.lifecycle.LiveData import androidx.lifecycle.ViewModel +import kotlinx.android.synthetic.main.fragment_navigation_overlay_orig.* +import org.mozilla.tv.firefox.R +import org.mozilla.tv.firefox.ScreenControllerStateMachine import org.mozilla.tv.firefox.ext.map +import org.mozilla.tv.firefox.focus.FocusRepo import org.mozilla.tv.firefox.session.SessionRepo import org.mozilla.tv.firefox.utils.URLs -class NavigationOverlayViewModel(sessionRepo: SessionRepo) : ViewModel() { +class NavigationOverlayViewModel(sessionRepo: SessionRepo, focusRepo: FocusRepo) : ViewModel() { + + val defaultFocusId = focusRepo.focusUpdate.map { + it.defaultFocusMap[ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY] ?: R.id.navUrlInput + } + + val focusUpdate = focusRepo.focusUpdate.map { it.focusNode } + @Suppress("DEPRECATION") val viewIsSplit: LiveData = sessionRepo.legacyState.map { it.currentUrl != URLs.APP_URL_HOME From 861ddd629a9654ede0db66b720ba430a233d232d Mon Sep 17 00:00:00 2001 From: Simon Chae Date: Wed, 24 Apr 2019 14:35:04 -0400 Subject: [PATCH 10/27] Issue #1395: Deprecate updateFocusableViews() --- .../NavigationOverlayFragment.kt | 53 ------------------- .../navigationoverlay/ToolbarUiController.kt | 3 -- 2 files changed, 56 deletions(-) diff --git a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt index e37da3e794..a545fdb5c3 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt @@ -182,7 +182,6 @@ class NavigationOverlayFragment : Fragment() { ToolbarUiController( toolbarViewModel, ::exitFirefox, - { updateFocusableViews() }, onNavigationEvent ).onCreateView(view, viewLifecycleOwner, fragmentManager!!) @@ -224,7 +223,6 @@ class NavigationOverlayFragment : Fragment() { registerForContextMenu(tileContainer) - updateFocusableViews() observeFocusState() .addTo(compositeDisposable) } @@ -295,16 +293,13 @@ class NavigationOverlayFragment : Fragment() { megaTileTryAgainButton.setOnClickListener { _ -> pocketViewModel.update() initMegaTile() - updateFocusableViews() pocketVideoMegaTileView.requestFocus() } - updateFocusableViews() } private fun hideMegaTileError() { pocketVideosContainer.visibility = View.VISIBLE pocketErrorContainer.visibility = View.GONE - updateFocusableViews() } private fun initMegaTile() { @@ -347,7 +342,6 @@ class NavigationOverlayFragment : Fragment() { pinnedTileViewModel.getTileList().observe(viewLifecycleOwner, Observer { if (it != null) { tileAdapter.setTiles(it) - updateFocusableViews() } }) @@ -406,52 +400,6 @@ class NavigationOverlayFragment : Fragment() { ) } - private fun updateFocusableViews(focusedView: View? = currFocus) { // TODO this will be replaced when FocusRepo is introduced - val toolbarState = toolbarViewModel.state.value - - // Prevent the focus from looping to the bottom row when reaching the last - // focusable element in the top row - navButtonReload.nextFocusLeftId = when { - toolbarState?.forwardEnabled == true -> R.id.navButtonForward - toolbarState?.backEnabled == true -> R.id.navButtonBack - else -> R.id.navButtonReload - } - navButtonForward.nextFocusLeftId = when { - toolbarState?.backEnabled == true -> R.id.navButtonBack - else -> R.id.navButtonForward - } - - navUrlInput.nextFocusDownId = when { - @Suppress("DEPRECATION") - lastPocketState is PocketViewModel.State.Feed -> R.id.pocketVideoMegaTileView - @Suppress("DEPRECATION") - lastPocketState === PocketViewModel.State.Error -> R.id.megaTileTryAgainButton - tileAdapter.itemCount == 0 -> R.id.navUrlInput - else -> R.id.tileContainer - } - - navUrlInput.nextFocusUpId = when { - toolbarState?.backEnabled == true -> R.id.navButtonBack - toolbarState?.forwardEnabled == true -> R.id.navButtonForward - toolbarState?.refreshEnabled == true -> R.id.navButtonReload - toolbarState?.pinEnabled == true -> R.id.pinButton - else -> R.id.turboButton - } - - pocketVideoMegaTileView.nextFocusDownId = when { - tileAdapter.itemCount == 0 -> R.id.pocketVideoMegaTileView - else -> R.id.tileContainer - } - - // We may have lost focus when disabling the focused view above. - // This looks more complex than is necessary, but the simpler implementation - // led to problems. See the commit message for 45940fa - val isFocusLost = focusedView != null && currFocus == null - if (isFocusLost) { - navUrlInput.requestFocus() - } - } - /** * Focus may be lost if all pinned items are removed via onContextItemSelected() * FIXME: requires OverlayFragment (LifecycleOwner) -> OverlayVM -> FocusRepo @@ -464,7 +412,6 @@ class NavigationOverlayFragment : Fragment() { megaTileTryAgainButton.requestFocus() } } - updateFocusableViews() } override fun onDestroyView() { diff --git a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/ToolbarUiController.kt b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/ToolbarUiController.kt index d0ea501806..1a6de196fc 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/ToolbarUiController.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/ToolbarUiController.kt @@ -39,7 +39,6 @@ private const val WRAP_CONTENT = LinearLayout.LayoutParams.WRAP_CONTENT class ToolbarUiController( private val toolbarViewModel: ToolbarViewModel, private val exitFirefox: () -> Unit, - private val updateFocusableViews: () -> Unit, private val onNavigationEvent: (NavigationEvent, String?, InlineAutocompleteEditText.AutocompleteResult?) -> Unit ) { @@ -142,8 +141,6 @@ class ToolbarUiController( updateOverlayButtonState(it.refreshEnabled, layout.navButtonReload) updateOverlayButtonState(it.desktopModeEnabled, layout.desktopModeButton) - updateFocusableViews() - layout.pinButton.isChecked = it.pinChecked layout.desktopModeButton.isChecked = it.desktopModeChecked layout.turboButton.isChecked = it.turboChecked From a7d80096d485cd4c53ae7bd08abd49f102d34ed4 Mon Sep 17 00:00:00 2001 From: Simon Chae Date: Wed, 24 Apr 2019 14:46:20 -0400 Subject: [PATCH 11/27] Issue #1395: Add NavigationOverlay rootView reference --- .../navigationoverlay/NavigationOverlayFragment.kt | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt index a545fdb5c3..a6904c9aa0 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt @@ -153,6 +153,8 @@ class NavigationOverlayFragment : Fragment() { private lateinit var tileAdapter: PinnedTileAdapter + private lateinit var rootView: View + // TODO: remove this when FocusRepo is in place #1395 private var defaultFocusTag = NavigationOverlayFragment.FRAGMENT_TAG @Deprecated(message = "VM state should be used reactively, not imperatively. See #1395, which will fix this") @@ -185,6 +187,8 @@ class NavigationOverlayFragment : Fragment() { onNavigationEvent ).onCreateView(view, viewLifecycleOwner, fragmentManager!!) + rootView = view + // TODO: Add back in once #1666 is ready to land. /* // Handle split overlay state on homescreen or webrender @@ -222,13 +226,12 @@ class NavigationOverlayFragment : Fragment() { } registerForContextMenu(tileContainer) - - observeFocusState() - .addTo(compositeDisposable) } override fun onStart() { super.onStart() + observeFocusState() + .addTo(compositeDisposable) observePocketState() .addTo(compositeDisposable) HintBinder.bindHintsToView(hintViewModel, hintBarContainer, animate = false) From 9193a2f02240c7c17b4770f3a763eaf1f1558154 Mon Sep 17 00:00:00 2001 From: Simon Chae Date: Wed, 24 Apr 2019 15:27:39 -0400 Subject: [PATCH 12/27] Issue #1395: Fix tilesContainer isEmpty visibility focus state --- .../java/org/mozilla/tv/firefox/focus/FocusRepo.kt | 2 +- .../navigationoverlay/NavigationOverlayFragment.kt | 13 ++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt index 74630b2e00..eeb7c57bf4 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt @@ -206,7 +206,7 @@ class FocusRepo( assert(focusedPocketMegatTileNode.viewId == R.id.pocketVideoMegaTileView) val nextFocusDownId = when { - pinnedTilesIsEmpty -> R.id.pocketVideoMegaTileView + pinnedTilesIsEmpty -> R.id.settingsTileContainer else -> R.id.tileContainer } diff --git a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt index a6904c9aa0..d0dccc50fe 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt @@ -141,9 +141,6 @@ class NavigationOverlayFragment : Fragment() { Unit } - private var currFocus: View? = null - get() = activity?.currentFocus - private lateinit var serviceLocator: ServiceLocator private lateinit var navigationOverlayViewModel: NavigationOverlayViewModel private lateinit var toolbarViewModel: ToolbarViewModel @@ -255,8 +252,9 @@ class NavigationOverlayFragment : Fragment() { private fun observeFocusState(): Disposable { return navigationOverlayViewModel.focusUpdate .subscribe { focusNode -> - val focusedView = rootView.findViewById(focusNode.viewId) - focusNode.updateViewNodeTree(focusedView) + rootView.findViewById(focusNode.viewId)?.let { focusedView -> + focusNode.updateViewNodeTree(focusedView) + } } } @@ -344,6 +342,11 @@ class NavigationOverlayFragment : Fragment() { pinnedTileViewModel.getTileList().observe(viewLifecycleOwner, Observer { if (it != null) { + if (it.isEmpty()) { + tileContainer.visibility = View.GONE + } else { + tileContainer.visibility = View.VISIBLE + } tileAdapter.setTiles(it) } }) From 1684f6ef7ab113395250f003e2274af1ee99904b Mon Sep 17 00:00:00 2001 From: Simon Chae Date: Wed, 24 Apr 2019 16:31:54 -0400 Subject: [PATCH 13/27] Issue #1395: Fix PocketErrorMegatile focus issues --- .../org/mozilla/tv/firefox/MainActivity.kt | 1 - .../org/mozilla/tv/firefox/focus/FocusRepo.kt | 30 ++++++++++------ .../NavigationOverlayFragment.kt | 36 ++++--------------- 3 files changed, 26 insertions(+), 41 deletions(-) diff --git a/app/src/main/java/org/mozilla/tv/firefox/MainActivity.kt b/app/src/main/java/org/mozilla/tv/firefox/MainActivity.kt index 5363c43794..4442dbde8c 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/MainActivity.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/MainActivity.kt @@ -13,7 +13,6 @@ import android.util.AttributeSet import android.util.Log import android.view.KeyEvent import android.view.View -import android.view.ViewTreeObserver import androidx.lifecycle.Observer import io.sentry.Sentry import kotlinx.android.synthetic.main.activity_main.* diff --git a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt index eeb7c57bf4..02036e5bdb 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt @@ -22,7 +22,8 @@ class FocusRepo( screenController: ScreenController, sessionRepo: SessionRepo, pinnedTileRepo: PinnedTileRepo, - pocketRepo: PocketVideoRepo): ViewTreeObserver.OnGlobalFocusChangeListener { + pocketRepo: PocketVideoRepo +) : ViewTreeObserver.OnGlobalFocusChangeListener { data class State( val focusNode: FocusNode, @@ -51,7 +52,7 @@ class FocusRepo( // TODO: potential for telemetry? private val _state: BehaviorSubject = BehaviorSubject.create() - + // Keep track of prevScreen to identify screen transitions private var prevScreen: ScreenControllerStateMachine.ActiveScreen = ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY @@ -100,7 +101,8 @@ class FocusRepo( activeScreen: ScreenControllerStateMachine.ActiveScreen, sessionState: SessionRepo.State, pinnedTilesIsEmpty: Boolean, - pocketState: PocketVideoRepo.FeedState): State { + pocketState: PocketVideoRepo.FeedState + ): State { var newState = _state.value!! when (activeScreen) { @@ -117,6 +119,9 @@ class FocusRepo( R.id.pocketVideoMegaTileView -> { newState = updatePocketMegaTileFocusTree(focusNode, pinnedTilesIsEmpty) } + R.id.megaTileTryAgainButton -> { + newState = updatePocketMegaTileFocusTree(focusNode, pinnedTilesIsEmpty) + } } } ScreenControllerStateMachine.ActiveScreen.WEB_RENDER -> Unit @@ -131,7 +136,8 @@ class FocusRepo( focusedNavUrlInputNode: FocusNode, sessionState: SessionRepo.State, pinnedTilesIsEmpty: Boolean, - pocketState: PocketVideoRepo.FeedState): State { + pocketState: PocketVideoRepo.FeedState + ): State { assert(focusedNavUrlInputNode.viewId == R.id.navUrlInput) @@ -139,7 +145,7 @@ class FocusRepo( pocketState is PocketVideoRepo.FeedState.FetchFailed -> R.id.megaTileTryAgainButton pocketState is PocketVideoRepo.FeedState.Inactive -> { if (pinnedTilesIsEmpty) { - R.id.navUrlInput // if + R.id.navUrlInput } else { R.id.tileContainer } @@ -164,7 +170,8 @@ class FocusRepo( private fun updateReloadButtonFocusTree( focusedReloadButtonNode: FocusNode, - sessionState: SessionRepo.State): State { + sessionState: SessionRepo.State + ): State { assert(focusedReloadButtonNode.viewId == R.id.navButtonReload) @@ -183,7 +190,8 @@ class FocusRepo( private fun updateForwardButtonFocusTree( focusedForwardButtonNode: FocusNode, - sessionState: SessionRepo.State): State { + sessionState: SessionRepo.State + ): State { assert(focusedForwardButtonNode.viewId == R.id.navButtonForward) @@ -201,16 +209,18 @@ class FocusRepo( private fun updatePocketMegaTileFocusTree( focusedPocketMegatTileNode: FocusNode, - pinnedTilesIsEmpty: Boolean): State { + pinnedTilesIsEmpty: Boolean + ): State { - assert(focusedPocketMegatTileNode.viewId == R.id.pocketVideoMegaTileView) + assert(focusedPocketMegatTileNode.viewId == R.id.pocketVideoMegaTileView || + focusedPocketMegatTileNode.viewId == R.id.megaTileTryAgainButton) val nextFocusDownId = when { pinnedTilesIsEmpty -> R.id.settingsTileContainer else -> R.id.tileContainer } - return State( + return State( focusNode = FocusNode( focusedPocketMegatTileNode.viewId, nextFocusDownId = nextFocusDownId), diff --git a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt index d0dccc50fe..9a14e061d6 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt @@ -25,19 +25,14 @@ import androidx.core.view.updateLayoutParams import androidx.core.widget.NestedScrollView import androidx.fragment.app.Fragment import androidx.lifecycle.Observer -import androidx.recyclerview.widget.GridLayoutManager -import androidx.recyclerview.widget.RecyclerView import io.reactivex.disposables.CompositeDisposable import io.reactivex.disposables.Disposable import io.reactivex.rxkotlin.addTo -import kotlinx.android.synthetic.main.activity_main.* import kotlinx.android.synthetic.main.fragment_navigation_overlay_orig.navUrlInput import kotlinx.android.synthetic.main.fragment_navigation_overlay_orig.pocketVideoMegaTileView import kotlinx.android.synthetic.main.fragment_navigation_overlay_orig.settingsTileContainer import kotlinx.android.synthetic.main.fragment_navigation_overlay_orig.tileContainer import kotlinx.android.synthetic.main.fragment_navigation_overlay_top_nav.exitButton -import kotlinx.android.synthetic.main.fragment_navigation_overlay_top_nav.navButtonForward -import kotlinx.android.synthetic.main.fragment_navigation_overlay_top_nav.navButtonReload import kotlinx.android.synthetic.main.hint_bar.hintBarContainer import kotlinx.android.synthetic.main.pocket_video_mega_tile.megaTileTryAgainButton import kotlinx.android.synthetic.main.pocket_video_mega_tile.pocketErrorContainer @@ -50,7 +45,6 @@ import org.mozilla.tv.firefox.architecture.FirefoxViewModelProviders import org.mozilla.tv.firefox.architecture.FocusOnShowDelegate import org.mozilla.tv.firefox.experiments.ExperimentConfig import org.mozilla.tv.firefox.ext.forceExhaustive -import org.mozilla.tv.firefox.ext.isEffectivelyVisible import org.mozilla.tv.firefox.ext.isVoiceViewEnabled import org.mozilla.tv.firefox.ext.serviceLocator import org.mozilla.tv.firefox.hint.HintBinder @@ -286,6 +280,9 @@ class NavigationOverlayFragment : Fragment() { pocketVideosContainer.visibility = View.GONE pocketErrorContainer.visibility = View.VISIBLE + // View.focusable = INT only available Android API > 26 :( + pocketVideoMegaTileView.setFocusable(false) + pocketMegaTileLoadError.text = resources.getString(R.string.pocket_video_feed_failed_to_load, resources.getString(R.string.pocket_brand_name)) megaTileTryAgainButton.contentDescription = resources.getString(R.string.pocket_video_feed_failed_to_load, @@ -301,6 +298,9 @@ class NavigationOverlayFragment : Fragment() { private fun hideMegaTileError() { pocketVideosContainer.visibility = View.VISIBLE pocketErrorContainer.visibility = View.GONE + + // View.focusable = INT only available Android API > 26 :( + pocketVideoMegaTileView.setFocusable(true) } private fun initMegaTile() { @@ -353,8 +353,6 @@ class NavigationOverlayFragment : Fragment() { adapter = tileAdapter - layoutManager = HomeTileManager(context, COL_COUNT) - setHasFixedSize(true) // We add bottomMargin to each tile in order to add spacing between them: this makes the @@ -428,28 +426,6 @@ class NavigationOverlayFragment : Fragment() { // but it'll add complexity that I don't think is probably worth it. uiLifecycleCancelJob.cancel() } - - inner class HomeTileManager( - context: Context, - colCount: Int - ) : GridLayoutManager(context, colCount) { - override fun onRequestChildFocus(parent: RecyclerView, state: RecyclerView.State, child: View, focused: View?): Boolean { - var position = spanCount - if (focused != null) { - position = getPosition(focused) - } - - // if position is less than spanCount, implies first row - if (position < spanCount) { - focused?.nextFocusUpId = when { - pocketVideosContainer.isEffectivelyVisible -> R.id.pocketVideoMegaTileView - megaTileTryAgainButton.isEffectivelyVisible -> R.id.megaTileTryAgainButton - else -> R.id.navUrlInput - } - } - return super.onRequestChildFocus(parent, state, child, focused) - } - } } /** From b38e25c599b73546219ac98710999c5289f539cb Mon Sep 17 00:00:00 2001 From: Simon Chae Date: Wed, 24 Apr 2019 17:08:43 -0400 Subject: [PATCH 14/27] Issue #1395: Update tilesContainer isEmpty case with Rx --- .../NavigationOverlayFragment.kt | 20 ++++++++++++++----- .../firefox/pinnedtile/PinnedTileViewModel.kt | 3 +-- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt index 9a14e061d6..f3407ab690 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt @@ -25,6 +25,7 @@ import androidx.core.view.updateLayoutParams import androidx.core.widget.NestedScrollView import androidx.fragment.app.Fragment import androidx.lifecycle.Observer +import androidx.recyclerview.widget.GridLayoutManager import io.reactivex.disposables.CompositeDisposable import io.reactivex.disposables.Disposable import io.reactivex.rxkotlin.addTo @@ -223,6 +224,8 @@ class NavigationOverlayFragment : Fragment() { super.onStart() observeFocusState() .addTo(compositeDisposable) + observeTilesContainer() + .addTo(compositeDisposable) observePocketState() .addTo(compositeDisposable) HintBinder.bindHintsToView(hintViewModel, hintBarContainer, animate = false) @@ -252,6 +255,16 @@ class NavigationOverlayFragment : Fragment() { } } + private fun observeTilesContainer(): Disposable { + return pinnedTileViewModel.isEmpty.subscribe { isEmpty -> + if (isEmpty) { + tileContainer.visibility = View.GONE + } else { + tileContainer.visibility = View.VISIBLE + } + } + } + private fun observePocketState(): Disposable { return pocketViewModel.state .subscribe { state -> @@ -342,17 +355,14 @@ class NavigationOverlayFragment : Fragment() { pinnedTileViewModel.getTileList().observe(viewLifecycleOwner, Observer { if (it != null) { - if (it.isEmpty()) { - tileContainer.visibility = View.GONE - } else { - tileContainer.visibility = View.VISIBLE - } tileAdapter.setTiles(it) } }) adapter = tileAdapter + layoutManager = GridLayoutManager(context, COL_COUNT) + setHasFixedSize(true) // We add bottomMargin to each tile in order to add spacing between them: this makes the diff --git a/app/src/main/java/org/mozilla/tv/firefox/pinnedtile/PinnedTileViewModel.kt b/app/src/main/java/org/mozilla/tv/firefox/pinnedtile/PinnedTileViewModel.kt index a33ab89732..592fd82009 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/pinnedtile/PinnedTileViewModel.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/pinnedtile/PinnedTileViewModel.kt @@ -6,7 +6,6 @@ package org.mozilla.tv.firefox.pinnedtile import androidx.lifecycle.LiveData import androidx.lifecycle.MediatorLiveData -import androidx.lifecycle.Transformations import androidx.lifecycle.ViewModel /** @@ -19,7 +18,7 @@ import androidx.lifecycle.ViewModel class PinnedTileViewModel(private val pinnedTileRepo: PinnedTileRepo) : ViewModel() { private val _tilesList = MediatorLiveData>() - val isEmpty: LiveData = Transformations.map(_tilesList) { input -> input.isEmpty() } + val isEmpty = pinnedTileRepo.isEmpty init { @Suppress("DEPRECATION") From 5ebbfccf0248a7d296bd7714744517eba0a8adee Mon Sep 17 00:00:00 2001 From: Simon Chae Date: Thu, 25 Apr 2019 10:08:46 -0400 Subject: [PATCH 15/27] Issue #1395: Add FocusRepo.Event --- .../org/mozilla/tv/firefox/focus/FocusRepo.kt | 41 +++++++++++++++++-- 1 file changed, 38 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt index 02036e5bdb..5e8d6659b3 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt @@ -10,6 +10,8 @@ import androidx.annotation.VisibleForTesting import io.reactivex.Observable import io.reactivex.rxkotlin.Observables import io.reactivex.subjects.BehaviorSubject +import io.reactivex.subjects.PublishSubject +import io.reactivex.subjects.Subject import org.mozilla.tv.firefox.R import org.mozilla.tv.firefox.ScreenController import org.mozilla.tv.firefox.ScreenControllerStateMachine @@ -30,6 +32,10 @@ class FocusRepo( val defaultFocusMap: HashMap ) + enum class Event { + ScreenChange, RequestFocus + } + /** * FocusNode describes quasi-directional focusable paths given viewId */ @@ -53,6 +59,9 @@ class FocusRepo( // TODO: potential for telemetry? private val _state: BehaviorSubject = BehaviorSubject.create() + private val _events: Subject = PublishSubject.create() + val events: Observable = _events.hide() + // Keep track of prevScreen to identify screen transitions private var prevScreen: ScreenControllerStateMachine.ActiveScreen = ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY @@ -124,11 +133,26 @@ class FocusRepo( } } } - ScreenControllerStateMachine.ActiveScreen.WEB_RENDER -> Unit - ScreenControllerStateMachine.ActiveScreen.POCKET -> Unit + ScreenControllerStateMachine.ActiveScreen.WEB_RENDER -> { + if (prevScreen == ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY) { + + } + } + ScreenControllerStateMachine.ActiveScreen.POCKET -> { + if (prevScreen == ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY) { + val focusMap = _state.value!!.defaultFocusMap + // FIXME: verify if a new state needs to be returned (see Event) + newState = updateDefaultFocusForOverlayWhenTransitioningToPocket(focusMap) + } + } ScreenControllerStateMachine.ActiveScreen.SETTINGS -> Unit } - prevScreen = activeScreen + + if (prevScreen != activeScreen) { + _events.onNext(Event.ScreenChange) + prevScreen = activeScreen + } + return newState } @@ -226,4 +250,15 @@ class FocusRepo( nextFocusDownId = nextFocusDownId), defaultFocusMap = _state.value!!.defaultFocusMap) } + + private fun updateDefaultFocusForOverlayWhenTransitioningToPocket( + focusMap: HashMap + ): State { + focusMap[ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY] = + R.id.pocketVideoMegaTileView + + return State( + focusNode = _state.value!!.focusNode, + defaultFocusMap = focusMap) + } } From c697068df4fe90f2f101fa034dce1f1ee4a7dd33 Mon Sep 17 00:00:00 2001 From: Simon Chae Date: Thu, 25 Apr 2019 11:05:18 -0400 Subject: [PATCH 16/27] Issue #1395: Add transition to Pocket default focus for Overlay --- .../org/mozilla/tv/firefox/focus/FocusRepo.kt | 35 ++++++++++++++---- .../NavigationOverlayFragment.kt | 37 ++++++++++--------- .../NavigationOverlayViewModel.kt | 18 +++++++-- 3 files changed, 60 insertions(+), 30 deletions(-) diff --git a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt index 5e8d6659b3..9444aafbd6 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt @@ -28,12 +28,14 @@ class FocusRepo( ) : ViewTreeObserver.OnGlobalFocusChangeListener { data class State( + val activeScreen: ScreenControllerStateMachine.ActiveScreen, val focusNode: FocusNode, val defaultFocusMap: HashMap ) enum class Event { - ScreenChange, RequestFocus + ScreenChange, + RequestFocus // to handle lost focus } /** @@ -84,6 +86,7 @@ class FocusRepo( override fun onGlobalFocusChanged(oldFocus: View?, newFocus: View?) { newFocus?.let { val newState = State( + activeScreen = _state.value!!.activeScreen, focusNode = FocusNode(it.id), defaultFocusMap = _state.value!!.defaultFocusMap) @@ -98,6 +101,7 @@ class FocusRepo( focusMap[ScreenControllerStateMachine.ActiveScreen.POCKET] = R.id.videoFeed val newState = State( + activeScreen = ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY, focusNode = FocusNode(R.id.navUrlInput), defaultFocusMap = focusMap) @@ -118,31 +122,36 @@ class FocusRepo( ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY -> { when (focusNode.viewId) { R.id.navUrlInput -> - newState = updateNavUrlInputFocusTree(focusNode, sessionState, pinnedTilesIsEmpty, pocketState) + newState = updateNavUrlInputFocusTree( + activeScreen, + focusNode, + sessionState, + pinnedTilesIsEmpty, + pocketState) R.id.navButtonReload -> { - newState = updateReloadButtonFocusTree(focusNode, sessionState) + newState = updateReloadButtonFocusTree(activeScreen, focusNode, sessionState) } R.id.navButtonForward -> { - newState = updateForwardButtonFocusTree(focusNode, sessionState) + newState = updateForwardButtonFocusTree(activeScreen, focusNode, sessionState) } R.id.pocketVideoMegaTileView -> { - newState = updatePocketMegaTileFocusTree(focusNode, pinnedTilesIsEmpty) + newState = updatePocketMegaTileFocusTree(activeScreen, focusNode, pinnedTilesIsEmpty) } R.id.megaTileTryAgainButton -> { - newState = updatePocketMegaTileFocusTree(focusNode, pinnedTilesIsEmpty) + newState = updatePocketMegaTileFocusTree(activeScreen, focusNode, pinnedTilesIsEmpty) } } } ScreenControllerStateMachine.ActiveScreen.WEB_RENDER -> { if (prevScreen == ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY) { - + // TODO } } ScreenControllerStateMachine.ActiveScreen.POCKET -> { if (prevScreen == ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY) { val focusMap = _state.value!!.defaultFocusMap // FIXME: verify if a new state needs to be returned (see Event) - newState = updateDefaultFocusForOverlayWhenTransitioningToPocket(focusMap) + newState = updateDefaultFocusForOverlayWhenTransitioningToPocket(activeScreen, focusMap) } } ScreenControllerStateMachine.ActiveScreen.SETTINGS -> Unit @@ -157,6 +166,7 @@ class FocusRepo( } private fun updateNavUrlInputFocusTree( + activeScreen: ScreenControllerStateMachine.ActiveScreen, focusedNavUrlInputNode: FocusNode, sessionState: SessionRepo.State, pinnedTilesIsEmpty: Boolean, @@ -185,6 +195,7 @@ class FocusRepo( } return State( + activeScreen = activeScreen, focusNode = FocusNode( focusedNavUrlInputNode.viewId, nextFocusUpId, @@ -193,6 +204,7 @@ class FocusRepo( } private fun updateReloadButtonFocusTree( + activeScreen: ScreenControllerStateMachine.ActiveScreen, focusedReloadButtonNode: FocusNode, sessionState: SessionRepo.State ): State { @@ -206,6 +218,7 @@ class FocusRepo( } return State( + activeScreen = activeScreen, focusNode = FocusNode( focusedReloadButtonNode.viewId, nextFocusLeftId = nextFocusLeftId), @@ -213,6 +226,7 @@ class FocusRepo( } private fun updateForwardButtonFocusTree( + activeScreen: ScreenControllerStateMachine.ActiveScreen, focusedForwardButtonNode: FocusNode, sessionState: SessionRepo.State ): State { @@ -225,6 +239,7 @@ class FocusRepo( } return State( + activeScreen = activeScreen, focusNode = FocusNode( focusedForwardButtonNode.viewId, nextFocusLeftId = nextFocusLeftId), @@ -232,6 +247,7 @@ class FocusRepo( } private fun updatePocketMegaTileFocusTree( + activeScreen: ScreenControllerStateMachine.ActiveScreen, focusedPocketMegatTileNode: FocusNode, pinnedTilesIsEmpty: Boolean ): State { @@ -245,6 +261,7 @@ class FocusRepo( } return State( + activeScreen = activeScreen, focusNode = FocusNode( focusedPocketMegatTileNode.viewId, nextFocusDownId = nextFocusDownId), @@ -252,12 +269,14 @@ class FocusRepo( } private fun updateDefaultFocusForOverlayWhenTransitioningToPocket( + activeScreen: ScreenControllerStateMachine.ActiveScreen, focusMap: HashMap ): State { focusMap[ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY] = R.id.pocketVideoMegaTileView return State( + activeScreen = activeScreen, focusNode = _state.value!!.focusNode, defaultFocusMap = focusMap) } diff --git a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt index f3407ab690..8bdaa2d1bf 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt @@ -43,7 +43,6 @@ import kotlinx.coroutines.Job import org.mozilla.tv.firefox.MainActivity import org.mozilla.tv.firefox.R import org.mozilla.tv.firefox.architecture.FirefoxViewModelProviders -import org.mozilla.tv.firefox.architecture.FocusOnShowDelegate import org.mozilla.tv.firefox.experiments.ExperimentConfig import org.mozilla.tv.firefox.ext.forceExhaustive import org.mozilla.tv.firefox.ext.isVoiceViewEnabled @@ -55,7 +54,6 @@ import org.mozilla.tv.firefox.navigationoverlay.channels.SettingsChannelAdapter import org.mozilla.tv.firefox.navigationoverlay.channels.SettingsScreen import org.mozilla.tv.firefox.pinnedtile.PinnedTileAdapter import org.mozilla.tv.firefox.pinnedtile.PinnedTileViewModel -import org.mozilla.tv.firefox.pocket.PocketVideoFragment import org.mozilla.tv.firefox.pocket.PocketViewModel import org.mozilla.tv.firefox.telemetry.TelemetryIntegration import org.mozilla.tv.firefox.telemetry.UrlTextInputLocation @@ -147,8 +145,6 @@ class NavigationOverlayFragment : Fragment() { private lateinit var rootView: View - // TODO: remove this when FocusRepo is in place #1395 - private var defaultFocusTag = NavigationOverlayFragment.FRAGMENT_TAG @Deprecated(message = "VM state should be used reactively, not imperatively. See #1395, which will fix this") private var lastPocketState: PocketViewModel.State? = null @@ -209,23 +205,25 @@ class NavigationOverlayFragment : Fragment() { navUrlInput.compoundDrawablesRelative.forEach(tintDrawable) // TODO: remove this when FocusRepo is in place #1395 - when (defaultFocusTag) { - PocketVideoFragment.FRAGMENT_TAG -> { - pocketVideoMegaTileView.requestFocus() - defaultFocusTag = NavigationOverlayFragment.FRAGMENT_TAG - } - NavigationOverlayFragment.FRAGMENT_TAG -> navUrlInput.requestFocus() - } +// when (defaultFocusTag) { +// PocketVideoFragment.FRAGMENT_TAG -> { +// pocketVideoMegaTileView.requestFocus() +// defaultFocusTag = NavigationOverlayFragment.FRAGMENT_TAG +// } +// NavigationOverlayFragment.FRAGMENT_TAG -> navUrlInput.requestFocus() +// } registerForContextMenu(tileContainer) } override fun onStart() { super.onStart() + observeRequestFocus() + .addTo(compositeDisposable) observeFocusState() - .addTo(compositeDisposable) + .addTo(compositeDisposable) observeTilesContainer() - .addTo(compositeDisposable) + .addTo(compositeDisposable) observePocketState() .addTo(compositeDisposable) HintBinder.bindHintsToView(hintViewModel, hintBarContainer, animate = false) @@ -237,15 +235,18 @@ class NavigationOverlayFragment : Fragment() { compositeDisposable.clear() } - override fun onHiddenChanged(hidden: Boolean) { - FocusOnShowDelegate().onHiddenChanged(this, hidden) - super.onHiddenChanged(hidden) - } - private fun exitFirefox() { activity!!.moveTaskToBack(true) } + private fun observeRequestFocus(): Disposable { + return navigationOverlayViewModel.focusRequest + .subscribe { viewId -> + val viewToFocus = rootView.findViewById(viewId) + viewToFocus?.requestFocus() + } + } + private fun observeFocusState(): Disposable { return navigationOverlayViewModel.focusUpdate .subscribe { focusNode -> diff --git a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayViewModel.kt b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayViewModel.kt index cad757d28d..df2a135e59 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayViewModel.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayViewModel.kt @@ -6,7 +6,8 @@ package org.mozilla.tv.firefox.navigationoverlay import androidx.lifecycle.LiveData import androidx.lifecycle.ViewModel -import kotlinx.android.synthetic.main.fragment_navigation_overlay_orig.* +import io.reactivex.Observable +import io.reactivex.rxkotlin.withLatestFrom import org.mozilla.tv.firefox.R import org.mozilla.tv.firefox.ScreenControllerStateMachine import org.mozilla.tv.firefox.ext.map @@ -16,11 +17,20 @@ import org.mozilla.tv.firefox.utils.URLs class NavigationOverlayViewModel(sessionRepo: SessionRepo, focusRepo: FocusRepo) : ViewModel() { - val defaultFocusId = focusRepo.focusUpdate.map { - it.defaultFocusMap[ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY] ?: R.id.navUrlInput + val focusRequest: Observable = focusRepo.events.withLatestFrom(focusRepo.focusUpdate) + .filter { (_, state) -> + state.activeScreen == ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY + } + .map { (event, state) -> + when (event) { + FocusRepo.Event.ScreenChange -> + state.defaultFocusMap[state.activeScreen] ?: R.id.navUrlInput + FocusRepo.Event.RequestFocus -> + state.focusNode.viewId + } } - val focusUpdate = focusRepo.focusUpdate.map { it.focusNode } + val focusUpdate: Observable = focusRepo.focusUpdate.map { it.focusNode } @Suppress("DEPRECATION") val viewIsSplit: LiveData = sessionRepo.legacyState.map { From d1d0ff5f68d3340245644354683451ed19417d10 Mon Sep 17 00:00:00 2001 From: Simon Chae Date: Thu, 25 Apr 2019 11:14:00 -0400 Subject: [PATCH 17/27] Issue #1395: Add transition to WebRender default focus for Overlay --- .../mozilla/tv/firefox/ScreenController.kt | 2 -- .../org/mozilla/tv/firefox/focus/FocusRepo.kt | 29 +++++++++++++++++-- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/org/mozilla/tv/firefox/ScreenController.kt b/app/src/main/java/org/mozilla/tv/firefox/ScreenController.kt index f83711d05e..b5374ea733 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/ScreenController.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/ScreenController.kt @@ -11,7 +11,6 @@ import android.view.KeyEvent import androidx.fragment.app.FragmentManager import io.reactivex.Observable import io.reactivex.subjects.BehaviorSubject -import kotlinx.android.synthetic.main.fragment_navigation_overlay.navUrlInput import mozilla.components.browser.session.Session import org.mozilla.tv.firefox.ScreenControllerStateMachine.ActiveScreen import org.mozilla.tv.firefox.ScreenControllerStateMachine.Transition @@ -143,7 +142,6 @@ class ScreenController(private val sessionRepo: SessionRepo) { transaction.show(overlayFragment) MenuInteractionMonitor.menuOpened() - overlayFragment.navUrlInput.requestFocus() // TODO: Disabled until Overlay refactor is complete #1666 // overlayFragment.navOverlayScrollView.updateOverlayForHomescreen(isOnHomeUrl(fragmentManager)) } else { diff --git a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt index 9444aafbd6..71073b4c6f 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt @@ -118,6 +118,7 @@ class FocusRepo( ): State { var newState = _state.value!! + val focusMap = _state.value!!.defaultFocusMap when (activeScreen) { ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY -> { when (focusNode.viewId) { @@ -144,13 +145,14 @@ class FocusRepo( } ScreenControllerStateMachine.ActiveScreen.WEB_RENDER -> { if (prevScreen == ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY) { - // TODO + newState = updateDefaultFocusForOverlayWhenTransitioningToWebRender( + activeScreen, + focusMap, + sessionState) } } ScreenControllerStateMachine.ActiveScreen.POCKET -> { if (prevScreen == ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY) { - val focusMap = _state.value!!.defaultFocusMap - // FIXME: verify if a new state needs to be returned (see Event) newState = updateDefaultFocusForOverlayWhenTransitioningToPocket(activeScreen, focusMap) } } @@ -268,6 +270,27 @@ class FocusRepo( defaultFocusMap = _state.value!!.defaultFocusMap) } + private fun updateDefaultFocusForOverlayWhenTransitioningToWebRender( + activeScreen: ScreenControllerStateMachine.ActiveScreen, + focusMap: HashMap, + sessionState: SessionRepo.State + ): State { + + // It doesn't make sense to be able to transition to WebRender if currUrl == APP_URL_HOME + assert(sessionState.currentUrl != URLs.APP_URL_HOME) + + focusMap[ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY] = when { + sessionState.backEnabled -> R.id.navButtonBack + sessionState.forwardEnabled -> R.id.navButtonForward + else -> R.id.navButtonReload + } + + return State( + activeScreen = activeScreen, + focusNode = _state.value!!.focusNode, + defaultFocusMap = focusMap) + } + private fun updateDefaultFocusForOverlayWhenTransitioningToPocket( activeScreen: ScreenControllerStateMachine.ActiveScreen, focusMap: HashMap From 1cf39010df569ccccf78c1ccd304ff2f7ce06752 Mon Sep 17 00:00:00 2001 From: Simon Chae Date: Thu, 25 Apr 2019 11:42:21 -0400 Subject: [PATCH 18/27] Issue #1395: Handle default focus for Overlay --- app/src/main/res/layout/fragment_navigation_overlay_orig.xml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/src/main/res/layout/fragment_navigation_overlay_orig.xml b/app/src/main/res/layout/fragment_navigation_overlay_orig.xml index df24610409..4a6e7720b4 100644 --- a/app/src/main/res/layout/fragment_navigation_overlay_orig.xml +++ b/app/src/main/res/layout/fragment_navigation_overlay_orig.xml @@ -78,7 +78,9 @@ android:textColorHint="@color/photonGrey10_a80p" android:textIsSelectable="false" android:fontFamily="@string/font_ember_regular" - tools:ignore="UnusedAttribute" /> + tools:ignore="UnusedAttribute" > + + Date: Thu, 25 Apr 2019 12:44:16 -0400 Subject: [PATCH 19/27] Issue #1395: Move defaultFocusMap update to when NavOverlay is active --- .../org/mozilla/tv/firefox/focus/FocusRepo.kt | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt index 71073b4c6f..49e567413c 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt @@ -121,6 +121,21 @@ class FocusRepo( val focusMap = _state.value!!.defaultFocusMap when (activeScreen) { ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY -> { + + // Check previous screen for defaultFocusMap updates + when (prevScreen) { + ScreenControllerStateMachine.ActiveScreen.WEB_RENDER -> { + newState = updateDefaultFocusForOverlayWhenTransitioningFromWebRender( + activeScreen, + focusMap, + sessionState) + } + ScreenControllerStateMachine.ActiveScreen.POCKET -> { + newState = updateDefaultFocusForOverlayWhenTransitioningFromPocket(activeScreen, focusMap) + } + else -> Unit + } + when (focusNode.viewId) { R.id.navUrlInput -> newState = updateNavUrlInputFocusTree( @@ -143,19 +158,8 @@ class FocusRepo( } } } - ScreenControllerStateMachine.ActiveScreen.WEB_RENDER -> { - if (prevScreen == ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY) { - newState = updateDefaultFocusForOverlayWhenTransitioningToWebRender( - activeScreen, - focusMap, - sessionState) - } - } - ScreenControllerStateMachine.ActiveScreen.POCKET -> { - if (prevScreen == ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY) { - newState = updateDefaultFocusForOverlayWhenTransitioningToPocket(activeScreen, focusMap) - } - } + ScreenControllerStateMachine.ActiveScreen.WEB_RENDER -> {} + ScreenControllerStateMachine.ActiveScreen.POCKET -> {} ScreenControllerStateMachine.ActiveScreen.SETTINGS -> Unit } @@ -270,7 +274,7 @@ class FocusRepo( defaultFocusMap = _state.value!!.defaultFocusMap) } - private fun updateDefaultFocusForOverlayWhenTransitioningToWebRender( + private fun updateDefaultFocusForOverlayWhenTransitioningFromWebRender( activeScreen: ScreenControllerStateMachine.ActiveScreen, focusMap: HashMap, sessionState: SessionRepo.State @@ -291,7 +295,7 @@ class FocusRepo( defaultFocusMap = focusMap) } - private fun updateDefaultFocusForOverlayWhenTransitioningToPocket( + private fun updateDefaultFocusForOverlayWhenTransitioningFromPocket( activeScreen: ScreenControllerStateMachine.ActiveScreen, focusMap: HashMap ): State { From c0eafeeb0de5bfe0a2505013b9f49a158b330866 Mon Sep 17 00:00:00 2001 From: Simon Chae Date: Thu, 25 Apr 2019 13:15:45 -0400 Subject: [PATCH 20/27] Issue #1395: Handle checkIfTilesFocusNeedRefresh() in FocusRepo --- .../org/mozilla/tv/firefox/focus/FocusRepo.kt | 44 +++++++++++++++++++ .../NavigationOverlayFragment.kt | 16 ------- 2 files changed, 44 insertions(+), 16 deletions(-) diff --git a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt index 49e567413c..e008e53e59 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt @@ -156,6 +156,18 @@ class FocusRepo( R.id.megaTileTryAgainButton -> { newState = updatePocketMegaTileFocusTree(activeScreen, focusNode, pinnedTilesIsEmpty) } + R.id.home_tile -> { + // If pinnedTiles is empty and current focus is on home_tile, we need to + // restore lost focus (this happens when you remove all tiles in the overlay) + if (pinnedTilesIsEmpty) { + newState = handleEmptyTilesLostFocus( + activeScreen, + focusNode, + sessionState, + pinnedTilesIsEmpty, + pocketState) + } + } } } ScreenControllerStateMachine.ActiveScreen.WEB_RENDER -> {} @@ -274,6 +286,38 @@ class FocusRepo( defaultFocusMap = _state.value!!.defaultFocusMap) } + /** + * When all the pinned tiles are removed, tilContainer no longer needs focus + */ + private fun handleEmptyTilesLostFocus( + activeScreen: ScreenControllerStateMachine.ActiveScreen, + tileContainerFocusNode: FocusNode, + sessionState: SessionRepo.State, + pinnedTilesIsEmpty: Boolean, + pocketState: PocketVideoRepo.FeedState + ): State { + + assert(tileContainerFocusNode.viewId == R.id.tileContainer) + + val viewId = when { + pocketState == PocketVideoRepo.FeedState.FetchFailed -> R.id.megaTileTryAgainButton + pocketState == PocketVideoRepo.FeedState.Inactive -> R.id.navUrlInput + else -> R.id.pocketVideoMegaTileView + } + + val newFocusNode = FocusNode(viewId) + val newState = if (newFocusNode.viewId == R.id.navUrlInput) { + updateNavUrlInputFocusTree(activeScreen, newFocusNode, sessionState, pinnedTilesIsEmpty, pocketState) + } else { + updatePocketMegaTileFocusTree(activeScreen, newFocusNode, pinnedTilesIsEmpty) + } + + // Request focus on newState + _events.onNext(Event.RequestFocus) + + return newState + } + private fun updateDefaultFocusForOverlayWhenTransitioningFromWebRender( activeScreen: ScreenControllerStateMachine.ActiveScreen, focusMap: HashMap, diff --git a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt index 8bdaa2d1bf..26fa037ed3 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt @@ -20,7 +20,6 @@ import android.view.ViewGroup import android.widget.ScrollView import android.widget.Toast import androidx.core.content.ContextCompat -import androidx.core.view.isVisible import androidx.core.view.updateLayoutParams import androidx.core.widget.NestedScrollView import androidx.fragment.app.Fragment @@ -392,7 +391,6 @@ class NavigationOverlayFragment : Fragment() { // that the Uri is valid, so we do not do error handling here. // TODO: NavigationOverlayFragment->ViewModel->Repo pinnedTileViewModel.unpin(tileToRemove.url) - checkIfTilesFocusNeedRefresh() TelemetryIntegration.INSTANCE.homeTileRemovedEvent(tileToRemove) return true } @@ -415,20 +413,6 @@ class NavigationOverlayFragment : Fragment() { ) } - /** - * Focus may be lost if all pinned items are removed via onContextItemSelected() - * FIXME: requires OverlayFragment (LifecycleOwner) -> OverlayVM -> FocusRepo - */ - private fun checkIfTilesFocusNeedRefresh() { - if (tileAdapter.itemCount == 0) { - if (pocketVideosContainer.isVisible) { - pocketVideoMegaTileView.requestFocus() - } else { - megaTileTryAgainButton.requestFocus() - } - } - } - override fun onDestroyView() { super.onDestroyView() From 40209663ce4b0d0a7699a8a9e46f2a9850c37e1d Mon Sep 17 00:00:00 2001 From: Simon Chae Date: Thu, 25 Apr 2019 15:49:55 -0400 Subject: [PATCH 21/27] Issue #1395: Handle PocketTryAgain lost focus --- .../org/mozilla/tv/firefox/focus/FocusRepo.kt | 30 ++++++++++++------- .../NavigationOverlayFragment.kt | 1 - 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt index e008e53e59..6d018dda69 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt @@ -154,13 +154,18 @@ class FocusRepo( newState = updatePocketMegaTileFocusTree(activeScreen, focusNode, pinnedTilesIsEmpty) } R.id.megaTileTryAgainButton -> { - newState = updatePocketMegaTileFocusTree(activeScreen, focusNode, pinnedTilesIsEmpty) + newState = handleLostFocusInOverlay( + activeScreen, + focusNode, + sessionState, + pinnedTilesIsEmpty, + pocketState) } R.id.home_tile -> { // If pinnedTiles is empty and current focus is on home_tile, we need to // restore lost focus (this happens when you remove all tiles in the overlay) if (pinnedTilesIsEmpty) { - newState = handleEmptyTilesLostFocus( + newState = handleLostFocusInOverlay( activeScreen, focusNode, sessionState, @@ -287,21 +292,24 @@ class FocusRepo( } /** - * When all the pinned tiles are removed, tilContainer no longer needs focus + * Two possible scenarios for losing focus when in overlay: + * 1. When all the pinned tiles are removed, tilContainer no longer needs focus + * 2. When click on [megaTileTryAgainButton] */ - private fun handleEmptyTilesLostFocus( + private fun handleLostFocusInOverlay( activeScreen: ScreenControllerStateMachine.ActiveScreen, - tileContainerFocusNode: FocusNode, + lostFocusNode: FocusNode, sessionState: SessionRepo.State, pinnedTilesIsEmpty: Boolean, pocketState: PocketVideoRepo.FeedState ): State { - assert(tileContainerFocusNode.viewId == R.id.tileContainer) + assert(lostFocusNode.viewId == R.id.tileContainer || + lostFocusNode.viewId == R.id.megaTileTryAgainButton) - val viewId = when { - pocketState == PocketVideoRepo.FeedState.FetchFailed -> R.id.megaTileTryAgainButton - pocketState == PocketVideoRepo.FeedState.Inactive -> R.id.navUrlInput + val viewId = when (pocketState) { + PocketVideoRepo.FeedState.FetchFailed -> R.id.megaTileTryAgainButton + PocketVideoRepo.FeedState.Inactive -> R.id.navUrlInput else -> R.id.pocketVideoMegaTileView } @@ -313,7 +321,9 @@ class FocusRepo( } // Request focus on newState - _events.onNext(Event.RequestFocus) + if (newFocusNode.viewId != lostFocusNode.viewId) { + _events.onNext(Event.RequestFocus) + } return newState } diff --git a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt index 26fa037ed3..d6b79be0cb 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt @@ -304,7 +304,6 @@ class NavigationOverlayFragment : Fragment() { megaTileTryAgainButton.setOnClickListener { _ -> pocketViewModel.update() initMegaTile() - pocketVideoMegaTileView.requestFocus() } } From d80694ae5b8883517d05aba27a5b25022d03d457 Mon Sep 17 00:00:00 2001 From: Simon Chae Date: Thu, 25 Apr 2019 16:11:20 -0400 Subject: [PATCH 22/27] Issue #1395: Handle lost focus case --- .../java/org/mozilla/tv/firefox/focus/FocusRepo.kt | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt index 6d018dda69..2e952ee503 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt @@ -20,6 +20,8 @@ import org.mozilla.tv.firefox.pocket.PocketVideoRepo import org.mozilla.tv.firefox.session.SessionRepo import org.mozilla.tv.firefox.utils.URLs +private const val INVALID_VIEW_ID = -1 + class FocusRepo( screenController: ScreenController, sessionRepo: SessionRepo, @@ -173,6 +175,18 @@ class FocusRepo( pocketState) } } + INVALID_VIEW_ID -> { + // Focus is lost so default it to navUrlInput and send a [Event.RequestFocus] + val newFocusNode = FocusNode(R.id.navUrlInput) + + newState = updateNavUrlInputFocusTree( + activeScreen, + newFocusNode, + sessionState, + pinnedTilesIsEmpty, + pocketState) + _events.onNext(Event.RequestFocus) + } } } ScreenControllerStateMachine.ActiveScreen.WEB_RENDER -> {} From 5cf678428e22a20c98fedc21a5418fcf269965c5 Mon Sep 17 00:00:00 2001 From: Simon Chae Date: Thu, 25 Apr 2019 16:13:13 -0400 Subject: [PATCH 23/27] Issue #1395: Remove all remaining deprecated focus logic in overlay fragment --- .../navigationoverlay/NavigationOverlayFragment.kt | 9 --------- 1 file changed, 9 deletions(-) diff --git a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt index d6b79be0cb..d184801231 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt @@ -203,15 +203,6 @@ class NavigationOverlayFragment : Fragment() { val tintDrawable: (Drawable?) -> Unit = { it?.setTint(ContextCompat.getColor(context!!, R.color.photonGrey10_a60p)) } navUrlInput.compoundDrawablesRelative.forEach(tintDrawable) - // TODO: remove this when FocusRepo is in place #1395 -// when (defaultFocusTag) { -// PocketVideoFragment.FRAGMENT_TAG -> { -// pocketVideoMegaTileView.requestFocus() -// defaultFocusTag = NavigationOverlayFragment.FRAGMENT_TAG -// } -// NavigationOverlayFragment.FRAGMENT_TAG -> navUrlInput.requestFocus() -// } - registerForContextMenu(tileContainer) } From f87b8d32961e1a16bbbb8af9183a12706a330129 Mon Sep 17 00:00:00 2001 From: Simon Chae Date: Thu, 25 Apr 2019 16:27:13 -0400 Subject: [PATCH 24/27] Issue #1395: Add WebRenderViewModel --- .../firefox/architecture/ViewModelFactory.kt | 17 +++++++---- .../tv/firefox/webrender/WebRenderFragment.kt | 4 +++ .../firefox/webrender/WebRenderViewModel.kt | 28 +++++++++++++++++++ 3 files changed, 43 insertions(+), 6 deletions(-) create mode 100644 app/src/main/java/org/mozilla/tv/firefox/webrender/WebRenderViewModel.kt diff --git a/app/src/main/java/org/mozilla/tv/firefox/architecture/ViewModelFactory.kt b/app/src/main/java/org/mozilla/tv/firefox/architecture/ViewModelFactory.kt index dfa8d953b9..4291c16724 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/architecture/ViewModelFactory.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/architecture/ViewModelFactory.kt @@ -17,6 +17,7 @@ import org.mozilla.tv.firefox.navigationoverlay.ToolbarViewModel import org.mozilla.tv.firefox.settings.SettingsViewModel import org.mozilla.tv.firefox.utils.ServiceLocator import org.mozilla.tv.firefox.webrender.WebRenderHintViewModel +import org.mozilla.tv.firefox.webrender.WebRenderViewModel import org.mozilla.tv.firefox.webrender.cursor.CursorViewModel /** @@ -67,15 +68,19 @@ class ViewModelFactory( ) as T OverlayHintViewModel::class.java -> OverlayHintViewModel( - serviceLocator.sessionRepo, - hintContentFactory.getCloseMenuHint() + serviceLocator.sessionRepo, + hintContentFactory.getCloseMenuHint() ) as T WebRenderHintViewModel::class.java -> WebRenderHintViewModel( - serviceLocator.sessionRepo, - serviceLocator.cursorEventRepo, - serviceLocator.screenController, - hintContentFactory.getOpenMenuHint() + serviceLocator.sessionRepo, + serviceLocator.cursorEventRepo, + serviceLocator.screenController, + hintContentFactory.getOpenMenuHint() + ) as T + + WebRenderViewModel::class.java -> WebRenderViewModel( + serviceLocator.focusRepo ) as T // This class needs to either return a ViewModel or throw, so we have no good way of silently handling diff --git a/app/src/main/java/org/mozilla/tv/firefox/webrender/WebRenderFragment.kt b/app/src/main/java/org/mozilla/tv/firefox/webrender/WebRenderFragment.kt index 8d53c58c7b..54551309ed 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/webrender/WebRenderFragment.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/webrender/WebRenderFragment.kt @@ -87,9 +87,13 @@ class WebRenderFragment : EngineViewLifecycleFragment(), Session.Observer { // work properly, so we !! private val youtubeBackHandler by lazy { YouTubeBackHandler(engineView!!, activity as MainActivity) } + private lateinit var webRenderViewModel: WebRenderViewModel + override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) initSession() + + webRenderViewModel = FirefoxViewModelProviders.of(this).get(WebRenderViewModel::class.java) } @SuppressLint("RestrictedApi") diff --git a/app/src/main/java/org/mozilla/tv/firefox/webrender/WebRenderViewModel.kt b/app/src/main/java/org/mozilla/tv/firefox/webrender/WebRenderViewModel.kt new file mode 100644 index 0000000000..791c941b48 --- /dev/null +++ b/app/src/main/java/org/mozilla/tv/firefox/webrender/WebRenderViewModel.kt @@ -0,0 +1,28 @@ +/* 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.tv.firefox.webrender + +import androidx.lifecycle.ViewModel +import io.reactivex.Observable +import io.reactivex.rxkotlin.withLatestFrom +import org.mozilla.tv.firefox.R +import org.mozilla.tv.firefox.ScreenControllerStateMachine +import org.mozilla.tv.firefox.focus.FocusRepo + +class WebRenderViewModel(focusRepo: FocusRepo) : ViewModel() { + + val focusRequest: Observable = focusRepo.events.withLatestFrom(focusRepo.focusUpdate) + .filter { (_, state) -> + state.activeScreen == ScreenControllerStateMachine.ActiveScreen.WEB_RENDER + } + .map { (event, state) -> + when (event) { + FocusRepo.Event.ScreenChange -> + state.defaultFocusMap[state.activeScreen] ?: R.id.navUrlInput + FocusRepo.Event.RequestFocus -> + state.focusNode.viewId + } + } +} From ff18874b5734d60037da7918533ae85817938a45 Mon Sep 17 00:00:00 2001 From: Simon Chae Date: Fri, 26 Apr 2019 12:37:56 -0400 Subject: [PATCH 25/27] Issue #1395: Hook up WebRenderVM to FocusRepo --- .../firefox/architecture/ViewModelFactory.kt | 3 +- .../org/mozilla/tv/firefox/focus/FocusRepo.kt | 10 +++++- .../tv/firefox/webrender/WebRenderFragment.kt | 31 ++++++++++++------- .../firefox/webrender/WebRenderViewModel.kt | 18 ++++------- 4 files changed, 37 insertions(+), 25 deletions(-) diff --git a/app/src/main/java/org/mozilla/tv/firefox/architecture/ViewModelFactory.kt b/app/src/main/java/org/mozilla/tv/firefox/architecture/ViewModelFactory.kt index 4291c16724..49c924a072 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/architecture/ViewModelFactory.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/architecture/ViewModelFactory.kt @@ -80,7 +80,8 @@ class ViewModelFactory( ) as T WebRenderViewModel::class.java -> WebRenderViewModel( - serviceLocator.focusRepo + serviceLocator.focusRepo, + serviceLocator.screenController.currentActiveScreen ) as T // This class needs to either return a ViewModel or throw, so we have no good way of silently handling diff --git a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt index 2e952ee503..214c00af0b 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt @@ -86,13 +86,21 @@ class FocusRepo( } override fun onGlobalFocusChanged(oldFocus: View?, newFocus: View?) { + fun BehaviorSubject.onNextIfNew(value: T) { + val currState = this.value as State + val newState = value as State + if (currState.focusNode.viewId != newState.focusNode.viewId && + newState.focusNode.viewId != -1) + this.onNext(value) + } + newFocus?.let { val newState = State( activeScreen = _state.value!!.activeScreen, focusNode = FocusNode(it.id), defaultFocusMap = _state.value!!.defaultFocusMap) - _state.onNext(newState) + _state.onNextIfNew(newState) } } diff --git a/app/src/main/java/org/mozilla/tv/firefox/webrender/WebRenderFragment.kt b/app/src/main/java/org/mozilla/tv/firefox/webrender/WebRenderFragment.kt index 54551309ed..eea939e064 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/webrender/WebRenderFragment.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/webrender/WebRenderFragment.kt @@ -36,7 +36,6 @@ import org.mozilla.tv.firefox.MediaSessionHolder import org.mozilla.tv.firefox.R import org.mozilla.tv.firefox.ScreenControllerStateMachine.ActiveScreen import org.mozilla.tv.firefox.architecture.FirefoxViewModelProviders -import org.mozilla.tv.firefox.architecture.FocusOnShowDelegate import org.mozilla.tv.firefox.ext.focusedDOMElement import org.mozilla.tv.firefox.ext.forceExhaustive import org.mozilla.tv.firefox.ext.isYoutubeTV @@ -88,6 +87,7 @@ class WebRenderFragment : EngineViewLifecycleFragment(), Session.Observer { private val youtubeBackHandler by lazy { YouTubeBackHandler(engineView!!, activity as MainActivity) } private lateinit var webRenderViewModel: WebRenderViewModel + private lateinit var rootView: View override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) @@ -162,19 +162,19 @@ class WebRenderFragment : EngineViewLifecycleFragment(), Session.Observer { return layout } + override fun onViewCreated(view: View, savedInstanceState: Bundle?) { + super.onViewCreated(view, savedInstanceState) + + rootView = view + } + + // TODO: this method needs to be renamed (#2053); preliminary onStart() setup override fun onEngineViewCreated(engineView: EngineView): Disposable? { return serviceLocator?.screenController?.currentActiveScreen?.subscribe { if (it == ActiveScreen.WEB_RENDER) { // Cache focused DOM element just before WebView gains focus. See comment in // FocusedDOMElementCacheInterface for details engineView.focusedDOMElement.cache() - - // EngineView focus may be lost after waking up from sleep & screen saver. - // Forcibly request focus onStart(), after DOMElement cache, IFF webRenderFragment - // is the current ActiveScreen - // TODO: move this when focus repo is in place (#1395) - // TODO: this method needs to be renamed (#2053); preliminary onStart() setup - engineView.asView().requestFocus() } else { // Pause all the videos when transitioning out of [WebRenderFragment] to mitigate possible // memory leak while clearing data. See [WebViewCache.clear] as well as #1720 @@ -186,6 +186,9 @@ class WebRenderFragment : EngineViewLifecycleFragment(), Session.Observer { override fun onStart() { super.onStart() + observeRequestFocus() + .addTo(startStopCompositeDisposable) + /** * When calling getOrCreateEngineSession(), [SessionManager] lazily creates an [EngineSession] * instance and links it with its respective [Session]. During the linking, [SessionManager] @@ -241,9 +244,15 @@ class WebRenderFragment : EngineViewLifecycleFragment(), Session.Observer { cursor = null } - override fun onHiddenChanged(hidden: Boolean) { - FocusOnShowDelegate().onHiddenChanged(this, hidden) - super.onHiddenChanged(hidden) + private fun observeRequestFocus(): Disposable { + // EngineView focus may be lost after waking up from sleep & screen saver. + // Forcibly request focus onStart(), after DOMElement cache, IFF webRenderFragment + // is the current ActiveScreen + return webRenderViewModel.focusRequest + .subscribe { viewId -> + val viewToFocus = rootView.findViewById(viewId) + viewToFocus.requestFocus() + } } fun loadUrl(url: String) { diff --git a/app/src/main/java/org/mozilla/tv/firefox/webrender/WebRenderViewModel.kt b/app/src/main/java/org/mozilla/tv/firefox/webrender/WebRenderViewModel.kt index 791c941b48..0173332663 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/webrender/WebRenderViewModel.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/webrender/WebRenderViewModel.kt @@ -11,18 +11,12 @@ import org.mozilla.tv.firefox.R import org.mozilla.tv.firefox.ScreenControllerStateMachine import org.mozilla.tv.firefox.focus.FocusRepo -class WebRenderViewModel(focusRepo: FocusRepo) : ViewModel() { +class WebRenderViewModel(focusRepo: FocusRepo, activeScreen: Observable) : ViewModel() { - val focusRequest: Observable = focusRepo.events.withLatestFrom(focusRepo.focusUpdate) - .filter { (_, state) -> - state.activeScreen == ScreenControllerStateMachine.ActiveScreen.WEB_RENDER - } - .map { (event, state) -> - when (event) { - FocusRepo.Event.ScreenChange -> - state.defaultFocusMap[state.activeScreen] ?: R.id.navUrlInput - FocusRepo.Event.RequestFocus -> - state.focusNode.viewId - } + val focusRequest: Observable = focusRepo.events.withLatestFrom(activeScreen) + .filter { (_, activeScreen) -> + activeScreen == ScreenControllerStateMachine.ActiveScreen.WEB_RENDER + }.map { + R.id.engineView } } From a8ed3e44d4a67c454e901c86937bead5ed1fea89 Mon Sep 17 00:00:00 2001 From: Simon Chae Date: Fri, 26 Apr 2019 12:55:01 -0400 Subject: [PATCH 26/27] Issue #1395: Remove activeScreen reference from FocusRepo.State --- .../firefox/architecture/ViewModelFactory.kt | 3 +- .../org/mozilla/tv/firefox/focus/FocusRepo.kt | 33 ++++--------------- .../NavigationOverlayViewModel.kt | 16 +++++---- 3 files changed, 18 insertions(+), 34 deletions(-) diff --git a/app/src/main/java/org/mozilla/tv/firefox/architecture/ViewModelFactory.kt b/app/src/main/java/org/mozilla/tv/firefox/architecture/ViewModelFactory.kt index 49c924a072..1ef099d007 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/architecture/ViewModelFactory.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/architecture/ViewModelFactory.kt @@ -64,7 +64,8 @@ class ViewModelFactory( NavigationOverlayViewModel::class.java -> NavigationOverlayViewModel( serviceLocator.sessionRepo, - serviceLocator.focusRepo + serviceLocator.focusRepo, + serviceLocator.screenController.currentActiveScreen ) as T OverlayHintViewModel::class.java -> OverlayHintViewModel( diff --git a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt index 214c00af0b..21407ed39f 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt @@ -30,7 +30,6 @@ class FocusRepo( ) : ViewTreeObserver.OnGlobalFocusChangeListener { data class State( - val activeScreen: ScreenControllerStateMachine.ActiveScreen, val focusNode: FocusNode, val defaultFocusMap: HashMap ) @@ -96,7 +95,6 @@ class FocusRepo( newFocus?.let { val newState = State( - activeScreen = _state.value!!.activeScreen, focusNode = FocusNode(it.id), defaultFocusMap = _state.value!!.defaultFocusMap) @@ -111,7 +109,6 @@ class FocusRepo( focusMap[ScreenControllerStateMachine.ActiveScreen.POCKET] = R.id.videoFeed val newState = State( - activeScreen = ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY, focusNode = FocusNode(R.id.navUrlInput), defaultFocusMap = focusMap) @@ -136,12 +133,11 @@ class FocusRepo( when (prevScreen) { ScreenControllerStateMachine.ActiveScreen.WEB_RENDER -> { newState = updateDefaultFocusForOverlayWhenTransitioningFromWebRender( - activeScreen, focusMap, sessionState) } ScreenControllerStateMachine.ActiveScreen.POCKET -> { - newState = updateDefaultFocusForOverlayWhenTransitioningFromPocket(activeScreen, focusMap) + newState = updateDefaultFocusForOverlayWhenTransitioningFromPocket(focusMap) } else -> Unit } @@ -149,23 +145,21 @@ class FocusRepo( when (focusNode.viewId) { R.id.navUrlInput -> newState = updateNavUrlInputFocusTree( - activeScreen, focusNode, sessionState, pinnedTilesIsEmpty, pocketState) R.id.navButtonReload -> { - newState = updateReloadButtonFocusTree(activeScreen, focusNode, sessionState) + newState = updateReloadButtonFocusTree(focusNode, sessionState) } R.id.navButtonForward -> { - newState = updateForwardButtonFocusTree(activeScreen, focusNode, sessionState) + newState = updateForwardButtonFocusTree(focusNode, sessionState) } R.id.pocketVideoMegaTileView -> { - newState = updatePocketMegaTileFocusTree(activeScreen, focusNode, pinnedTilesIsEmpty) + newState = updatePocketMegaTileFocusTree(focusNode, pinnedTilesIsEmpty) } R.id.megaTileTryAgainButton -> { newState = handleLostFocusInOverlay( - activeScreen, focusNode, sessionState, pinnedTilesIsEmpty, @@ -176,7 +170,6 @@ class FocusRepo( // restore lost focus (this happens when you remove all tiles in the overlay) if (pinnedTilesIsEmpty) { newState = handleLostFocusInOverlay( - activeScreen, focusNode, sessionState, pinnedTilesIsEmpty, @@ -188,7 +181,6 @@ class FocusRepo( val newFocusNode = FocusNode(R.id.navUrlInput) newState = updateNavUrlInputFocusTree( - activeScreen, newFocusNode, sessionState, pinnedTilesIsEmpty, @@ -211,7 +203,6 @@ class FocusRepo( } private fun updateNavUrlInputFocusTree( - activeScreen: ScreenControllerStateMachine.ActiveScreen, focusedNavUrlInputNode: FocusNode, sessionState: SessionRepo.State, pinnedTilesIsEmpty: Boolean, @@ -240,7 +231,6 @@ class FocusRepo( } return State( - activeScreen = activeScreen, focusNode = FocusNode( focusedNavUrlInputNode.viewId, nextFocusUpId, @@ -249,7 +239,6 @@ class FocusRepo( } private fun updateReloadButtonFocusTree( - activeScreen: ScreenControllerStateMachine.ActiveScreen, focusedReloadButtonNode: FocusNode, sessionState: SessionRepo.State ): State { @@ -263,7 +252,6 @@ class FocusRepo( } return State( - activeScreen = activeScreen, focusNode = FocusNode( focusedReloadButtonNode.viewId, nextFocusLeftId = nextFocusLeftId), @@ -271,7 +259,6 @@ class FocusRepo( } private fun updateForwardButtonFocusTree( - activeScreen: ScreenControllerStateMachine.ActiveScreen, focusedForwardButtonNode: FocusNode, sessionState: SessionRepo.State ): State { @@ -284,7 +271,6 @@ class FocusRepo( } return State( - activeScreen = activeScreen, focusNode = FocusNode( focusedForwardButtonNode.viewId, nextFocusLeftId = nextFocusLeftId), @@ -292,7 +278,6 @@ class FocusRepo( } private fun updatePocketMegaTileFocusTree( - activeScreen: ScreenControllerStateMachine.ActiveScreen, focusedPocketMegatTileNode: FocusNode, pinnedTilesIsEmpty: Boolean ): State { @@ -306,7 +291,6 @@ class FocusRepo( } return State( - activeScreen = activeScreen, focusNode = FocusNode( focusedPocketMegatTileNode.viewId, nextFocusDownId = nextFocusDownId), @@ -319,7 +303,6 @@ class FocusRepo( * 2. When click on [megaTileTryAgainButton] */ private fun handleLostFocusInOverlay( - activeScreen: ScreenControllerStateMachine.ActiveScreen, lostFocusNode: FocusNode, sessionState: SessionRepo.State, pinnedTilesIsEmpty: Boolean, @@ -337,9 +320,9 @@ class FocusRepo( val newFocusNode = FocusNode(viewId) val newState = if (newFocusNode.viewId == R.id.navUrlInput) { - updateNavUrlInputFocusTree(activeScreen, newFocusNode, sessionState, pinnedTilesIsEmpty, pocketState) + updateNavUrlInputFocusTree(newFocusNode, sessionState, pinnedTilesIsEmpty, pocketState) } else { - updatePocketMegaTileFocusTree(activeScreen, newFocusNode, pinnedTilesIsEmpty) + updatePocketMegaTileFocusTree(newFocusNode, pinnedTilesIsEmpty) } // Request focus on newState @@ -351,7 +334,6 @@ class FocusRepo( } private fun updateDefaultFocusForOverlayWhenTransitioningFromWebRender( - activeScreen: ScreenControllerStateMachine.ActiveScreen, focusMap: HashMap, sessionState: SessionRepo.State ): State { @@ -366,20 +348,17 @@ class FocusRepo( } return State( - activeScreen = activeScreen, focusNode = _state.value!!.focusNode, defaultFocusMap = focusMap) } private fun updateDefaultFocusForOverlayWhenTransitioningFromPocket( - activeScreen: ScreenControllerStateMachine.ActiveScreen, focusMap: HashMap ): State { focusMap[ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY] = R.id.pocketVideoMegaTileView return State( - activeScreen = activeScreen, focusNode = _state.value!!.focusNode, defaultFocusMap = focusMap) } diff --git a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayViewModel.kt b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayViewModel.kt index df2a135e59..cbea94ed50 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayViewModel.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayViewModel.kt @@ -15,16 +15,20 @@ import org.mozilla.tv.firefox.focus.FocusRepo import org.mozilla.tv.firefox.session.SessionRepo import org.mozilla.tv.firefox.utils.URLs -class NavigationOverlayViewModel(sessionRepo: SessionRepo, focusRepo: FocusRepo) : ViewModel() { +class NavigationOverlayViewModel( + sessionRepo: SessionRepo, + focusRepo: FocusRepo, + currActiveScreen: Observable +) : ViewModel() { - val focusRequest: Observable = focusRepo.events.withLatestFrom(focusRepo.focusUpdate) - .filter { (_, state) -> - state.activeScreen == ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY + val focusRequest: Observable = focusRepo.events.withLatestFrom(focusRepo.focusUpdate, currActiveScreen) + .filter { (_, _, activeScreen) -> + activeScreen == ScreenControllerStateMachine.ActiveScreen.NAVIGATION_OVERLAY } - .map { (event, state) -> + .map { (event, state, activeScreen) -> when (event) { FocusRepo.Event.ScreenChange -> - state.defaultFocusMap[state.activeScreen] ?: R.id.navUrlInput + state.defaultFocusMap[activeScreen] ?: R.id.navUrlInput FocusRepo.Event.RequestFocus -> state.focusNode.viewId } From 60bc0565e41b7b68441bc5e13f91d7eaf1fd29c6 Mon Sep 17 00:00:00 2001 From: Severin Rudie Date: Sat, 27 Apr 2019 14:15:06 -0700 Subject: [PATCH 27/27] hacked up possible alternate focus repo approach --- .../org/mozilla/tv/firefox/MainActivity.kt | 1 + .../java/org/mozilla/tv/firefox/ext/View.kt | 11 + .../org/mozilla/tv/firefox/focus/FocusRepo.kt | 202 ++++++++++++++---- .../NavigationOverlayFragment.kt | 27 ++- .../NavigationOverlayViewModel.kt | 6 +- app/src/main/res/values/ids.xml | 1 + .../mozilla/tv/firefox/ext/EngineSession.kt | 5 +- .../java/org/mozilla/tv/firefox/RxTest.kt | 19 ++ 8 files changed, 218 insertions(+), 54 deletions(-) diff --git a/app/src/main/java/org/mozilla/tv/firefox/MainActivity.kt b/app/src/main/java/org/mozilla/tv/firefox/MainActivity.kt index 4442dbde8c..b705892508 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/MainActivity.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/MainActivity.kt @@ -16,6 +16,7 @@ import android.view.View import androidx.lifecycle.Observer import io.sentry.Sentry import kotlinx.android.synthetic.main.activity_main.* +import kotlinx.android.synthetic.main.fragment_navigation_overlay_top_nav.navButtonBack import kotlinx.android.synthetic.main.overlay_debug.debugLog import mozilla.components.browser.session.Session import mozilla.components.concept.engine.EngineView diff --git a/app/src/main/java/org/mozilla/tv/firefox/ext/View.kt b/app/src/main/java/org/mozilla/tv/firefox/ext/View.kt index c1ea71ea05..a717ea5384 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/ext/View.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/ext/View.kt @@ -17,3 +17,14 @@ val View.isEffectivelyVisible: Boolean get() { } return true } + +// TODO test this. likely off by 1 error here +fun View.itAndAncestorsAreVisible(generationsUp: Int = Int.MAX_VALUE): Boolean { + var generations = generationsUp + var node: View? = this + while (node != null && generations-- > 0) { + if (node.visibility != View.VISIBLE) return false + node = node.parent as? View + } + return true +} diff --git a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt index 21407ed39f..3893bc1bb3 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/focus/FocusRepo.kt @@ -15,6 +15,8 @@ import io.reactivex.subjects.Subject import org.mozilla.tv.firefox.R import org.mozilla.tv.firefox.ScreenController import org.mozilla.tv.firefox.ScreenControllerStateMachine +import org.mozilla.tv.firefox.ScreenControllerStateMachine.ActiveScreen +import org.mozilla.tv.firefox.ext.itAndAncestorsAreVisible import org.mozilla.tv.firefox.pinnedtile.PinnedTileRepo import org.mozilla.tv.firefox.pocket.PocketVideoRepo import org.mozilla.tv.firefox.session.SessionRepo @@ -22,6 +24,54 @@ import org.mozilla.tv.firefox.utils.URLs private const val INVALID_VIEW_ID = -1 +// TODO ↓ SEVERIN ↓ + +class FocusRequest(private vararg val ids: Int) { + fun requestOnFirstEnabled(root: View) { + ids.toList() + .firstAvailableViewIfAny(root) + ?.requestFocus() + } +} +private val NO_FOCUS_REQUEST = FocusRequest() + +class FocusNode( + val viewId: Int, + private val nextFocusUpIds: List? = null, + private val nextFocusDownIds: List? = null, + private val nextFocusLeftIds: List? = null, + private val nextFocusRightIds: List? = null +) { + fun updateViewNodeTree(focusedView: View) { + assert(focusedView.id == viewId) // todo assert or require? + val root = focusedView.rootView + + fun List?.applyFirstAvailableIdToView(root: View, action: View.(Int) -> Unit) { + this?: return + val firstAvailable = this.firstAvailableViewIfAny(root) ?: return + focusedView.action(firstAvailable.id) + } + + nextFocusUpIds.applyFirstAvailableIdToView(root) { nextFocusUpId = it } + nextFocusDownIds.applyFirstAvailableIdToView(root) { nextFocusDownId = it } + nextFocusLeftIds.applyFirstAvailableIdToView(root) { nextFocusLeftId = it } + nextFocusRightIds.applyFirstAvailableIdToView(root) { nextFocusRightId = it } + } +} + +private val invalidIds = listOf( + -1, + R.id.nested_web_view +) + +// Sequence prevents unnecessary findViewById calls +private fun List.firstAvailableViewIfAny(root: View): View? { + return this.asSequence() + .mapNotNull { root.findViewById(it) } + .filter { it.isEnabled && it.itAndAncestorsAreVisible(3) } + .firstOrNull() +} + class FocusRepo( screenController: ScreenController, sessionRepo: SessionRepo, @@ -29,9 +79,70 @@ class FocusRepo( pocketRepo: PocketVideoRepo ) : ViewTreeObserver.OnGlobalFocusChangeListener { + private val focusChanged = PublishSubject.create>() + + // Edge case, url bar is unaffected on startup (works if it regains focus) + val focusNodeForCurrentlyFocusedView: Observable = focusChanged + .map { oldAndNewFocus -> oldAndNewFocus.second } + .map { id -> + when (id) { + R.id.navUrlInput -> getUrlBarFocusNode() + R.id.pocketVideoMegaTileView -> getPocketMegatileFocusNode() + else -> FocusNode(viewId = id) + } + } + + val focusRequests: Observable by lazy { + Observable.merge( + defaultViewAfterScreenChange, + requestAfterFocusLost + ) + } + + private fun getUrlBarFocusNode() = FocusNode( + viewId = R.id.navUrlInput, + nextFocusUpIds = listOf(R.id.navButtonBack, R.id.navButtonForward, R.id.navButtonReload, R.id.turboButton), + nextFocusDownIds = listOf(R.id.pocketVideoMegaTileView, R.id.megaTileTryAgainButton, R.id.tileContainer, R.id.navUrlInput) + ) + + private fun getPocketMegatileFocusNode() = FocusNode( + viewId = R.id.pocketVideoMegaTileView, + nextFocusDownIds = listOf(R.id.tileContainer, R.id.settingsTileContainer) + ) + + private val defaultViewAfterScreenChange: Observable = screenController.currentActiveScreen + .startWith(ActiveScreen.NAVIGATION_OVERLAY) + .buffer(2, 1) // This emits a list of the previous and current screen. See RxTest.kt + .map { (previousScreen, currentScreen) -> + when (currentScreen) { + ActiveScreen.NAVIGATION_OVERLAY -> { + when (previousScreen) { + ActiveScreen.WEB_RENDER -> FocusRequest(R.id.navButtonBack, R.id.navButtonForward, R.id.navButtonReload, R.id.navUrlInput) + ActiveScreen.POCKET -> FocusRequest(R.id.pocketVideoMegaTileView) + ActiveScreen.SETTINGS -> FocusRequest(R.id.settings_tile_telemetry) + ActiveScreen.NAVIGATION_OVERLAY -> FocusRequest(R.id.navUrlInput) + null -> NO_FOCUS_REQUEST + } + } + ActiveScreen.WEB_RENDER -> FocusRequest(R.id.engineView) + ActiveScreen.POCKET -> FocusRequest(R.id.videoFeed) + ActiveScreen.SETTINGS -> NO_FOCUS_REQUEST + null -> NO_FOCUS_REQUEST + } + } + .filter { it != NO_FOCUS_REQUEST } + .replay(1) + .autoConnect(0) + + private val requestAfterFocusLost: Observable = focusChanged + .filter { (_, newFocus) -> invalidIds.contains(newFocus) } + .flatMap { defaultViewAfterScreenChange.take(1) } + + // TODO ^ SEVERIN ^ + data class State( - val focusNode: FocusNode, - val defaultFocusMap: HashMap + val focusNode: OldFocusNode, + val defaultFocusMap: HashMap ) enum class Event { @@ -40,9 +151,9 @@ class FocusRepo( } /** - * FocusNode describes quasi-directional focusable paths given viewId + * OldFocusNode describes quasi-directional focusable paths given viewId */ - data class FocusNode( + data class OldFocusNode( val viewId: Int, val nextFocusUpId: Int? = null, val nextFocusDownId: Int? = null, @@ -85,21 +196,22 @@ class FocusRepo( } override fun onGlobalFocusChanged(oldFocus: View?, newFocus: View?) { - fun BehaviorSubject.onNextIfNew(value: T) { - val currState = this.value as State - val newState = value as State - if (currState.focusNode.viewId != newState.focusNode.viewId && - newState.focusNode.viewId != -1) - this.onNext(value) - } - - newFocus?.let { - val newState = State( - focusNode = FocusNode(it.id), - defaultFocusMap = _state.value!!.defaultFocusMap) - - _state.onNextIfNew(newState) - } +// fun BehaviorSubject.onNextIfNew(value: T) { +// val currState = this.value as State +// val newState = value as State +// if (currState.focusNode.viewId != newState.focusNode.viewId && +// newState.focusNode.viewId != -1) +// this.onNext(value) +// } + +// val newState = State( +// focusNode = OldFocusNode(it.id), +// defaultFocusMap = _state.value!!.defaultFocusMap) +// +// _state.onNextIfNew(newState) + val oldId = oldFocus?.id ?: -1 + val newId = newFocus?.id ?: -1 + focusChanged.onNext(oldId to newId) } private fun initializeDefaultFocus() { @@ -109,7 +221,7 @@ class FocusRepo( focusMap[ScreenControllerStateMachine.ActiveScreen.POCKET] = R.id.videoFeed val newState = State( - focusNode = FocusNode(R.id.navUrlInput), + focusNode = OldFocusNode(R.id.navUrlInput), defaultFocusMap = focusMap) _state.onNext(newState) @@ -117,11 +229,11 @@ class FocusRepo( @VisibleForTesting private fun dispatchFocusUpdates( - focusNode: FocusNode, - activeScreen: ScreenControllerStateMachine.ActiveScreen, - sessionState: SessionRepo.State, - pinnedTilesIsEmpty: Boolean, - pocketState: PocketVideoRepo.FeedState + focusNode: OldFocusNode, + activeScreen: ScreenControllerStateMachine.ActiveScreen, + sessionState: SessionRepo.State, + pinnedTilesIsEmpty: Boolean, + pocketState: PocketVideoRepo.FeedState ): State { var newState = _state.value!! @@ -178,7 +290,7 @@ class FocusRepo( } INVALID_VIEW_ID -> { // Focus is lost so default it to navUrlInput and send a [Event.RequestFocus] - val newFocusNode = FocusNode(R.id.navUrlInput) + val newFocusNode = OldFocusNode(R.id.navUrlInput) newState = updateNavUrlInputFocusTree( newFocusNode, @@ -203,10 +315,10 @@ class FocusRepo( } private fun updateNavUrlInputFocusTree( - focusedNavUrlInputNode: FocusNode, - sessionState: SessionRepo.State, - pinnedTilesIsEmpty: Boolean, - pocketState: PocketVideoRepo.FeedState + focusedNavUrlInputNode: OldFocusNode, + sessionState: SessionRepo.State, + pinnedTilesIsEmpty: Boolean, + pocketState: PocketVideoRepo.FeedState ): State { assert(focusedNavUrlInputNode.viewId == R.id.navUrlInput) @@ -231,7 +343,7 @@ class FocusRepo( } return State( - focusNode = FocusNode( + focusNode = OldFocusNode( focusedNavUrlInputNode.viewId, nextFocusUpId, nextFocusDownId), @@ -239,8 +351,8 @@ class FocusRepo( } private fun updateReloadButtonFocusTree( - focusedReloadButtonNode: FocusNode, - sessionState: SessionRepo.State + focusedReloadButtonNode: OldFocusNode, + sessionState: SessionRepo.State ): State { assert(focusedReloadButtonNode.viewId == R.id.navButtonReload) @@ -252,15 +364,15 @@ class FocusRepo( } return State( - focusNode = FocusNode( + focusNode = OldFocusNode( focusedReloadButtonNode.viewId, nextFocusLeftId = nextFocusLeftId), defaultFocusMap = _state.value!!.defaultFocusMap) } private fun updateForwardButtonFocusTree( - focusedForwardButtonNode: FocusNode, - sessionState: SessionRepo.State + focusedForwardButtonNode: OldFocusNode, + sessionState: SessionRepo.State ): State { assert(focusedForwardButtonNode.viewId == R.id.navButtonForward) @@ -271,15 +383,15 @@ class FocusRepo( } return State( - focusNode = FocusNode( + focusNode = OldFocusNode( focusedForwardButtonNode.viewId, nextFocusLeftId = nextFocusLeftId), defaultFocusMap = _state.value!!.defaultFocusMap) } private fun updatePocketMegaTileFocusTree( - focusedPocketMegatTileNode: FocusNode, - pinnedTilesIsEmpty: Boolean + focusedPocketMegatTileNode: OldFocusNode, + pinnedTilesIsEmpty: Boolean ): State { assert(focusedPocketMegatTileNode.viewId == R.id.pocketVideoMegaTileView || @@ -291,7 +403,7 @@ class FocusRepo( } return State( - focusNode = FocusNode( + focusNode = OldFocusNode( focusedPocketMegatTileNode.viewId, nextFocusDownId = nextFocusDownId), defaultFocusMap = _state.value!!.defaultFocusMap) @@ -303,10 +415,10 @@ class FocusRepo( * 2. When click on [megaTileTryAgainButton] */ private fun handleLostFocusInOverlay( - lostFocusNode: FocusNode, - sessionState: SessionRepo.State, - pinnedTilesIsEmpty: Boolean, - pocketState: PocketVideoRepo.FeedState + lostFocusNode: OldFocusNode, + sessionState: SessionRepo.State, + pinnedTilesIsEmpty: Boolean, + pocketState: PocketVideoRepo.FeedState ): State { assert(lostFocusNode.viewId == R.id.tileContainer || @@ -318,7 +430,7 @@ class FocusRepo( else -> R.id.pocketVideoMegaTileView } - val newFocusNode = FocusNode(viewId) + val newFocusNode = OldFocusNode(viewId) val newState = if (newFocusNode.viewId == R.id.navUrlInput) { updateNavUrlInputFocusTree(newFocusNode, sessionState, pinnedTilesIsEmpty, pocketState) } else { diff --git a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt index d184801231..5a071d9dd9 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayFragment.kt @@ -25,6 +25,7 @@ import androidx.core.widget.NestedScrollView import androidx.fragment.app.Fragment import androidx.lifecycle.Observer import androidx.recyclerview.widget.GridLayoutManager +import io.reactivex.android.schedulers.AndroidSchedulers import io.reactivex.disposables.CompositeDisposable import io.reactivex.disposables.Disposable import io.reactivex.rxkotlin.addTo @@ -59,6 +60,7 @@ import org.mozilla.tv.firefox.telemetry.UrlTextInputLocation import org.mozilla.tv.firefox.utils.ServiceLocator import org.mozilla.tv.firefox.widget.InlineAutocompleteEditText import java.lang.ref.WeakReference +import java.util.concurrent.TimeUnit private const val SHOW_UNPIN_TOAST_COUNTER_PREF = "show_upin_toast_counter" private const val MAX_UNPIN_TOAST_COUNT = 3 @@ -216,6 +218,17 @@ class NavigationOverlayFragment : Fragment() { .addTo(compositeDisposable) observePocketState() .addTo(compositeDisposable) + navigationOverlayViewModel.focusRequests + .subscribe { focusRequest -> + view?.let { view -> focusRequest.requestOnFirstEnabled(view) } + }.addTo(compositeDisposable) + navigationOverlayViewModel.focusNodeForCurrentlyFocusedView + .subscribe { focusNode -> + rootView.findViewById(focusNode.viewId)?.let { focusedView -> + focusNode.updateViewNodeTree(focusedView) + } + }.addTo(compositeDisposable) + HintBinder.bindHintsToView(hintViewModel, hintBarContainer, animate = false) .forEach { compositeDisposable.add(it) } } @@ -231,18 +244,18 @@ class NavigationOverlayFragment : Fragment() { private fun observeRequestFocus(): Disposable { return navigationOverlayViewModel.focusRequest - .subscribe { viewId -> - val viewToFocus = rootView.findViewById(viewId) - viewToFocus?.requestFocus() + .subscribe { _ -> +// val viewToFocus = rootView.findViewById(viewId) +// viewToFocus?.requestFocus() } } private fun observeFocusState(): Disposable { return navigationOverlayViewModel.focusUpdate - .subscribe { focusNode -> - rootView.findViewById(focusNode.viewId)?.let { focusedView -> - focusNode.updateViewNodeTree(focusedView) - } + .subscribe { _ -> +// rootView.findViewById(focusNode.viewId)?.let { focusedView -> +// focusNode.updateViewNodeTree(focusedView) +// } } } diff --git a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayViewModel.kt b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayViewModel.kt index cbea94ed50..267f67a926 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayViewModel.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/navigationoverlay/NavigationOverlayViewModel.kt @@ -34,7 +34,11 @@ class NavigationOverlayViewModel( } } - val focusUpdate: Observable = focusRepo.focusUpdate.map { it.focusNode } + val focusUpdate: Observable = focusRepo.focusUpdate.map { it.focusNode } + + val focusRequests = focusRepo.focusRequests + + val focusNodeForCurrentlyFocusedView = focusRepo.focusNodeForCurrentlyFocusedView @Suppress("DEPRECATION") val viewIsSplit: LiveData = sessionRepo.legacyState.map { diff --git a/app/src/main/res/values/ids.xml b/app/src/main/res/values/ids.xml index 5de7556acb..2cb3d6872a 100644 --- a/app/src/main/res/values/ids.xml +++ b/app/src/main/res/values/ids.xml @@ -7,4 +7,5 @@ + diff --git a/app/src/system/java/org/mozilla/tv/firefox/ext/EngineSession.kt b/app/src/system/java/org/mozilla/tv/firefox/ext/EngineSession.kt index 512b659867..d69c601f89 100644 --- a/app/src/system/java/org/mozilla/tv/firefox/ext/EngineSession.kt +++ b/app/src/system/java/org/mozilla/tv/firefox/ext/EngineSession.kt @@ -8,6 +8,7 @@ import android.content.Context import mozilla.components.browser.engine.system.NestedWebView import mozilla.components.browser.engine.system.SystemEngineSession import mozilla.components.concept.engine.EngineSession +import org.mozilla.tv.firefox.R /** * [AmazonWebView] requires ActivityContext in order to show 4K resolution rendering option (#277) @@ -16,5 +17,7 @@ import mozilla.components.concept.engine.EngineSession * override the webView instance */ fun EngineSession.resetView(context: Context) { - (this as SystemEngineSession).webView = NestedWebView(context) + (this as SystemEngineSession).webView = NestedWebView(context).apply { + this.id = R.id.nested_web_view + } } diff --git a/app/src/test/java/org/mozilla/tv/firefox/RxTest.kt b/app/src/test/java/org/mozilla/tv/firefox/RxTest.kt index 15ce5191e7..f40bd801c1 100644 --- a/app/src/test/java/org/mozilla/tv/firefox/RxTest.kt +++ b/app/src/test/java/org/mozilla/tv/firefox/RxTest.kt @@ -4,7 +4,9 @@ package org.mozilla.tv.firefox +import io.reactivex.Observable import io.reactivex.subjects.PublishSubject +import org.junit.Assert.assertEquals import org.junit.Test /** @@ -53,4 +55,21 @@ class RxTest { observable3.assertValues(3) } + + @Test + fun `display buffer behavior`() { + val numbers = Observable.just(1, 2, 3, 4, 5) + + val basicBuffer = numbers.buffer(2) + assertEquals( + listOf(listOf(1, 2), listOf(3, 4), listOf(5)), + basicBuffer.test().values() + ) + + val slidingBuffer = numbers.buffer(2, 1) + assertEquals( + listOf(listOf(1, 2), listOf(2, 3), listOf(3, 4), listOf(4, 5), listOf(5)), + slidingBuffer.test().values() + ) + } }