fix: Address PR comments

openedx · Feb 6, 2024 · cf395f6 · cf395f6
1 parent b1af700
commit cf395f6
Show file tree

Hide file tree

Showing 18 changed files with 211 additions and 193 deletions.
diff --git a/app/src/main/java/org/openedx/app/InDevelopmentFragment.kt b/app/src/main/java/org/openedx/app/InDevelopmentFragment.kt
@@ -53,4 +53,4 @@ class InDevelopmentFragment : Fragment() {
             }
         }
     }
-}
+}
diff --git a/auth/src/main/java/org/openedx/auth/presentation/ui/AuthUI.kt b/auth/src/main/java/org/openedx/auth/presentation/ui/AuthUI.kt
@@ -50,6 +50,7 @@ import org.openedx.auth.R
 import org.openedx.core.domain.model.RegistrationField
 import org.openedx.core.domain.model.RegistrationFieldType
 import org.openedx.core.extension.TextConverter
+import org.openedx.core.extension.withoutSpaces
 import org.openedx.core.ui.HyperlinkText
 import org.openedx.core.ui.SheetContent
 import org.openedx.core.ui.noRippleClickable
@@ -86,7 +87,7 @@ fun RequiredFields(
                 val linkedText =
                     TextConverter.htmlTextToLinkedText(field.label)
                 HyperlinkText(
-                    modifier = Modifier.testTag("txt_${field.name}"),
+                    modifier = Modifier.testTag("txt_${field.name.withoutSpaces()}"),
                     fullText = linkedText.text,
                     hyperLinks = linkedText.links,
                     linkTextColor = MaterialTheme.appColors.primary
@@ -308,7 +309,7 @@ fun InputRegistrationField(
     Column {
         Text(
             modifier = Modifier
-                .testTag("txt_${registrationField.name}_label")
+                .testTag("txt_${registrationField.name.withoutSpaces()}_label")
                 .fillMaxWidth(),
             text = registrationField.label,
             style = MaterialTheme.appTypography.labelLarge,
@@ -332,7 +333,7 @@ fun InputRegistrationField(
             shape = MaterialTheme.appShapes.textFieldShape,
             placeholder = {
                 Text(
-                    modifier = modifier.testTag("txt_${registrationField.name}_placeholder"),
+                    modifier = modifier.testTag("txt_${registrationField.name.withoutSpaces()}_placeholder"),
                     text = registrationField.label,
                     color = MaterialTheme.appColors.textFieldHint,
                     style = MaterialTheme.appTypography.bodyMedium
@@ -348,11 +349,11 @@ fun InputRegistrationField(
             },
             textStyle = MaterialTheme.appTypography.bodyMedium,
             singleLine = isSingleLine,
-            modifier = modifier.testTag("tf_${registrationField.name}")
+            modifier = modifier.testTag("tf_${registrationField.name.withoutSpaces()}")
         )
         Spacer(modifier = Modifier.height(6.dp))
         Text(
-            modifier = Modifier.testTag("txt_${registrationField.name}_description"),
+            modifier = Modifier.testTag("txt_${registrationField.name.withoutSpaces()}_description"),
             text = helperText,
             style = MaterialTheme.appTypography.bodySmall,
             color = helperTextColor
@@ -395,7 +396,7 @@ fun SelectableRegisterField(
     ) {
         Text(
             modifier = Modifier
-                .testTag("txt_${registrationField.name}_label")
+                .testTag("txt_${registrationField.name.withoutSpaces()}_label")
                 .fillMaxWidth(),
             text = registrationField.label,
             style = MaterialTheme.appTypography.labelLarge,
@@ -417,14 +418,14 @@ fun SelectableRegisterField(
             textStyle = MaterialTheme.appTypography.bodyMedium,
             onValueChange = { },
             modifier = Modifier
-                .testTag("tf_${registrationField.name}")
+                .testTag("tf_${registrationField.name.withoutSpaces()}")
                 .fillMaxWidth()
                 .noRippleClickable {
                     onClick(registrationField.name, registrationField.options)
                 },
             placeholder = {
                 Text(
-                    modifier = Modifier.testTag("txt_${registrationField.name}_placeholder"),
+                    modifier = Modifier.testTag("txt_${registrationField.name.withoutSpaces()}_placeholder"),
                     text = registrationField.label,
                     color = MaterialTheme.appColors.textFieldHint,
                     style = MaterialTheme.appTypography.bodyMedium
@@ -440,7 +441,7 @@ fun SelectableRegisterField(
         )
         Spacer(modifier = Modifier.height(6.dp))
         Text(
-            modifier = Modifier.testTag("txt_${registrationField.name}_description"),
+            modifier = Modifier.testTag("txt_${registrationField.name.withoutSpaces()}_description"),
             text = helperText,
             style = MaterialTheme.appTypography.bodySmall,
             color = helperTextColor

diff --git a/core/src/main/java/org/openedx/core/domain/model/VideoSettings.kt b/core/src/main/java/org/openedx/core/domain/model/VideoSettings.kt
@@ -17,10 +17,28 @@ enum class VideoQuality(
     val width: Int,
     val height: Int
 ) {
-    AUTO(R.string.video_quality_auto, R.string.video_quality_auto_description, 0, 0),
-    OPTION_360P(R.string.video_quality_p360, R.string.video_quality_p360_description, 640, 360),
-    OPTION_540P(R.string.video_quality_p540, 0, 960, 540),
-    OPTION_720P(R.string.video_quality_p720, R.string.video_quality_p720_description, 1280, 720);
-
-    val value: String = this.name.replace("OPTION_", "").lowercase()
+    AUTO(
+        titleResId = R.string.core_video_quality_auto,
+        desResId = R.string.core_video_quality_auto_description,
+        width = 0,
+        height = 0
+    ),
+    OPTION_360P(
+        titleResId = R.string.core_video_quality_p360,
+        desResId = R.string.core_video_quality_p360_description,
+        width = 640,
+        height = 360
+    ),
+    OPTION_540P(
+        titleResId = R.string.core_video_quality_p540,
+        desResId = 0,
+        width = 960,
+        height = 540
+    ),
+    OPTION_720P(
+        titleResId = R.string.core_video_quality_p720,
+        desResId = R.string.core_video_quality_p720_description,
+        width = 1280,
+        height = 720
+    );
 }
diff --git a/core/src/main/java/org/openedx/core/extension/StringExt.kt b/core/src/main/java/org/openedx/core/extension/StringExt.kt
@@ -28,3 +28,5 @@ fun String.replaceLinkTags(isDarkTheme: Boolean): String {
     }
     return text
 }
+
+fun String.withoutSpaces(): String = this.replace(" ", "")
diff --git a/core/src/main/java/org/openedx/core/presentation/global/app_upgrade/AppUpdateUI.kt b/core/src/main/java/org/openedx/core/presentation/global/app_upgrade/AppUpdateUI.kt
@@ -77,9 +77,9 @@ fun AppUpgradeRequiredScreen(
     ) {
         Text(
             modifier = Modifier
+                .testTag("txt_app_upgrade_deprecated")
                 .fillMaxWidth()
-                .padding(top = 10.dp, bottom = 12.dp)
-                .testTag("txt_app_upgrade_deprecated"),
+                .padding(top = 10.dp, bottom = 12.dp),
             text = stringResource(id = R.string.core_deprecated_app_version),
             color = MaterialTheme.appColors.textPrimary,
             style = MaterialTheme.appTypography.titleMedium,
@@ -119,12 +119,12 @@ fun AppUpgradeRecommendDialog(
     ) {
         Box(
             modifier = modifier
+                .testTag("btn_upgrade_dialog_not_now")
                 .fillMaxSize()
                 .padding(horizontal = 4.dp)
                 .noRippleClickable {
                     onNotNowClick()
-                }
-                .testTag("btn_upgrade_dialog_not_now"),
+                },
             contentAlignment = Alignment.Center
         ) {
             Box(
@@ -264,8 +264,8 @@ fun TransparentTextButton(
 ) {
     Button(
         modifier = Modifier
-            .height(42.dp)
-            .testTag("btn_upgrade_dialog_$text"),
+            .testTag("btn_upgrade_dialog_$text")
+            .height(42.dp),
         colors = ButtonDefaults.buttonColors(
             backgroundColor = Color.Transparent
         ),
@@ -289,8 +289,8 @@ fun DefaultTextButton(
 ) {
     Button(
         modifier = Modifier
-            .height(42.dp)
-            .testTag("btn_upgrade_dialog_$text"),
+            .testTag("btn_upgrade_dialog_$text")
+            .height(42.dp),
         colors = ButtonDefaults.buttonColors(
             backgroundColor = MaterialTheme.appColors.buttonBackground
         ),
@@ -319,12 +319,12 @@ fun AppUpgradeRecommendedBox(
 ) {
     Card(
         modifier = modifier
+            .testTag("btn_upgrade_box")
             .fillMaxWidth()
             .padding(20.dp)
             .clickable {
                 onClick()
-            }
-            .testTag("btn_upgrade_box"),
+            },
         shape = MaterialTheme.appShapes.cardShape,
         backgroundColor = MaterialTheme.appColors.primary
     ) {
-Original file line number
+Diff line change
@@ Expand Up / @@ -53,4 +53,4 @@ class InDevelopmentFragment : Fragment() { @@
                 }
             }
         }
-    }
+    }