Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,7 @@ enum class AnalyticsEvent(override val siteless: Boolean = false) : IAnalyticsEv
MAIN_TAB_ORDERS_RESELECTED,
MAIN_TAB_PRODUCTS_SELECTED,
MAIN_TAB_PRODUCTS_RESELECTED,
MAIN_TAB_POS_SELECTED,
MAIN_TAB_HUB_MENU_SELECTED,
MAIN_TAB_HUB_MENU_RESELECTED,

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,22 @@ import com.woocommerce.android.R
const val MY_STORE_POSITION = 0
const val ORDERS_POSITION = 1
const val PRODUCTS_POSITION = 2
const val MORE_POSITION = 3
const val POS_POSITION = 3
const val MORE_POSITION = 4

enum class BottomNavigationPosition(val position: Int, val id: Int) {
MY_STORE(MY_STORE_POSITION, R.id.dashboard),
ORDERS(ORDERS_POSITION, R.id.orders),
PRODUCTS(PRODUCTS_POSITION, R.id.products),
POS(POS_POSITION, R.id.point_of_sale),
MORE(MORE_POSITION, R.id.moreMenu)
}

fun findNavigationPositionById(id: Int): BottomNavigationPosition = when (id) {
BottomNavigationPosition.MY_STORE.id -> BottomNavigationPosition.MY_STORE
BottomNavigationPosition.ORDERS.id -> BottomNavigationPosition.ORDERS
BottomNavigationPosition.PRODUCTS.id -> BottomNavigationPosition.PRODUCTS
BottomNavigationPosition.POS.id -> BottomNavigationPosition.POS
BottomNavigationPosition.MORE.id -> BottomNavigationPosition.MORE
else -> BottomNavigationPosition.MY_STORE
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ import com.woocommerce.android.ui.login.LoginActivity
import com.woocommerce.android.ui.main.BottomNavigationPosition.MORE
import com.woocommerce.android.ui.main.BottomNavigationPosition.MY_STORE
import com.woocommerce.android.ui.main.BottomNavigationPosition.ORDERS
import com.woocommerce.android.ui.main.BottomNavigationPosition.POS
import com.woocommerce.android.ui.main.BottomNavigationPosition.PRODUCTS
import com.woocommerce.android.ui.main.MainActivityViewModel.BottomBarState
import com.woocommerce.android.ui.main.MainActivityViewModel.MoreMenuBadgeState.Hidden
Expand Down Expand Up @@ -749,18 +750,28 @@ class MainActivity :
binding.bottomNav.setOrderBadgeCount(0)
}

