diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index c16be9d572c..52bbdb01e52 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -1,4 +1,8 @@ *** PLEASE FOLLOW THIS FORMAT: [] [] + +17.3 +----- +- [*] [Internal] Enhanced user experience in shipping label creation with automatic scrolling to the first invalid field upon form submission failure [https://github.com/woocommerce/woocommerce-android/pull/10657] 17.2 ----- - [**] [Available for users with WooCommerce version of 8.7+, which is not released yet] Every order have a receipt now. The receipts can be shared via many apps installed on the phone [https://github.com/woocommerce/woocommerce-android/pull/10650] diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/shippinglabels/creation/CreateShippingLabelEvent.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/shippinglabels/creation/CreateShippingLabelEvent.kt index d04195e2f53..95548aed33e 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/shippinglabels/creation/CreateShippingLabelEvent.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/shippinglabels/creation/CreateShippingLabelEvent.kt @@ -68,4 +68,9 @@ sealed class CreateShippingLabelEvent : MultiLiveEvent.Event() { data class ShowPrintShippingLabels(val orderId: Long, val labels: List) : CreateShippingLabelEvent() object ShowWooDiscountBottomSheet : CreateShippingLabelEvent() + object CloseKeyboard : CreateShippingLabelEvent() + data class ScrollToFirstErrorField( + val field: EditShippingLabelAddressViewModel.Field, + val isStateFieldSpinner: Boolean? + ) : CreateShippingLabelEvent() } diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/shippinglabels/creation/EditShippingLabelAddressFragment.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/shippinglabels/creation/EditShippingLabelAddressFragment.kt index 0e22edcb5f8..730c6d819c0 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/shippinglabels/creation/EditShippingLabelAddressFragment.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/shippinglabels/creation/EditShippingLabelAddressFragment.kt @@ -29,8 +29,10 @@ import com.woocommerce.android.ui.base.BaseFragment import com.woocommerce.android.ui.base.UIMessageResolver import com.woocommerce.android.ui.common.InputField import com.woocommerce.android.ui.main.MainActivity.Companion.BackPressListener +import com.woocommerce.android.ui.orders.shippinglabels.creation.CreateShippingLabelEvent.CloseKeyboard import com.woocommerce.android.ui.orders.shippinglabels.creation.CreateShippingLabelEvent.DialPhoneNumber import com.woocommerce.android.ui.orders.shippinglabels.creation.CreateShippingLabelEvent.OpenMapWithAddress +import com.woocommerce.android.ui.orders.shippinglabels.creation.CreateShippingLabelEvent.ScrollToFirstErrorField import com.woocommerce.android.ui.orders.shippinglabels.creation.CreateShippingLabelEvent.ShowCountrySelector import com.woocommerce.android.ui.orders.shippinglabels.creation.CreateShippingLabelEvent.ShowStateSelector import com.woocommerce.android.ui.orders.shippinglabels.creation.CreateShippingLabelEvent.ShowSuggestedAddress @@ -40,6 +42,7 @@ import com.woocommerce.android.ui.orders.shippinglabels.creation.ShippingLabelAd import com.woocommerce.android.ui.searchfilter.SearchFilterItem import com.woocommerce.android.util.ActivityUtils.dialPhoneNumber import com.woocommerce.android.util.UiHelpers +import com.woocommerce.android.util.isViewVisibleInScrollView import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.Exit import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.ExitWithResult import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.ShowSnackbar @@ -61,14 +64,19 @@ class EditShippingLabelAddressFragment : const val SELECT_STATE_REQUEST = "select_state_request" const val EDIT_ADDRESS_RESULT = "key_edit_address_dialog_result" const val EDIT_ADDRESS_CLOSED = "key_edit_address_dialog_closed" + const val ERROR_SCROLL_DELAY = 300L } - @Inject lateinit var uiMessageResolver: UIMessageResolver + @Inject + lateinit var uiMessageResolver: UIMessageResolver private var progressDialog: CustomProgressDialog? = null val viewModel: EditShippingLabelAddressViewModel by viewModels() + private var _binding: FragmentEditShippingLabelAddressBinding? = null + private val binding get() = _binding!! + private var screenTitle = "" set(value) { field = value @@ -92,12 +100,17 @@ class EditShippingLabelAddressFragment : } } + override fun onDestroyView() { + super.onDestroyView() + _binding = null + } + override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) requireActivity().addMenuProvider(this, viewLifecycleOwner) - val binding = FragmentEditShippingLabelAddressBinding.bind(view) + _binding = FragmentEditShippingLabelAddressBinding.bind(view) initializeViewModel(binding) initializeViews(binding) @@ -133,10 +146,10 @@ class EditShippingLabelAddressFragment : override fun onMenuItemSelected(item: MenuItem): Boolean { return when (item.itemId) { R.id.menu_done -> { - ActivityUtils.hideKeyboard(activity) viewModel.onDoneButtonClicked() true } + else -> false } } @@ -219,6 +232,27 @@ class EditShippingLabelAddressFragment : } } + private fun scrollToFirstErrorField(field: Field, isStateFieldSpinner: Boolean?) { + val errorView = when (field) { + Field.Name -> binding.name + Field.Company -> binding.company + Field.Phone -> binding.phone + Field.Address1 -> binding.address1 + Field.Address2 -> binding.address2 + Field.City -> binding.city + Field.Zip -> binding.zip + Field.State -> if (isStateFieldSpinner == true) binding.stateSpinner else binding.state + Field.Country -> binding.countrySpinner + } + + if (!isViewVisibleInScrollView(binding.scrollView, errorView)) { + binding.scrollView.postDelayed({ + binding.scrollView.smoothScrollTo(0, errorView.top) + errorView.editText?.requestFocus() + }, ERROR_SCROLL_DELAY) + } + } + @SuppressLint("SetTextI18n") private fun observeEvents() { viewModel.event.observe(viewLifecycleOwner) { event -> @@ -235,6 +269,7 @@ class EditShippingLabelAddressFragment : ) findNavController().navigateSafely(action) } + is ShowCountrySelector -> { val action = EditShippingLabelAddressFragmentDirections.actionSearchFilterFragment( items = event.locations.map { @@ -249,6 +284,7 @@ class EditShippingLabelAddressFragment : ) findNavController().navigateSafely(action) } + is ShowStateSelector -> { val action = EditShippingLabelAddressFragmentDirections.actionSearchFilterFragment( items = event.locations.map { @@ -263,8 +299,17 @@ class EditShippingLabelAddressFragment : ) findNavController().navigateSafely(action) } + is OpenMapWithAddress -> launchMapsWithAddress(event.address) is DialPhoneNumber -> dialPhoneNumber(requireContext(), event.phoneNumber) + is ScrollToFirstErrorField -> scrollToFirstErrorField( + event.field, + event.isStateFieldSpinner + ) + CloseKeyboard -> { + activity?.let { ActivityUtils.hideKeyboard(it) } + } + else -> event.isHandled = false } } diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/shippinglabels/creation/EditShippingLabelAddressViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/shippinglabels/creation/EditShippingLabelAddressViewModel.kt index d4099fa89f7..fbc37ebc822 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/shippinglabels/creation/EditShippingLabelAddressViewModel.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/shippinglabels/creation/EditShippingLabelAddressViewModel.kt @@ -17,6 +17,7 @@ import com.woocommerce.android.tools.SelectedSite import com.woocommerce.android.ui.common.InputField import com.woocommerce.android.ui.common.OptionalField import com.woocommerce.android.ui.common.RequiredField +import com.woocommerce.android.ui.orders.shippinglabels.creation.CreateShippingLabelEvent.CloseKeyboard import com.woocommerce.android.ui.orders.shippinglabels.creation.CreateShippingLabelEvent.DialPhoneNumber import com.woocommerce.android.ui.orders.shippinglabels.creation.CreateShippingLabelEvent.OpenMapWithAddress import com.woocommerce.android.ui.orders.shippinglabels.creation.CreateShippingLabelEvent.ShowCountrySelector @@ -75,7 +76,9 @@ class EditShippingLabelAddressViewModel @Inject constructor( val fullCountriesList = dataStore.getCountries() val supportedCountries = if (arguments.addressType == ORIGIN) { fullCountriesList.filter { ACCEPTED_USPS_ORIGIN_COUNTRIES.contains(it.code) } - } else fullCountriesList + } else { + fullCountriesList + } return supportedCountries.map { it.toAppModel() } } @@ -102,6 +105,7 @@ class EditShippingLabelAddressViewModel @Inject constructor( viewState = viewState.validateAllFields() val address = viewState.getAddress() if (viewState.areAllRequiredFieldsValid) { + triggerEvent(CloseKeyboard) launch { viewState = viewState.copy( isValidationProgressDialogVisible = true, @@ -115,9 +119,23 @@ class EditShippingLabelAddressViewModel @Inject constructor( handleValidationResult(address, result) viewState = viewState.copy(isValidationProgressDialogVisible = false) } + } else { + triggerEvent(ShowSnackbar(R.string.shipping_label_address_data_invalid_snackbar_message)) + triggerScrollToFirstErrorFieldEvent() } } + private fun triggerScrollToFirstErrorFieldEvent() { + val firstErrorField = viewState.findFirstErrorField() + firstErrorField?.let { + triggerEvent(CreateShippingLabelEvent.ScrollToFirstErrorField(it, viewState.isStateFieldSpinner)) + } + } + + private fun ViewState.findFirstErrorField(): Field? { + return Field.values().firstOrNull { this[it].error != null } + } + private fun loadCountriesAndStates() { launch { if (countries.isEmpty()) { @@ -147,13 +165,17 @@ class EditShippingLabelAddressViewModel @Inject constructor( ValidationResult.Valid -> { exitWithAddress(address) } + is ValidationResult.Invalid -> { val validationErrorMessage = getAddressErrorStringRes(result.message) viewState = viewState.copy( address1Field = viewState.address1Field.copy(validationError = validationErrorMessage).validate(), bannerMessage = resourceProvider.getString( - if (arguments.addressType == ORIGIN) R.string.shipping_label_edit_origin_address_error_warning - else R.string.shipping_label_edit_address_error_warning + if (arguments.addressType == ORIGIN) { + R.string.shipping_label_edit_origin_address_error_warning + } else { + R.string.shipping_label_edit_address_error_warning + } ) ) } @@ -209,9 +231,11 @@ class EditShippingLabelAddressViewModel @Inject constructor( // Validate fields locally viewState = viewState.validateAllFields() if (viewState.areAllRequiredFieldsValid) { + triggerEvent(CloseKeyboard) exitWithAddress(viewState.getAddress()) } else { triggerEvent(ShowSnackbar(R.string.shipping_label_address_data_invalid_snackbar_message)) + triggerScrollToFirstErrorFieldEvent() } } @@ -437,8 +461,11 @@ class EditShippingLabelAddressViewModel @Inject constructor( val companyContent: String ) : InputField(content) { override fun validateInternal(): UiString? { - return if (content.isNotBlank() || companyContent.isNotBlank()) null - else UiStringRes(R.string.error_required_field) + return if (content.isNotBlank() || companyContent.isNotBlank()) { + null + } else { + UiStringRes(R.string.error_required_field) + } } } @@ -463,8 +490,9 @@ class EditShippingLabelAddressViewModel @Inject constructor( val addressType: AddressType ) : InputField(content) { override fun validateInternal(): UiString? { - return if (content.isValidPhoneNumber(addressType, isCustomsFormRequired)) null - else { + return if (content.isValidPhoneNumber(addressType, isCustomsFormRequired)) { + null + } else { when { content.isBlank() -> UiStringRes(R.string.shipping_label_address_phone_required) addressType == ORIGIN -> @@ -485,8 +513,11 @@ class EditShippingLabelAddressViewModel @Inject constructor( val isRequired: Boolean = false ) : InputField(location.name) { override fun validateInternal(): UiString? { - return if (isRequired && content.isBlank()) UiStringRes(R.string.error_required_field) - else null + return if (isRequired && content.isBlank()) { + UiStringRes(R.string.error_required_field) + } else { + null + } } } diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/util/ViewUtils.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/util/ViewUtils.kt index bda46067d06..2e2f2908b5f 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/util/ViewUtils.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/util/ViewUtils.kt @@ -1,6 +1,9 @@ package com.woocommerce.android.util import android.content.Context +import android.graphics.Rect +import android.view.View +import android.widget.ScrollView /** * Converts a pixel to a density pixel to match the density of @@ -10,3 +13,11 @@ fun getDensityPixel(context: Context, dps: Int): Int { val scale = context.resources.displayMetrics.density return (dps * scale + 0.5f).toInt() } + +fun isViewVisibleInScrollView(scrollView: ScrollView, targetView: View): Boolean { + val scrollBounds = Rect() + scrollView.getDrawingRect(scrollBounds) + val top = targetView.y + scrollView.y + val bottom = top + targetView.height + return scrollBounds.top < top && scrollBounds.bottom > bottom +} diff --git a/WooCommerce/src/main/res/layout/fragment_edit_shipping_label_address.xml b/WooCommerce/src/main/res/layout/fragment_edit_shipping_label_address.xml index f0fd9547657..d77d29d27cc 100644 --- a/WooCommerce/src/main/res/layout/fragment_edit_shipping_label_address.xml +++ b/WooCommerce/src/main/res/layout/fragment_edit_shipping_label_address.xml @@ -2,6 +2,7 @@ diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/shippinglabels/creation/EditShippingLabelAddressViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/shippinglabels/creation/EditShippingLabelAddressViewModelTest.kt index 30db74a54f3..e73894bfac0 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/shippinglabels/creation/EditShippingLabelAddressViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/shippinglabels/creation/EditShippingLabelAddressViewModelTest.kt @@ -5,8 +5,10 @@ import com.woocommerce.android.R.string import com.woocommerce.android.model.UiString.UiStringRes import com.woocommerce.android.model.toAppModel import com.woocommerce.android.tools.SelectedSite +import com.woocommerce.android.ui.orders.shippinglabels.creation.CreateShippingLabelEvent.CloseKeyboard import com.woocommerce.android.ui.orders.shippinglabels.creation.CreateShippingLabelEvent.DialPhoneNumber import com.woocommerce.android.ui.orders.shippinglabels.creation.CreateShippingLabelEvent.OpenMapWithAddress +import com.woocommerce.android.ui.orders.shippinglabels.creation.CreateShippingLabelEvent.ScrollToFirstErrorField import com.woocommerce.android.ui.orders.shippinglabels.creation.CreateShippingLabelEvent.ShowCountrySelector import com.woocommerce.android.ui.orders.shippinglabels.creation.CreateShippingLabelEvent.ShowStateSelector import com.woocommerce.android.ui.orders.shippinglabels.creation.CreateShippingLabelEvent.ShowSuggestedAddress @@ -380,4 +382,85 @@ class EditShippingLabelAddressViewModelTest : BaseUnitTest() { val viewState = viewModel.viewStateData.liveData.value!! assertThat(viewState.areAllRequiredFieldsValid).isTrue() } + + @Test + fun `given validation fails for a set of fields, when on done clicked, then ScrollToFirstErrorField event is triggered with correct field`() = + testBlocking { + viewModel.onFieldEdited(Field.Name, "") + viewModel.onFieldEdited(Field.Company, "") + + var event: Event? = null + viewModel.event.observeForever { event = it } + + viewModel.onDoneButtonClicked() + + verify(addressValidator, never()).validateAddress(any(), any(), any()) + + assertThat(event).isInstanceOf(ScrollToFirstErrorField::class.java) + if (event is ScrollToFirstErrorField) { + assertThat((event as ScrollToFirstErrorField).field).isEqualTo(Field.Name) + } + } + + @Test + fun `given all fields are valid, when on done clicked, then ScrollToFirstErrorField event is not triggered`() = + testBlocking { + var event: Event? = null + viewModel.event.observeForever { event = it } + + viewModel.onDoneButtonClicked() + + verify(addressValidator, atLeastOnce()).validateAddress(any(), any(), any()) + + assertThat(event).isNotInstanceOf(ScrollToFirstErrorField::class.java) + } + + @Test + fun `given all fields are valid, when onDoneButtonClicked, then CloseKeyboard event is triggered`() = testBlocking { + var event: Event? = null + viewModel.event.observeForever { event = it } + + viewModel.onDoneButtonClicked() + + assertThat(event).isEqualTo(CloseKeyboard) + } + + @Test + fun `given all fields are valid, when onUseAddressAsIsButtonClicked, then CloseKeyboard event is triggered`() = + testBlocking { + val events = mutableListOf() + viewModel.event.observeForever { events.add(it) } + + viewModel.onUseAddressAsIsButtonClicked() + + assertThat(events).contains(CloseKeyboard) + } + + @Test + fun `given fields are invalid, when onDoneButtonClicked, then CloseKeyboard event is not triggered`() = + testBlocking { + viewModel.onFieldEdited(Field.Name, "") + viewModel.onFieldEdited(Field.Company, "") + + var event: Event? = null + viewModel.event.observeForever { event = it } + + viewModel.onDoneButtonClicked() + + assertThat(event).isNotEqualTo(CloseKeyboard) + } + + @Test + fun `given fields are invalid, when onUseAddressAsIsButtonClicked, then CloseKeyboard event is not triggered`() = + testBlocking { + viewModel.onFieldEdited(Field.Name, "") + viewModel.onFieldEdited(Field.Company, "") + + var event: Event? = null + viewModel.event.observeForever { event = it } + + viewModel.onUseAddressAsIsButtonClicked() + + assertThat(event).isNotEqualTo(CloseKeyboard) + } }