Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Products] Use Authenticated WebView for previewing products #13494

Merged
merged 8 commits into from
Feb 21, 2025
Merged
1 change: 1 addition & 0 deletions RELEASE-NOTES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
- [*] Improved the Authenticated WebView implementation, and it will now handle opening the Analytics reports directly from the app without requiring additional authentication [https://github.com/woocommerce/woocommerce-android/pull/13487]
- [*] [Internal] All non-login tracking events will now include the selected site data [https://github.com/woocommerce/woocommerce-android/pull/13575]
- [*] Fixed a crash that occurred when the app was backgrounded during bulk variations update [https://github.com/woocommerce/woocommerce-android/pull/13584]
- [*] Improved handling of product previews by using the Authenticated WebView in supported scenarios [https://github.com/woocommerce/woocommerce-android/pull/13494]

21.7
-----
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import android.view.View
import androidx.annotation.IdRes
import androidx.fragment.app.Fragment
import androidx.lifecycle.lifecycleScope
import androidx.navigation.NavController
import androidx.navigation.fragment.NavHostFragment
import androidx.navigation.fragment.findNavController
import com.google.android.material.appbar.AppBarLayout
import com.google.android.material.datepicker.CalendarConstraints
Expand Down Expand Up @@ -31,11 +33,18 @@ import kotlin.math.abs
* @param [destinationId] an optional destinationId, that can be used to navigate up to a specified destination
*
*/
fun <T> Fragment.navigateBackWithResult(key: String, result: T, @IdRes destinationId: Int? = null) {
fun <T> Fragment.navigateBackWithResult(
key: String,
result: T,
@IdRes destinationId: Int? = null,
@IdRes navHostId: Int? = null,
) {
val navController = if (navHostId != null) findNavController(navHostId) else findNavController()

val entry = if (destinationId != null) {
findNavController().getBackStackEntry(destinationId)
navController.getBackStackEntry(destinationId)
} else {
findNavController().previousBackStackEntry
navController.previousBackStackEntry
}
entry?.savedStateHandle?.set(key, result)

Expand All @@ -54,7 +63,7 @@ fun <T> Fragment.navigateBackWithResult(key: String, result: T, @IdRes destinati
* @param [key] A unique string that is the same as the one used in [handleResult]
* @param [result] A result value to be returned
* @param [childId] an destinationId, that used to navigate up from the specified destination
*
* @param [navHostId] An optional ID of the NavHostFragment, it's useful when the fragment is used in two-pane layouts
*/
fun <T> Fragment.navigateToParentWithResult(key: String, result: T, @IdRes childId: Int) {
if (findNavController().currentDestination?.id != childId) {
Expand All @@ -70,9 +79,14 @@ fun <T> Fragment.navigateToParentWithResult(key: String, result: T, @IdRes child
*
* @param [key] A unique string that is the same as the one used in [handleNotice]
* @param [destinationId] an optional destinationId, that can be used to navigating up to a specified destination
* @param [navHostId] An optional ID of the NavHostFragment, it's useful when the fragment is used in two-pane layouts
*/
fun Fragment.navigateBackWithNotice(key: String, @IdRes destinationId: Int? = null) {
navigateBackWithResult(key, key, destinationId)
fun Fragment.navigateBackWithNotice(
key: String,
@IdRes destinationId: Int? = null,
@IdRes navHostId: Int? = null,
) {
navigateBackWithResult(key, key, destinationId, navHostId)
}

/**
Expand All @@ -82,17 +96,25 @@ fun Fragment.navigateBackWithNotice(key: String, @IdRes destinationId: Int? = nu
* @param [key] A unique string that is the same as the one used in [navigateBackWithResult]
* @param [entryId] An optional ID to identify the correct back stack entry. It's required when calling [handleResult]
* from TopLevelFragment or Dialog (otherwise the result will get lost upon configuration change)
* @param [navHostId] An optional ID of the NavHostFragment, it's useful when the fragment is used in two-pane layouts
* @param [handler] A result handler
*
* Note: The handler is called only if the value wasn't handled before (i.e. the data is fresh). Once the observer is
* called, the boolean value is updated. This puts a limit on the number of observers for a particular key-result pair
* to 1.
*/
fun <T> Fragment.handleResult(key: String, entryId: Int? = null, handler: (T) -> Unit) {
fun <T> Fragment.handleResult(
key: String,
@IdRes entryId: Int? = null,
@IdRes navHostId: Int? = null,
handler: (T) -> Unit
) {
val navController = if (navHostId != null) findNavController(navHostId) else findNavController()

val entry = if (entryId != null) {
findNavController().getBackStackEntry(entryId)
navController.getBackStackEntry(entryId)
} else {
findNavController().currentBackStackEntry
navController.currentBackStackEntry
}

entry?.savedStateHandle?.let { saveState ->
Expand All @@ -113,14 +135,20 @@ fun <T> Fragment.handleResult(key: String, entryId: Int? = null, handler: (T) ->
* @param [key] A unique string that is the same as the one used in [navigateBackWithResult]
* @param [entryId] A mandatory ID to identify the correct back stack entry. It's required when calling [handleResult]
* from TopLevelFragment or Dialog (otherwise the result will get lost upon configuration change)
* @param [navHostId] An optional ID of the NavHostFragment, it's useful when the fragment is used in two-pane layouts
* @param [handler] A result handler
*
* Note: The handler is called only if the value wasn't handled before (i.e. the data is fresh). Once the observer is
* called, the value is nulled and the handler won't be called. This puts a limit on the number of observers for
* a particular key-result pair to 1.
*/
fun <T> Fragment.handleDialogResult(key: String, entryId: Int, handler: (T) -> Unit) {
handleResult(key, entryId, handler)
fun <T> Fragment.handleDialogResult(
key: String,
@IdRes entryId: Int,
@IdRes navHostId: Int? = null,
handler: (T) -> Unit
) {
handleResult(key, entryId, navHostId, handler)
}

/**
Expand All @@ -131,14 +159,20 @@ fun <T> Fragment.handleDialogResult(key: String, entryId: Int, handler: (T) -> U
* @param [key] A unique string that is the same as the one used in [navigateBackWithNotice]
* @param [entryId] A mandatory ID to identify the correct back stack entry. It's required when calling [handleNotice]
* from TopLevelFragment or Dialog (otherwise the result will get lost upon configuration change)
* @param [navHostId] An optional ID of the NavHostFragment, it's useful when the fragment is used in two-pane layouts
* @param [handler] A result handler
*
* Note: The handler is called only if the value wasn't handled before (i.e. the data is fresh). Once the observer is
* called, the value is nulled and the handler won't be called. This puts a limit on the number of observers for
* a particular key-result pair to 1.
*/
fun Fragment.handleDialogNotice(key: String, entryId: Int, handler: () -> Unit) {
handleNotice(key, entryId, handler)
fun Fragment.handleDialogNotice(
key: String,
@IdRes entryId: Int,
@IdRes navHostId: Int? = null,
handler: () -> Unit
) {
handleNotice(key, entryId, navHostId, handler)
}

/**
Expand All @@ -148,17 +182,25 @@ fun Fragment.handleDialogNotice(key: String, entryId: Int, handler: () -> Unit)
* @param [key] A unique string that is the same as the one used in [navigateBackWithNotice]
* @param [entryId] A mandatory ID to identify the correct back stack entry. It's required when calling [handleNotice]
* from TopLevelFragment or Dialog (otherwise the result will get lost upon configuration change)
* @param [navHostId] An optional ID of the NavHostFragment, it's useful when the fragment is used in two-pane layouts
* @param [handler] A result handler
*
* Note: The handler is called only if the value wasn't handled before (i.e. the data is fresh). Once the observer is
* called, the value is nulled and the handler won't be called. This puts a limit on the number of observers for
* a particular key-result pair to 1.
*/
fun Fragment.handleNotice(key: String, entryId: Int? = null, handler: () -> Unit) {
fun Fragment.handleNotice(
key: String,
@IdRes entryId: Int? = null,
@IdRes navHostId: Int? = null,
handler: () -> Unit
) {
val navController = if (navHostId != null) findNavController(navHostId) else findNavController()

val entry = if (entryId != null) {
findNavController().getBackStackEntry(entryId)
navController.getBackStackEntry(entryId)
} else {
findNavController().currentBackStackEntry
navController.currentBackStackEntry
}

entry?.savedStateHandle?.let { saveState ->
Expand Down Expand Up @@ -236,3 +278,21 @@ fun Fragment.showDateRangePicker(
onCustomRangeSelected(start, end)
}
}

/**
* Finds the NavController associated with the NavHostFragment with the given ID,
* useful when trying to find a parent NavController from a child fragment used in two-pane layouts.
*
* @param navHostId The ID of the NavHostFragment
* @return The NavController
*/
fun Fragment.findNavController(@IdRes navHostId: Int): NavController {
var parentFragment = parentFragment
while (parentFragment != null) {
if (parentFragment is NavHostFragment && parentFragment.id == navHostId) {
return parentFragment.navController
}
parentFragment = parentFragment.parentFragment
}
error("No NavHostFragment found with id $navHostId")
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,12 @@ import android.view.View
import androidx.appcompat.content.res.AppCompatResources
import androidx.core.view.isVisible
import androidx.fragment.app.viewModels
import androidx.navigation.fragment.findNavController
import androidx.recyclerview.widget.LinearLayoutManager
import com.woocommerce.android.AppUrls
import com.woocommerce.android.AppUrls.FETCH_PAYMENT_METHOD_URL_PATH
import com.woocommerce.android.NavGraphOrdersDirections
import com.woocommerce.android.NavGraphMainDirections
import com.woocommerce.android.R
import com.woocommerce.android.databinding.FragmentEditShippingLabelPaymentBinding
import com.woocommerce.android.extensions.findNavController
import com.woocommerce.android.extensions.handleNotice
import com.woocommerce.android.extensions.navigateBackWithNotice
import com.woocommerce.android.extensions.navigateBackWithResult
Expand Down Expand Up @@ -198,10 +197,10 @@ class EditShippingLabelPaymentFragment :
viewModel.event.observe(viewLifecycleOwner) { event ->
when (event) {
is AddPaymentMethod -> {
findNavController().navigateSafely(
NavGraphOrdersDirections.actionGlobalAuthenticatedWebViewFragment(
findNavController(R.id.nav_host_fragment_main).navigateSafely(
NavGraphMainDirections.actionGlobalAuthenticatedWebViewFragment(
urlToLoad = AppUrls.WPCOM_ADD_PAYMENT_METHOD,
urlsToTriggerExit = arrayOf(FETCH_PAYMENT_METHOD_URL_PATH),
urlsToTriggerExit = arrayOf(AppUrls.FETCH_PAYMENT_METHOD_URL_PATH),
title = getFragmentTitle()
)
)
Expand All @@ -215,7 +214,7 @@ class EditShippingLabelPaymentFragment :
}

private fun setupResultHandlers() {
handleNotice(AuthenticatedWebViewFragment.WEBVIEW_RESULT) {
handleNotice(AuthenticatedWebViewFragment.WEBVIEW_RESULT, navHostId = R.id.nav_host_fragment_main) {
Copy link
Member Author

Choose a reason for hiding this comment

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

We need to set the navHostId for the result handler too to make sure we are using the correct NavController.

viewModel.onPaymentMethodAdded()
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@ import com.google.android.material.dialog.MaterialAlertDialogBuilder
import com.google.android.material.snackbar.Snackbar
import com.google.android.material.transition.MaterialContainerTransform
import com.woocommerce.android.AppUrls
import com.woocommerce.android.NavGraphMainDirections
import com.woocommerce.android.R
import com.woocommerce.android.RequestCodes
import com.woocommerce.android.analytics.AnalyticsEvent
import com.woocommerce.android.analytics.AnalyticsTracker
import com.woocommerce.android.databinding.FragmentProductDetailBinding
import com.woocommerce.android.extensions.fastStripHtml
import com.woocommerce.android.extensions.findNavController
import com.woocommerce.android.extensions.handleNotice
import com.woocommerce.android.extensions.handleResult
import com.woocommerce.android.extensions.hide
Expand Down Expand Up @@ -85,6 +87,7 @@ import com.woocommerce.android.ui.promobanner.PromoBannerType
import com.woocommerce.android.util.ChromeCustomTabUtils
import com.woocommerce.android.util.UiHelpers.getTextOfUiString
import com.woocommerce.android.util.WooAnimUtils
import com.woocommerce.android.viewmodel.MultiLiveEvent.Event
import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.LaunchUrlInChromeTab
import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.ShowUiStringSnackbar
import com.woocommerce.android.widgets.CustomProgressDialog
Expand Down Expand Up @@ -391,6 +394,8 @@ class ProductDetailFragment :
viewModel.event.observe(viewLifecycleOwner) { event ->
when (event) {
is LaunchUrlInChromeTab -> ChromeCustomTabUtils.launchUrl(requireContext(), event.url)
is Event.LaunchUrlInAuthenticatedWebView -> findNavController(R.id.nav_host_fragment_main)
.navigateSafely(NavGraphMainDirections.actionGlobalAuthenticatedWebViewFragment(event.url))
is RefreshMenu -> toolbarHelper.setupToolbar()

is TrashProduct -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import com.woocommerce.android.tools.SelectedSite
import com.woocommerce.android.tools.SiteConnectionType
import com.woocommerce.android.ui.blaze.IsBlazeEnabled
import com.woocommerce.android.ui.blaze.IsProductCurrentlyPromoted
import com.woocommerce.android.ui.common.webview.CanAutoAuthenticateInWebView
import com.woocommerce.android.ui.customfields.CustomFieldsRepository
import com.woocommerce.android.ui.media.MediaFileUploadHandler
import com.woocommerce.android.ui.media.getMediaUploadErrorMessage
Expand All @@ -72,6 +73,7 @@ import com.woocommerce.android.ui.products.canDisplayMessage
import com.woocommerce.android.ui.products.categories.ProductCategoriesRepository
import com.woocommerce.android.ui.products.categories.ProductCategoryItemUiModel
import com.woocommerce.android.ui.products.details.ProductDetailBottomSheetBuilder.ProductDetailBottomSheetUiItem
import com.woocommerce.android.ui.products.details.ProductDetailViewModel.Companion.DEFAULT_ADD_NEW_PRODUCT_ID
import com.woocommerce.android.ui.products.list.ProductListRepository
import com.woocommerce.android.ui.products.models.ProductPropertyCard
import com.woocommerce.android.ui.products.models.SiteParameters
Expand All @@ -93,7 +95,6 @@ import com.woocommerce.android.util.IsWindowClassLargeThanCompact
import com.woocommerce.android.util.WooLog
import com.woocommerce.android.viewmodel.LiveDataDelegate
import com.woocommerce.android.viewmodel.MultiLiveEvent.Event
import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.LaunchUrlInChromeTab
import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.ShowDialog
import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.ShowSnackbar
import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.ShowUiStringSnackbar
Expand Down Expand Up @@ -162,7 +163,8 @@ class ProductDetailViewModel @Inject constructor(
private val isProductCurrentlyPromoted: IsProductCurrentlyPromoted,
private val isWindowClassLargeThanCompact: IsWindowClassLargeThanCompact,
private val determineProductPasswordApi: DetermineProductPasswordApi,
private val customFieldsRepository: CustomFieldsRepository
private val customFieldsRepository: CustomFieldsRepository,
private val canAutoAuthenticateInWebView: CanAutoAuthenticateInWebView
) : ScopedViewModel(savedState) {
companion object {
private const val KEY_PRODUCT_PARAMETERS = "key_product_parameters"
Expand Down Expand Up @@ -342,11 +344,16 @@ class ProductDetailViewModel @Inject constructor(
val showShareOptionAsActionWithText =
showShareOption && !showSaveOptionAsActionWithText && !showPublishOption

// Show "View Product" option only if the product is published or we can auto-authenticate the user
// in a WebView.
val showViewProductOption = !isProductUnderCreation &&
(isProductPublished || canAutoAuthenticateInWebView(productDraft.permalink))

MenuButtonsState(
saveOption = showSaveOptionAsActionWithText,
saveAsDraftOption = canBeSavedAsDraft,
publishOption = showPublishOption,
viewProductOption = isProductPublished && !isProductUnderCreation,
viewProductOption = showViewProductOption,
shareOption = showShareOption,
showShareOptionAsActionWithText = showShareOptionAsActionWithText,
trashOption = !isProductUnderCreation && navArgs.isTrashEnabled
Expand Down Expand Up @@ -549,9 +556,7 @@ class ProductDetailViewModel @Inject constructor(
}

fun onLearnMoreClicked() {
triggerEvent(
LaunchUrlInChromeTab(AppUrls.AUTOMATTIC_AI_GUIDELINES)
)
triggerEvent(Event.LaunchUrlInChromeTab(AppUrls.AUTOMATTIC_AI_GUIDELINES))
}

fun onBlazeClicked() {
Expand Down Expand Up @@ -1214,7 +1219,11 @@ class ProductDetailViewModel @Inject constructor(
fun onViewProductOnStoreLinkClicked() {
tracker.track(AnalyticsEvent.PRODUCT_DETAIL_VIEW_EXTERNAL_TAPPED)
viewState.productDraft?.permalink?.let { url ->
triggerEvent(LaunchUrlInChromeTab(url))
if (canAutoAuthenticateInWebView(url)) {
triggerEvent(Event.LaunchUrlInAuthenticatedWebView(url))
} else {
triggerEvent(Event.LaunchUrlInChromeTab(url))
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ open class MultiLiveEvent<T : Event> : MutableLiveData<T>() {

data class ExitWithResult<out T>(val data: T, val key: String? = null) : Event()

data class LaunchUrlInAuthenticatedWebView(val url: String) : Event()

data class LaunchUrlInChromeTab(val url: String) : Event()

data class ShowDialogFragment(
Expand Down
38 changes: 0 additions & 38 deletions WooCommerce/src/main/res/navigation/nav_graph_orders.xml
Original file line number Diff line number Diff line change
Expand Up @@ -486,44 +486,6 @@
android:label="EditShippingLabelPaymentFragment"
tools:layout="@layout/fragment_edit_shipping_label_payment">
</fragment>
<action
android:id="@+id/action_global_AuthenticatedWebViewFragment"
app:destination="@id/AuthenticatedWebViewFragment" />
Comment on lines -489 to -491
Copy link
Member Author

@hichamboushaba hichamboushaba Feb 7, 2025

Choose a reason for hiding this comment

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

Because the multi-pane setup, we previously duplicated the destination in this nested navgraph, this is because when the app is using two panes, the when calling findNavController from the details pane, it will return the nested NavController and not the main one, and this one won't know about destinations declared in the main nav graph.

With the new extension findNavController(@IdRes navHostId: Int), we don't need this duplication, and the logic is simpler. This also avoids the issues of this duplication: making the maintenance harder, because if we needed to change the list of arguments for the destination, then we need to do it for every nav graph that defines, or we might get crypted build failures or worse runtime crashes (check this PR for an example).

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

<fragment
android:id="@+id/AuthenticatedWebViewFragment"
android:name="com.woocommerce.android.ui.common.webview.AuthenticatedWebViewFragment"
android:label="AuthenticatedWebViewFragment"
tools:layout="@layout/fragment_wpcom_webview">
<argument
android:name="urlToLoad"
app:argType="string" />
<argument
android:name="title"
android:defaultValue="@null"
app:argType="string"
app:nullable="true" />
<argument
android:name="urlsToTriggerExit"
android:defaultValue="@null"
app:argType="string[]"
app:nullable="true" />
<argument
android:name="captureBackButton"
android:defaultValue="true"
app:argType="boolean" />
<argument
android:name="displayMode"
android:defaultValue="REGULAR"
app:argType="com.woocommerce.android.ui.common.webview.AuthenticatedWebViewViewModel$DisplayMode" />
<argument
android:name="urlComparisonMode"
android:defaultValue="PARTIAL"
app:argType="com.woocommerce.android.ui.common.webview.AuthenticatedWebViewViewModel$UrlComparisonMode" />
<argument
android:name="clearCache"
android:defaultValue="false"
app:argType="boolean" />
</fragment>
<fragment
android:id="@+id/shippingCarrierRatesFragment"
android:name="com.woocommerce.android.ui.orders.shippinglabels.creation.ShippingCarrierRatesFragment"
Expand Down
Loading
Loading