override fun onNavItemSelected(navPos: BottomNavigationPosition) {
override fun onNavItemSelected(navPos: BottomNavigationPosition): Boolean {
val stat = when (navPos) {
MY_STORE -> AnalyticsEvent.MAIN_TAB_DASHBOARD_SELECTED
ORDERS -> AnalyticsEvent.MAIN_TAB_ORDERS_SELECTED
PRODUCTS -> AnalyticsEvent.MAIN_TAB_PRODUCTS_SELECTED
POS -> AnalyticsEvent.MAIN_TAB_POS_SELECTED
MORE -> AnalyticsEvent.MAIN_TAB_HUB_MENU_SELECTED
}
AnalyticsTracker.track(stat, mapOf(KEY_HORIZONTAL_SIZE_CLASS to deviceTypeToAnalyticsString))

if (navPos == ORDERS) {
viewModel.removeOrderNotifications()
}

if (navPos == POS) {
posTabController.navigateToPOS()

// Do not keep the tab selected for POS
return false
}

return true
}

override fun onNavItemReselected(navPos: BottomNavigationPosition) {
Expand All @@ -769,8 +780,11 @@ class MainActivity :
ORDERS -> AnalyticsEvent.MAIN_TAB_ORDERS_RESELECTED
PRODUCTS -> AnalyticsEvent.MAIN_TAB_PRODUCTS_RESELECTED
MORE -> AnalyticsEvent.MAIN_TAB_HUB_MENU_RESELECTED
else -> null
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The POS tab cannot be reselected.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 I'd consider explicitly listing POS instead of relying on else.
If someone introduces a new BottomNavigationPosition, they might forget to update this when, however, if the when doesn't have else statement, the compiler will inform them about their mistake.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! Done in 10aff47

}
stat?.let {
AnalyticsTracker.track(it, mapOf(KEY_HORIZONTAL_SIZE_CLASS to deviceTypeToAnalyticsString))
}
AnalyticsTracker.track(stat, mapOf(KEY_HORIZONTAL_SIZE_CLASS to deviceTypeToAnalyticsString))

// if we're at the root scroll the active fragment to the top
// TODO bring back clearing the backstack when the navgraphs are fixed to support multiple backstacks:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class MainBottomNavigationView @JvmOverloads constructor(
private lateinit var moreMenuBadge: BadgeDrawable

interface MainNavigationListener {
fun onNavItemSelected(navPos: BottomNavigationPosition)
fun onNavItemSelected(navPos: BottomNavigationPosition): Boolean
fun onNavItemReselected(navPos: BottomNavigationPosition)
}

Expand Down Expand Up @@ -135,11 +135,8 @@ class MainBottomNavigationView @JvmOverloads constructor(

override fun onNavigationItemSelected(item: MenuItem): Boolean {
navController?.let { navController ->
val navSuccess = NavigationUI.onNavDestinationSelected(item, navController)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's necessary to call listener.onNavItemSelected only when onNavDestinationSelected is successful. The listener's name suggests that it's triggered when an item is selected, regardless of whether navigation occurred. We need to adjust this for the POS tab, where navigation follows a custom flow and doesn't succeed through the usual mechanism.

if (navSuccess) {
listener.onNavItemSelected(findNavigationPositionById(item.itemId))
return true
}
NavigationUI.onNavDestinationSelected(item, navController)
return listener.onNavItemSelected(findNavigationPositionById(item.itemId))
}
return false
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,24 @@ import androidx.lifecycle.DefaultLifecycleObserver
import androidx.lifecycle.LifecycleOwner
import androidx.lifecycle.lifecycleScope
import androidx.navigation.NavController
import androidx.navigation.ui.NavigationUI
import com.woocommerce.android.AppPrefs
import com.woocommerce.android.R
import com.woocommerce.android.databinding.ActivityMainBinding
import com.woocommerce.android.tools.SelectedSite
import com.woocommerce.android.ui.main.MainActivity
import com.woocommerce.android.ui.woopos.WooPosIsEnabled
import com.woocommerce.android.ui.woopos.root.WooPosActivity
import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsEvent
import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsTracker
import kotlinx.coroutines.launch
import javax.inject.Inject

class WooPosTabController @Inject constructor(
private val appPrefs: AppPrefs,
private val selectedSite: SelectedSite,
private val isWooPosEnabled: WooPosIsEnabled,
private val isPosAsTabEnabled: WooPosIsPosAsTabEnabled
private val isPosAsTabEnabled: WooPosIsPosAsTabEnabled,
private val analyticsTracker: WooPosAnalyticsTracker
) : DefaultLifecycleObserver {

private lateinit var activity: MainActivity
Expand All @@ -37,11 +39,7 @@ class WooPosTabController @Inject constructor(
this.navController = navController

activity.lifecycle.addObserver(this)
}

override fun onCreate(owner: LifecycleOwner) {
super.onCreate(owner)
setupPOSTab()
setPOSTabVisibility(false)
}

override fun onResume(owner: LifecycleOwner) {
Expand All @@ -54,13 +52,6 @@ class WooPosTabController @Inject constructor(
owner.lifecycle.removeObserver(this)
}

private fun setupPOSTab() {
setPOSTabVisibility(false)
if (isPosAsTabEnabled()) {
setupPOSTabNavigation()
}
}

fun refreshPOSTabVisibility() {
setPOSTabVisibility(false)
if (isPosAsTabEnabled()) {
Expand All @@ -72,6 +63,10 @@ class WooPosTabController @Inject constructor(
}
}

fun navigateToPOS() {
activity.startActivity(Intent(activity, WooPosActivity::class.java))
}

private fun updatePOSTabVisibilityFromPrefs() = setPOSTabVisibility(
appPrefs.isPOSTabVisibleForSite(selectedSite.getSelectedSiteId())
)
Expand All @@ -81,22 +76,11 @@ class WooPosTabController @Inject constructor(
val isWooPosEnabledValue = isWooPosEnabled()
setPOSTabVisibility(isWooPosEnabledValue)
appPrefs.setPOSTabVisibilityForSite(selectedSite.getSelectedSiteId(), isWooPosEnabledValue)
analyticsTracker.track(WooPosAnalyticsEvent.Event.TabVisibilityChecked(isWooPosEnabledValue))
}
}

private fun setPOSTabVisibility(isVisible: Boolean) {
binding.bottomNav.menu.findItem(R.id.point_of_sale)?.isVisible = isVisible
}

private fun setupPOSTabNavigation() {
binding.bottomNav.setOnItemSelectedListener { item ->
when (item.itemId) {
R.id.point_of_sale -> {
activity.startActivity(Intent(activity, WooPosActivity::class.java))
false // return false to *not* keep the tab selected
}
else -> NavigationUI.onNavDestinationSelected(item, navController)
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,19 @@ sealed class WooPosAnalyticsEvent : IAnalyticsEvent {
)
}
}

data class TabVisibilityChecked(val isVisible: Boolean) : Event() {
override val name: String = "tab_visibility_checked"

init {
addProperties(
mapOf(
"is_visible" to isVisible.toString()
)
)
}
}

data object ExitTapped : Event() {
override val name: String = "exit_menu_item_tapped"
}
Expand Down