feat: App Level Webview/No Internet Error Handling #357

farhan-arshad-dev · 2024-07-05T07:19:10Z

Description

Not Internet Error Handling with reload CTA
Unknown error while loading the web page with reload CTA.

fixes: LEARNER-10046

Unknown error on web-page load	No Internet Connection (Reuse)

omerhabib26

Some minor code changes and improvements.

omerhabib26 · 2024-07-09T12:14:20Z

core/src/main/res/drawable/core_unkown.xml

Please update the file name
core_ic_unknown_error.xml

omerhabib26 · 2024-07-09T12:29:30Z

core/src/main/java/org/openedx/core/ui/ComposeCommon.kt

+) {
+    ErrorScreen(
+        modifier = modifier,
+        title = stringResource(id = R.string.core_no_internet_connection),


we can create an EnumClass for these types.

enum class ErrorType( val iconResId: Int = 0, val titleResId: Int = 0, val descriptionResId: Int = 0, val actionResId: Int = 0, ) { CONNECTION_ERROR( iconResId = R.drawable.core_no_internet_connection, titleResId = R.string.core_no_internet_connection, descriptionResId = R.string.core_no_internet_connection_description, actionResId = R.string.core_reload, ), UNKNOWN_ERROR( iconResId = R.drawable.core_unkown, titleResId = R.string.core_try_again, descriptionResId = R.string.core_something_went_wrong_description, actionResId = R.string.core_reload, ), }

omerhabib26 · 2024-07-09T13:19:44Z

course/src/main/java/org/openedx/course/presentation/unit/html/HtmlUnitFragment.kt

@@ -88,6 +90,10 @@ class HtmlUnitFragment : Fragment() {
                    mutableStateOf(true)
                }

+                var isError by remember {


we can move this in ViewModel

omerhabib26 · 2024-07-09T13:20:32Z

course/src/main/java/org/openedx/course/presentation/unit/html/HtmlUnitFragment.kt

                                hasInternetConnection = viewModel.isOnline
                            }
+                            if (!hasInternetConnection) {
+                                ConnectionErrorView(
+                                    modifier = Modifier


we can move the modifier inside CommonCompose.

omerhabib26 · 2024-07-09T13:40:35Z

discovery/src/main/java/org/openedx/discovery/presentation/program/ProgramFragment.kt

ErrorHandling not working on MyPrograms.

- Not Internet Error Handling with reload CTA - Unknown error while load webpage with reload CTA. fixes: LEARNER-10046

omerhabib26

Some minor code improvements.

omerhabib26 · 2024-07-12T16:08:31Z

core/src/main/java/org/openedx/core/ui/ComposeCommon.kt

+
+@Composable
+fun ErrorScreen(
+    title: String,


we can simply pass errorType here and set the properties accordingly.

fun ErrorScreen( errorType: ErrorType, onReloadClick: () -> Unit )

as we introduce this function as common compose, IMO it's better not to couple it with some constant.

omerhabib26 · 2024-07-12T17:03:08Z

core/src/main/java/org/openedx/core/presentation/global/webview/WebViewState.kt

IMO, WebViewUIState is more appropriate.

as it relates to whether the web view loaded the URL or not, IMO WebViewState is more appropriate.

omerhabib26 · 2024-07-12T17:10:29Z

course/src/main/java/org/openedx/course/presentation/unit/html/HtmlUnitUIState.kt

+
+import org.openedx.core.presentation.global.ErrorType
+
+sealed class HtmlUnitUIState {


Can we use WebViewUIState here as well ?

this is coupled with the UI screen, better to place separately.

omerhabib26 · 2024-07-15T10:31:30Z

course/src/main/java/org/openedx/course/presentation/unit/html/HtmlUnitViewModel.kt

@@ -20,6 +21,9 @@ class HtmlUnitViewModel(
    private val notifier: CourseNotifier
 ) : BaseViewModel() {

+    private val _uiState = MutableStateFlow<HtmlUnitUIState>(HtmlUnitUIState.Loading)


Can we use WebViewUIState here?

omerhabib26 · 2024-07-15T10:48:31Z

course/src/main/java/org/openedx/course/presentation/unit/html/HtmlUnitFragment.kt

+                        }
+                        if (uiState is HtmlUnitUIState.Error) {
+                            val errorType = (uiState as HtmlUnitUIState.Error).errorType
+                            ErrorScreen(


Please use ErrorType

omerhabib26 · 2024-07-15T11:38:28Z

discovery/src/main/java/org/openedx/discovery/presentation/DiscoveryUIState.kt

 import org.openedx.discovery.domain.model.Course

 sealed class DiscoveryUIState {
    data class Courses(val courses: List<Course>) : DiscoveryUIState()
    data object Loading : DiscoveryUIState()
+    data object Loaded : DiscoveryUIState()


instead of adding new objects in DiscoveryUIState, we can use WebviewUIState

omerhabib26 · 2024-07-15T11:40:25Z

discovery/src/main/java/org/openedx/discovery/presentation/info/CourseInfoUIState.kt

+    ) : CourseInfoUIState()
+}
+
+enum class CourseInfoUIAction {


We can define a generalised WebView State. here and other occurrences

enum class WebPageState{ WEB_PAGE_LOADED, WEB_PAGE_ERROR, RELOAD_WEB_PAGE }

omerhabib26 · 2024-07-15T11:41:29Z

discovery/src/main/java/org/openedx/discovery/presentation/WebViewDiscoveryFragment.kt

+                        onDiscoveryUIAction(DiscoveryUIAction.WEB_PAGE_ERROR)
+                    }
+                    if (uiState is DiscoveryUIState.Error) {
+                        ErrorScreen(


we can simply call ConnectionErrorView()

omerhabib26 · 2024-07-15T11:51:14Z

discovery/src/main/java/org/openedx/discovery/presentation/program/ProgramFragment.kt

+                    }
+
+                    if (uiState is ProgramUIState.Error) {
+                        ErrorScreen(


we can simply pass errorType.

omerhabib26 · 2024-07-15T11:51:48Z

discovery/src/main/java/org/openedx/discovery/presentation/info/CourseInfoFragment.kt

+                        onCourseInfoUIAction(CourseInfoUIAction.WEB_PAGE_ERROR)
+                    }
+                    if (webViewState is WebViewState.Error) {
+                        ErrorScreen(


omerhabib26

Connectivity error not handled on view all courses,
else only 2 minor nits,

omerhabib26 · 2024-07-16T19:52:14Z

discovery/src/main/java/org/openedx/discovery/presentation/DiscoveryUIState.kt

@@ -1,5 +1,6 @@
 package org.openedx.discovery.presentation

+import org.openedx.core.presentation.global.ErrorType


please remove unused import.

omerhabib26 · 2024-07-16T19:53:06Z

discovery/src/main/java/org/openedx/discovery/presentation/NativeDiscoveryFragment.kt

@@ -417,6 +417,8 @@ internal fun DiscoveryScreen(
                                }
                            }
                        }
+


unused code, can be removed

omerhabib26

LGTM

farhan-arshad-dev · 2024-07-17T14:00:34Z

We're closing this PR and moving these changes to our own fork, as they are critical for us. After our release, we hope to find a way to contribute to these changes back, thanks!

farhan-arshad-dev requested review from HamzaIsrar12, omerhabib26, volodymyr-chekyrta, k1rill and dixidroid and removed request for HamzaIsrar12 and omerhabib26 July 5, 2024 07:19

omerhabib26 requested changes Jul 9, 2024

View reviewed changes

feat: App Level Webview/No Internet Error Handling

9d597ef

- Not Internet Error Handling with reload CTA - Unknown error while load webpage with reload CTA. fixes: LEARNER-10046

farhan-arshad-dev force-pushed the farhan_ar/LEARNER-10046 branch from 59de37f to 5d31c60 Compare July 12, 2024 08:19

fix: address PR comments by Moin

ca66582

farhan-arshad-dev force-pushed the farhan_ar/LEARNER-10046 branch from 5d31c60 to ca66582 Compare July 12, 2024 08:21

omerhabib26 requested changes Jul 15, 2024

View reviewed changes

fix: address PR comments by Moin-2

1f106d5

omerhabib26 requested changes Jul 17, 2024

View reviewed changes

fix: address PR comments by Moin-3

e8a866e

omerhabib26 approved these changes Jul 17, 2024

View reviewed changes

farhan-arshad-dev closed this Jul 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: App Level Webview/No Internet Error Handling #357

feat: App Level Webview/No Internet Error Handling #357

farhan-arshad-dev commented Jul 5, 2024

omerhabib26 left a comment

omerhabib26 Jul 9, 2024

omerhabib26 Jul 9, 2024

omerhabib26 Jul 9, 2024

omerhabib26 Jul 9, 2024

omerhabib26 Jul 9, 2024

omerhabib26 left a comment

omerhabib26 Jul 12, 2024

farhan-arshad-dev Jul 15, 2024

omerhabib26 Jul 12, 2024

farhan-arshad-dev Jul 15, 2024

omerhabib26 Jul 12, 2024

farhan-arshad-dev Jul 15, 2024

omerhabib26 Jul 15, 2024

omerhabib26 Jul 15, 2024

farhan-arshad-dev Jul 15, 2024

omerhabib26 Jul 15, 2024

omerhabib26 Jul 15, 2024

omerhabib26 Jul 15, 2024

omerhabib26 Jul 15, 2024

omerhabib26 Jul 15, 2024

omerhabib26 left a comment

omerhabib26 Jul 16, 2024

omerhabib26 Jul 16, 2024

omerhabib26 left a comment

farhan-arshad-dev commented Jul 17, 2024


		import org.openedx.core.presentation.global.ErrorType

		sealed class HtmlUnitUIState {

		@@ -1,5 +1,6 @@
		package org.openedx.discovery.presentation

		import org.openedx.core.presentation.global.ErrorType

@@ @@ -417,6 +417,8 @@ internal fun DiscoveryScreen( @@
                                               }
                                           }
                                       }

feat: App Level Webview/No Internet Error Handling #357

feat: App Level Webview/No Internet Error Handling #357

Conversation

farhan-arshad-dev commented Jul 5, 2024

Description

omerhabib26 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

omerhabib26 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

omerhabib26 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

omerhabib26 left a comment

Choose a reason for hiding this comment

farhan-arshad-dev commented Jul 17, 2024