Archives
/
iceraven-browser
mirror of https://github.com/fork-maintainers/iceraven-browser
2
0
Fork 0
Code Issues Releases Wiki Activity

For #19746 - Remove feature flag for the three-dot menu redesign (#19750)

Browse Source 
* Remove feature flag for the three-dot menu redesign

* Remove menu feature flag from unit tests
upstream-sync
Elise Richards 3 years ago committed by GitHub
parent eaf74a1868
commit 6dbe5acc5b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 77 additions and 405 deletions
Show Stats Download Patch File Download Diff File

5
app/src/main/java/org/mozilla/fenix/FeatureFlags.kt
Unescape Escape View File

@ -34,11 +34,6 @@ object FeatureFlags {
*/
val webAuthFeature = Config.channel.isNightlyOrDebug
/**
* Shows new three-dot toolbar menu design.
*/
const val toolbarMenuFeature = true
/**
* Enables the tabs tray re-write with Synced Tabs.
*/

199
app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt
Unescape Escape View File

@ -34,15 +34,11 @@ import mozilla.components.feature.webcompat.reporter.WebCompatReporterFeature
import mozilla.components.lib.state.ext.flowScoped
import mozilla.components.support.ktx.android.content.getColorFromAttr
import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifAnyChanged
import org.mozilla.fenix.FeatureFlags
import org.mozilla.fenix.FeatureFlags.tabsTrayRewrite
import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.R
import org.mozilla.fenix.browser.browsingmode.BrowsingMode
import org.mozilla.fenix.components.accounts.FenixAccountManager
import org.mozilla.fenix.experiments.ExperimentBranch
import org.mozilla.fenix.experiments.FeatureId
import org.mozilla.fenix.ext.asActivity
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.settings
import org.mozilla.fenix.ext.withExperiment
@ -82,12 +78,7 @@ open class DefaultToolbarMenu(
override val menuBuilder by lazy {
WebExtensionBrowserMenuBuilder(
items =
if (FeatureFlags.toolbarMenuFeature) {
newCoreMenuItems
} else {
oldCoreMenuItems
},
items = coreMenuItems,
endOfMenuAlwaysVisible = shouldUseBottomToolbar,
store = store,
style = WebExtensionBrowserMenuBuilder.Style(
@ -161,27 +152,7 @@ open class DefaultToolbarMenu(
registerForIsBookmarkedUpdates()
if (FeatureFlags.toolbarMenuFeature) {
BrowserMenuItemToolbar(listOf(back, forward, share, refresh), isSticky = true)
} else {
val bookmark = BrowserMenuItemToolbar.TwoStateButton(
primaryImageResource = R.drawable.ic_bookmark_filled,
primaryContentDescription = context.getString(R.string.browser_menu_edit_bookmark),
primaryImageTintResource = primaryTextColor(),
// TwoStateButton.isInPrimaryState must be synchronous, and checking bookmark state is
// relatively slow. The best we can do here is periodically compute and cache a new "is
// bookmarked" state, and use that whenever the menu has been opened.
isInPrimaryState = { isCurrentUrlBookmarked },
secondaryImageResource = R.drawable.ic_bookmark_outline,
secondaryContentDescription = context.getString(R.string.browser_menu_bookmark),
secondaryImageTintResource = primaryTextColor(),
disableInSecondaryState = false
) {
handleBookmarkItemTapped()
}
BrowserMenuItemToolbar(listOf(back, forward, bookmark, share, refresh))
}
BrowserMenuItemToolbar(listOf(back, forward, share, refresh), isSticky = true)
}
// Predicates that need to be repeatedly called as the session changes
@ -222,170 +193,6 @@ open class DefaultToolbarMenu(
onItemTapped.invoke(ToolbarMenu.Item.InstallPwaToHomeScreen)
}
private val oldCoreMenuItems by lazy {
val settings = BrowserMenuHighlightableItem(
label = context.getString(R.string.browser_menu_settings),
startImageResource = R.drawable.ic_settings,
iconTintColorResource = if (hasAccountProblem)
ThemeManager.resolveAttribute(R.attr.syncDisconnected, context) else
primaryTextColor(),
textColorResource = if (hasAccountProblem)
ThemeManager.resolveAttribute(R.attr.primaryText, context) else
primaryTextColor(),
highlight = BrowserMenuHighlight.HighPriority(
endImageResource = R.drawable.ic_sync_disconnected,
backgroundTint = context.getColorFromAttr(R.attr.syncDisconnectedBackground),
canPropagate = false
),
isHighlighted = { hasAccountProblem }
) {
onItemTapped.invoke(ToolbarMenu.Item.Settings)
}
val desktopMode = BrowserMenuImageSwitch(
imageResource = R.drawable.ic_desktop,
label = context.getString(R.string.browser_menu_desktop_site),
initialState = {
selectedSession?.content?.desktopMode ?: false
}
) { checked ->
onItemTapped.invoke(ToolbarMenu.Item.RequestDesktop(checked))
}
val addToTopSites = BrowserMenuImageText(
label = context.getString(R.string.browser_menu_add_to_top_sites),
imageResource = R.drawable.ic_top_sites,
iconTintColorResource = primaryTextColor()
) {
onItemTapped.invoke(ToolbarMenu.Item.AddToTopSites)
}
val addToHomescreen = BrowserMenuImageText(
label = context.getString(R.string.browser_menu_add_to_homescreen),
imageResource = R.drawable.ic_add_to_homescreen,
iconTintColorResource = primaryTextColor()
) {
onItemTapped.invoke(ToolbarMenu.Item.AddToHomeScreen)
}
val syncedTabs = BrowserMenuImageText(
label = context.getString(R.string.synced_tabs),
imageResource = R.drawable.ic_synced_tabs,
iconTintColorResource = primaryTextColor()
) {
onItemTapped.invoke(ToolbarMenu.Item.SyncedTabs)
}
val findInPage = BrowserMenuImageText(
label = context.getString(R.string.browser_menu_find_in_page),
imageResource = R.drawable.mozac_ic_search,
iconTintColorResource = primaryTextColor()
) {
onItemTapped.invoke(ToolbarMenu.Item.FindInPage)
}
val reportSiteIssuePlaceholder = WebExtensionPlaceholderMenuItem(
id = WebCompatReporterFeature.WEBCOMPAT_REPORTER_EXTENSION_ID
)
val saveToCollection = BrowserMenuImageText(
label = context.getString(R.string.browser_menu_save_to_collection_2),
imageResource = R.drawable.ic_tab_collection,
iconTintColorResource = primaryTextColor()
) {
onItemTapped.invoke(ToolbarMenu.Item.SaveToCollection)
}
val deleteDataOnQuit = BrowserMenuImageText(
label = context.getString(R.string.delete_browsing_data_on_quit_action),
imageResource = R.drawable.ic_exit,
iconTintColorResource = primaryTextColor()
) {
onItemTapped.invoke(ToolbarMenu.Item.Quit)
}
val readerAppearance = BrowserMenuImageText(
label = context.getString(R.string.browser_menu_read_appearance),
imageResource = R.drawable.ic_readermode_appearance,
iconTintColorResource = primaryTextColor()
) {
onItemTapped.invoke(ToolbarMenu.Item.CustomizeReaderView)
}
val openInApp = BrowserMenuHighlightableItem(
label = context.getString(R.string.browser_menu_open_app_link),
startImageResource = R.drawable.ic_open_in_app,
iconTintColorResource = primaryTextColor(),
highlight = BrowserMenuHighlight.LowPriority(
label = context.getString(R.string.browser_menu_open_app_link),
notificationTint = getColor(context, R.color.whats_new_notification_color)
),
isHighlighted = { !context.settings().openInAppOpened }
) {
onItemTapped.invoke(ToolbarMenu.Item.OpenInApp)
}
val historyItem = BrowserMenuImageText(
context.getString(R.string.library_history),
R.drawable.ic_history,
primaryTextColor()
) {
onItemTapped.invoke(ToolbarMenu.Item.History)
}
val bookmarksItem = BrowserMenuImageText(
context.getString(R.string.library_bookmarks),
R.drawable.ic_bookmark_filled,
primaryTextColor()
) {
onItemTapped.invoke(ToolbarMenu.Item.Bookmarks)
}
val downloadsItem = BrowserMenuImageText(
context.getString(R.string.library_downloads),
R.drawable.ic_download,
primaryTextColor()
) {
onItemTapped.invoke(ToolbarMenu.Item.Downloads)
}
// Predicates that are called once, during screen init
val shouldShowSaveToCollection = (context.asActivity() as? HomeActivity)
?.browsingModeManager?.mode == BrowsingMode.Normal
val shouldDeleteDataOnQuit = context.components.settings
.shouldDeleteBrowsingDataOnQuit
val menuItems = listOfNotNull(
downloadsItem,
historyItem,
bookmarksItem,
syncedTabs,
settings,
if (shouldDeleteDataOnQuit) deleteDataOnQuit else null,
BrowserMenuDivider(),
reportSiteIssuePlaceholder,
findInPage,
getSetDefaultBrowserItem()?.let { BrowserMenuDivider() },
getSetDefaultBrowserItem(),
getSetDefaultBrowserItem()?.let { BrowserMenuDivider() },
addToTopSites,
addToHomescreen.apply { visible = ::canAddToHomescreen },
installToHomescreen.apply { visible = ::canInstall },
if (shouldShowSaveToCollection) saveToCollection else null,
desktopMode,
openInApp.apply { visible = ::shouldShowOpenInApp },
readerAppearance.apply { visible = ::shouldShowReaderViewCustomization },
BrowserMenuDivider(),
menuToolbar
)
if (shouldUseBottomToolbar) {
menuItems
} else {
menuItems.reversed()
}
}
val newTabItem = BrowserMenuImageText(
context.getString(R.string.library_new_tab),
R.drawable.ic_new,
@ -554,7 +361,7 @@ open class DefaultToolbarMenu(
}
@VisibleForTesting(otherwise = PRIVATE)
val newCoreMenuItems by lazy {
val coreMenuItems by lazy {
val menuItems =
listOfNotNull(
if (shouldUseBottomToolbar) null else menuToolbar,

116
app/src/main/java/org/mozilla/fenix/home/HomeMenu.kt
Unescape Escape View File

@ -23,7 +23,6 @@ import mozilla.components.concept.sync.AccountObserver
import mozilla.components.concept.sync.AuthType
import mozilla.components.concept.sync.OAuthAccount
import mozilla.components.support.ktx.android.content.getColorFromAttr
import org.mozilla.fenix.FeatureFlags
import org.mozilla.fenix.FeatureFlags.tabsTrayRewrite
import org.mozilla.fenix.R
import org.mozilla.fenix.components.accounts.FenixAccountManager
@ -120,104 +119,6 @@ class HomeMenu(
onItemTapped.invoke(Item.SyncAccount(accountManager.signedInToFxa()))
}
private val oldCoreMenuItems by lazy {
val whatsNewItem = BrowserMenuHighlightableItem(
context.getString(R.string.browser_menu_whats_new),
R.drawable.ic_whats_new,
iconTintColorResource = primaryTextColor,
highlight = BrowserMenuHighlight.LowPriority(
notificationTint = getColor(context, R.color.whats_new_notification_color)
),
isHighlighted = { WhatsNew.shouldHighlightWhatsNew(context) }
) {
onItemTapped.invoke(Item.WhatsNew)
}
val experiments = context.components.analytics.experiments
val bookmarksItem = BrowserMenuImageText(
context.getString(R.string.library_bookmarks),
R.drawable.ic_bookmark_list,
primaryTextColor
) {
onItemTapped.invoke(Item.Bookmarks)
}
val historyItem = BrowserMenuImageText(
context.getString(R.string.library_history),
R.drawable.ic_history,
primaryTextColor
) {
onItemTapped.invoke(Item.History)
}
val addons = BrowserMenuImageText(
context.getString(R.string.browser_menu_add_ons),
R.drawable.ic_addons_extensions,
primaryTextColor
) {
onItemTapped.invoke(Item.Extensions)
}
// Use nimbus to set the icon and title.
val variables = experiments.getVariables(FeatureId.NIMBUS_VALIDATION)
val settingsItem = BrowserMenuImageText(
variables.getText("settings-title") ?: context.getString(R.string.browser_menu_settings),
variables.getDrawableResource("settings-icon") ?: R.drawable.ic_settings,
primaryTextColor
) {
onItemTapped.invoke(Item.Settings)
}
val helpItem = BrowserMenuImageText(
context.getString(R.string.browser_menu_help),
R.drawable.ic_help,
primaryTextColor
) {
onItemTapped.invoke(Item.Help)
}
val downloadsItem = BrowserMenuImageText(
context.getString(R.string.library_downloads),
R.drawable.ic_download,
primaryTextColor
) {
onItemTapped.invoke(Item.Downloads)
}
// Only query account manager if it has been initialized.
// We don't want to cause its initialization just for this check.
val accountAuthItem = if (context.components.backgroundServices.accountManagerAvailableQueue.isReady()) {
if (context.components.backgroundServices.accountManager.accountNeedsReauth()) reconnectToSyncItem else null
} else {
null
}
val settings = context.components.settings
val menuItems = listOfNotNull(
if (settings.shouldDeleteBrowsingDataOnQuit) quitItem else null,
settingsItem,
BrowserMenuDivider(),
syncedTabsItem,
bookmarksItem,
historyItem,
downloadsItem,
BrowserMenuDivider(),
addons,
BrowserMenuDivider(),
whatsNewItem,
helpItem,
accountAuthItem
).also { items ->
items.getHighlight()?.let { onHighlightPresent(it) }
}
if (shouldUseBottomToolbar) {
menuItems.reversed()
} else {
menuItems
}
}
val desktopItem = BrowserMenuImageSwitch(
imageResource = R.drawable.ic_desktop,
label = context.getString(R.string.browser_menu_desktop_site),
@ -227,7 +128,7 @@ class HomeMenu(
}
@Suppress("ComplexMethod")
private fun newCoreMenuItems(): List<BrowserMenuItem> {
private fun coreMenuItems(): List<BrowserMenuItem> {
val experiments = context.components.analytics.experiments
val settings = context.components.settings
@ -325,22 +226,11 @@ class HomeMenu(
}
init {
val menuItems = if (FeatureFlags.toolbarMenuFeature) {
newCoreMenuItems()
} else {
oldCoreMenuItems
}
val menuItems = coreMenuItems()
// Report initial state.
onMenuBuilderChanged(BrowserMenuBuilder(menuItems))
val menuItemsWithReconnectItem = if (FeatureFlags.toolbarMenuFeature) {
menuItems
} else {
// reconnect item is manually added to the beginning of the list
listOf(reconnectToSyncItem) + menuItems
}
// Observe account state changes, and update menu item builder with a new set of items.
context.components.backgroundServices.accountManagerAvailableQueue.runIfReadyOrQueue {
// This task isn't relevant if our parent fragment isn't around anymore.
@ -352,7 +242,7 @@ class HomeMenu(
lifecycleOwner.lifecycleScope.launch(Dispatchers.Main) {
onMenuBuilderChanged(
BrowserMenuBuilder(
menuItemsWithReconnectItem
menuItems
)
)
}

97
app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarMenuControllerTest.kt
Unescape Escape View File

@ -48,7 +48,6 @@ import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.mozilla.fenix.FeatureFlags
import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.NavGraphDirections
import org.mozilla.fenix.R
@ -68,7 +67,6 @@ import org.mozilla.fenix.utils.Settings
@OptIn(ExperimentalCoroutinesApi::class)
@RunWith(FenixRobolectricTestRunner::class)
@Suppress("ForbiddenComment")
class DefaultBrowserToolbarMenuControllerTest {
@get:Rule
@ -137,76 +135,67 @@ class DefaultBrowserToolbarMenuControllerTest {
unmockkObject(FenixSnackbar.Companion)
}
// TODO: These can be removed for https://github.com/mozilla-mobile/fenix/issues/17870
// todo === Start ===
@Test
fun handleToolbarBookmarkPressWithReaderModeInactive() = runBlockingTest {
if (!FeatureFlags.toolbarMenuFeature) {
val item = ToolbarMenu.Item.Bookmark
val title = "Mozilla"
val url = "https://mozilla.org"
val regularTab = createTab(
url = url,
readerState = ReaderState(active = false, activeUrl = "https://1234.org"),
title = title
)
val store =
BrowserStore(BrowserState(tabs = listOf(regularTab), selectedTabId = regularTab.id))
val item = ToolbarMenu.Item.Bookmark
val controller = createController(scope = this, store = store)
controller.handleToolbarItemInteraction(item)
val title = "Mozilla"
val url = "https://mozilla.org"
val regularTab = createTab(
url = url,
readerState = ReaderState(active = false, activeUrl = "https://1234.org"),
title = title
)
val store =
BrowserStore(BrowserState(tabs = listOf(regularTab), selectedTabId = regularTab.id))
verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.BOOKMARK)) }
verify { bookmarkTapped(url, title) }
}
val controller = createController(scope = this, store = store)
controller.handleToolbarItemInteraction(item)
verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.BOOKMARK)) }
verify { bookmarkTapped(url, title) }
}
@Test
fun `IF reader mode is active WHEN bookmark menu item is pressed THEN menu item is handled`() = runBlockingTest {
if (!FeatureFlags.toolbarMenuFeature) {
val item = ToolbarMenu.Item.Bookmark
val title = "Mozilla"
val readerUrl = "moz-extension://1234"
val readerTab = createTab(
url = readerUrl,
readerState = ReaderState(active = true, activeUrl = "https://mozilla.org"),
title = title
)
browserStore =
BrowserStore(BrowserState(tabs = listOf(readerTab), selectedTabId = readerTab.id))
val item = ToolbarMenu.Item.Bookmark
val title = "Mozilla"
val readerUrl = "moz-extension://1234"
val readerTab = createTab(
url = readerUrl,
readerState = ReaderState(active = true, activeUrl = "https://mozilla.org"),
title = title
)
browserStore =
BrowserStore(BrowserState(tabs = listOf(readerTab), selectedTabId = readerTab.id))
val controller = createController(scope = this, store = browserStore)
controller.handleToolbarItemInteraction(item)
val controller = createController(scope = this, store = browserStore)
controller.handleToolbarItemInteraction(item)
verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.BOOKMARK)) }
verify { bookmarkTapped("https://mozilla.org", title) }
}
verify { metrics.track(Event.BrowserMenuItemTapped(Event.BrowserMenuItemTapped.Item.BOOKMARK)) }
verify { bookmarkTapped("https://mozilla.org", title) }
}
@Test
fun `WHEN open in Fenix menu item is pressed THEN menu item is handled correctly`() = runBlockingTest {
if (!FeatureFlags.toolbarMenuFeature) {
val customTab = createCustomTab("https://mozilla.org")
browserStore.dispatch(CustomTabListAction.AddCustomTabAction(customTab)).joinBlocking()
val controller = createController(
scope = this,
store = browserStore,
customTabSessionId = customTab.id
)
val customTab = createCustomTab("https://mozilla.org")
browserStore.dispatch(CustomTabListAction.AddCustomTabAction(customTab)).joinBlocking()
val controller = createController(
scope = this,
store = browserStore,
customTabSessionId = customTab.id
)
val item = ToolbarMenu.Item.OpenInFenix
val item = ToolbarMenu.Item.OpenInFenix
every { activity.startActivity(any()) } just Runs
controller.handleToolbarItemInteraction(item)
every { activity.startActivity(any()) } just Runs
controller.handleToolbarItemInteraction(item)
verify { sessionFeature.release() }
verify { customTabUseCases.migrate(customTab.id, true) }
verify { activity.startActivity(openInFenixIntent) }
verify { activity.finishAndRemoveTask() }
}
verify { sessionFeature.release() }
verify { customTabUseCases.migrate(customTab.id, true) }
verify { activity.startActivity(openInFenixIntent) }
verify { activity.finishAndRemoveTask() }
}
// todo === End ===
@Test
fun `WHEN reader mode menu item is pressed THEN handle appearance change`() = runBlockingTest {

65
app/src/test/java/org/mozilla/fenix/toolbar/DefaultToolbarMenuTest.kt
Unescape Escape View File

@ -26,7 +26,6 @@ import org.junit.Before
import org.junit.Ignore
import org.junit.Rule
import org.junit.Test
import org.mozilla.fenix.FeatureFlags
import org.mozilla.fenix.components.toolbar.DefaultToolbarMenu
import org.mozilla.fenix.ext.settings
@ -88,70 +87,62 @@ class DefaultToolbarMenuTest {
@Test
@Ignore("Intermittent test: https://github.com/mozilla-mobile/fenix/issues/18822")
fun `WHEN the bottom toolbar is set THEN the first item in the list is not the navigation`() {
if (FeatureFlags.toolbarMenuFeature) {
every { context.settings().shouldUseBottomToolbar } returns true
createMenu()
every { context.settings().shouldUseBottomToolbar } returns true
createMenu()
val menuItems = toolbarMenu.newCoreMenuItems
assertNotNull(menuItems)
val menuItems = toolbarMenu.coreMenuItems
assertNotNull(menuItems)
val firstItem = menuItems[0]
val newTabItem = toolbarMenu.newTabItem
val firstItem = menuItems[0]
val newTabItem = toolbarMenu.newTabItem
assertEquals(newTabItem, firstItem)
}
assertEquals(newTabItem, firstItem)
}
@Test
@Ignore("Intermittent test: https://github.com/mozilla-mobile/fenix/issues/18822")
fun `WHEN the top toolbar is set THEN the first item in the list is the navigation`() {
if (FeatureFlags.toolbarMenuFeature) {
every { context.settings().shouldUseBottomToolbar } returns false
createMenu()
every { context.settings().shouldUseBottomToolbar } returns false
createMenu()
val menuItems = toolbarMenu.newCoreMenuItems
assertNotNull(menuItems)
val menuItems = toolbarMenu.coreMenuItems
assertNotNull(menuItems)
val firstItem = menuItems[0]
val navToolbar = toolbarMenu.menuToolbar
val firstItem = menuItems[0]
val navToolbar = toolbarMenu.menuToolbar
assertEquals(navToolbar, firstItem)
}
assertEquals(navToolbar, firstItem)
}
@Test
@Ignore("Intermittent test: https://github.com/mozilla-mobile/fenix/issues/18822")
fun `WHEN the bottom toolbar is set THEN the nav menu should be the last item`() {
if (FeatureFlags.toolbarMenuFeature) {
every { context.settings().shouldUseBottomToolbar } returns true
every { context.settings().shouldUseBottomToolbar } returns true
createMenu()
createMenu()
val menuItems = toolbarMenu.newCoreMenuItems
assertNotNull(menuItems)
val menuItems = toolbarMenu.coreMenuItems
assertNotNull(menuItems)
val lastItem = menuItems[menuItems.size - 1]
val navToolbar = toolbarMenu.menuToolbar
val lastItem = menuItems[menuItems.size - 1]
val navToolbar = toolbarMenu.menuToolbar
assertEquals(navToolbar, lastItem)
}
assertEquals(navToolbar, lastItem)
}
@Test
@Ignore("Intermittent test: https://github.com/mozilla-mobile/fenix/issues/18822")
fun `WHEN the top toolbar is set THEN settings should be the last item`() {
if (FeatureFlags.toolbarMenuFeature) {
every { context.settings().shouldUseBottomToolbar } returns false
every { context.settings().shouldUseBottomToolbar } returns false
createMenu()
createMenu()
val menuItems = toolbarMenu.newCoreMenuItems
assertNotNull(menuItems)
val menuItems = toolbarMenu.coreMenuItems
assertNotNull(menuItems)
val lastItem = menuItems[menuItems.size - 1]
val settingsItem = toolbarMenu.settingsItem
val lastItem = menuItems[menuItems.size - 1]
val settingsItem = toolbarMenu.settingsItem
assertEquals(settingsItem, lastItem)
}
assertEquals(settingsItem, lastItem)
}
}

Write Preview
Loading…
Cancel
Save