From 9a89bedc43b405793365e326f5ab86b803f39452 Mon Sep 17 00:00:00 2001 From: TanishMoral11 Date: Sat, 2 Nov 2024 22:01:27 +0530 Subject: [PATCH 1/5] Fix: Correct home screen topic grid misalignment after returning from lesson (#5150) --- .../databinding/MarginBindingAdapters.java | 21 ++----------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/databinding/MarginBindingAdapters.java b/app/src/main/java/org/oppia/android/app/databinding/MarginBindingAdapters.java index 2c7cd8be971..a94e1deb30e 100644 --- a/app/src/main/java/org/oppia/android/app/databinding/MarginBindingAdapters.java +++ b/app/src/main/java/org/oppia/android/app/databinding/MarginBindingAdapters.java @@ -15,7 +15,7 @@ public static void setLayoutMarginStart(@NonNull View view, float marginStart) { if (view.getLayoutParams() instanceof MarginLayoutParams) { MarginLayoutParams params = (MarginLayoutParams) view.getLayoutParams(); MarginLayoutParamsCompat.setMarginStart(params, (int) marginStart); - view.requestLayout(); + view.setLayoutParams(params); } } @@ -25,7 +25,7 @@ public static void setLayoutMarginEnd(@NonNull View view, float marginEnd) { if (view.getLayoutParams() instanceof MarginLayoutParams) { MarginLayoutParams params = (MarginLayoutParams) view.getLayoutParams(); MarginLayoutParamsCompat.setMarginEnd(params, (int) marginEnd); - view.requestLayout(); + view.setLayoutParams(params); } } @@ -36,7 +36,6 @@ public static void setLayoutMarginTop(@NonNull View view, float marginTop) { MarginLayoutParams params = (MarginLayoutParams) view.getLayoutParams(); params.topMargin = (int) marginTop; view.setLayoutParams(params); - view.requestLayout(); } } @@ -47,22 +46,6 @@ public static void setLayoutMarginBottom(@NonNull View view, float marginBottom) MarginLayoutParams params = (MarginLayoutParams) view.getLayoutParams(); params.bottomMargin = (int) marginBottom; view.setLayoutParams(params); - view.requestLayout(); - } - } - - /** Used to set a margin for views. */ - @BindingAdapter("layoutMargin") - public static void setLayoutMargin(@NonNull View view, float margin) { - if (view.getLayoutParams() instanceof MarginLayoutParams) { - MarginLayoutParams params = (MarginLayoutParams) view.getLayoutParams(); - params.setMargins( - (int) margin, - (int) margin, - (int) margin, - (int) margin - ); - view.requestLayout(); } } } From 2dddb9928d7a0de6e725a2f9b9fd2ba07bc5c12e Mon Sep 17 00:00:00 2001 From: TanishMoral11 Date: Tue, 17 Dec 2024 04:22:15 +0530 Subject: [PATCH 2/5] Added Test --- .../databinding/MarginBindingAdapters.java | 21 +++-- .../databinding/MarginBindingAdaptersTest.kt | 80 ++++++++++++++++++- 2 files changed, 94 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/databinding/MarginBindingAdapters.java b/app/src/main/java/org/oppia/android/app/databinding/MarginBindingAdapters.java index a94e1deb30e..49c1a87ecf3 100644 --- a/app/src/main/java/org/oppia/android/app/databinding/MarginBindingAdapters.java +++ b/app/src/main/java/org/oppia/android/app/databinding/MarginBindingAdapters.java @@ -2,14 +2,19 @@ import android.view.View; import android.view.ViewGroup.MarginLayoutParams; + import androidx.annotation.NonNull; import androidx.core.view.MarginLayoutParamsCompat; import androidx.databinding.BindingAdapter; -/** Holds all custom binding adapters that set margin values. */ +/** + * Holds all custom binding adapters that set margin values. + */ public final class MarginBindingAdapters { - /** Sets the start margin for a view, accounting for RTL scenarios. */ + /** + * Sets the start margin for a view, accounting for RTL scenarios. + */ @BindingAdapter("layoutMarginStart") public static void setLayoutMarginStart(@NonNull View view, float marginStart) { if (view.getLayoutParams() instanceof MarginLayoutParams) { @@ -19,7 +24,9 @@ public static void setLayoutMarginStart(@NonNull View view, float marginStart) { } } - /** Sets the end margin for a view, accounting for RTL scenarios. */ + /** + * Sets the end margin for a view, accounting for RTL scenarios. + */ @BindingAdapter("layoutMarginEnd") public static void setLayoutMarginEnd(@NonNull View view, float marginEnd) { if (view.getLayoutParams() instanceof MarginLayoutParams) { @@ -29,7 +36,9 @@ public static void setLayoutMarginEnd(@NonNull View view, float marginEnd) { } } - /** Used to set a margin-top for views. */ + /** + * Used to set a margin-top for views. + */ @BindingAdapter("layoutMarginTop") public static void setLayoutMarginTop(@NonNull View view, float marginTop) { if (view.getLayoutParams() instanceof MarginLayoutParams) { @@ -39,7 +48,9 @@ public static void setLayoutMarginTop(@NonNull View view, float marginTop) { } } - /** Used to set a margin-bottom for views. */ + /** + * Used to set a margin-bottom for views. + */ @BindingAdapter("layoutMarginBottom") public static void setLayoutMarginBottom(@NonNull View view, float marginBottom) { if (view.getLayoutParams() instanceof MarginLayoutParams) { diff --git a/app/src/sharedTest/java/org/oppia/android/app/databinding/MarginBindingAdaptersTest.kt b/app/src/sharedTest/java/org/oppia/android/app/databinding/MarginBindingAdaptersTest.kt index 826d629d355..1629fc8c3c6 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/databinding/MarginBindingAdaptersTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/databinding/MarginBindingAdaptersTest.kt @@ -5,6 +5,7 @@ import android.app.Application import android.content.Context import android.content.Intent import android.view.View +import android.view.ViewGroup import android.widget.TextView import androidx.appcompat.app.AppCompatActivity import androidx.core.view.ViewCompat @@ -113,8 +114,11 @@ class MarginBindingAdaptersTest { @get:Rule val initializeDefaultLocaleRule = InitializeDefaultLocaleRule() - @Inject lateinit var context: Context - @Inject lateinit var testCoroutineDispatchers: TestCoroutineDispatchers + @Inject + lateinit var context: Context + + @Inject + lateinit var testCoroutineDispatchers: TestCoroutineDispatchers @get:Rule val oppiaTestRule = OppiaTestRule() @@ -293,6 +297,78 @@ class MarginBindingAdaptersTest { assertThat(textView.marginEnd.toFloat()).isWithin(TOLERANCE).of(40f) } + @Config(qualifiers = "port") + @Test + fun testMarginBindableAdapters_layoutParamsSettingPreservesMargins() { + val textView = activityRule.scenario.runWithActivity { + val textView: TextView = it.findViewById(R.id.test_margin_text_view) + + // Set initial margins + setLayoutMarginStart(textView, /* marginStart= */ 24f) + setLayoutMarginEnd(textView, /* marginEnd= */ 40f) + setLayoutMarginTop(textView, /* marginTop= */ 16f) + setLayoutMarginBottom(textView, /* marginBottom= */ 32f) + + return@runWithActivity textView + } + + // Verify that the margins are correctly set after method calls + assertThat(textView.marginStart.toFloat()).isWithin(TOLERANCE).of(24f) + assertThat(textView.marginEnd.toFloat()).isWithin(TOLERANCE).of(40f) + + val layoutParams = textView.layoutParams as ViewGroup.MarginLayoutParams + assertThat(layoutParams.topMargin.toFloat()).isWithin(TOLERANCE).of(16f) + assertThat(layoutParams.bottomMargin.toFloat()).isWithin(TOLERANCE).of(32f) + } + + @Config(qualifiers = "land") + @Test + fun testMarginBindableAdapters_layoutParamsSettingPreservesMargins_landscape() { + val textView = activityRule.scenario.runWithActivity { + val textView: TextView = it.findViewById(R.id.test_margin_text_view) + + // Set initial margins + setLayoutMarginStart(textView, /* marginStart= */ 24f) + setLayoutMarginEnd(textView, /* marginEnd= */ 40f) + setLayoutMarginTop(textView, /* marginTop= */ 16f) + setLayoutMarginBottom(textView, /* marginBottom= */ 32f) + + return@runWithActivity textView + } + + // Verify that the margins are correctly set after method calls + assertThat(textView.marginStart.toFloat()).isWithin(TOLERANCE).of(24f) + assertThat(textView.marginEnd.toFloat()).isWithin(TOLERANCE).of(40f) + + val layoutParams = textView.layoutParams as ViewGroup.MarginLayoutParams + assertThat(layoutParams.topMargin.toFloat()).isWithin(TOLERANCE).of(16f) + assertThat(layoutParams.bottomMargin.toFloat()).isWithin(TOLERANCE).of(32f) + } + + @Config(qualifiers = "sw600dp-port") + @Test + fun testMarginBindableAdapters_layoutParamsSettingPreservesMargins_tablet() { + val textView = activityRule.scenario.runWithActivity { + val textView: TextView = it.findViewById(R.id.test_margin_text_view) + + // Set initial margins + setLayoutMarginStart(textView, /* marginStart= */ 24f) + setLayoutMarginEnd(textView, /* marginEnd= */ 40f) + setLayoutMarginTop(textView, /* marginTop= */ 16f) + setLayoutMarginBottom(textView, /* marginBottom= */ 32f) + + return@runWithActivity textView + } + + // Verify that the margins are correctly set after method calls + assertThat(textView.marginStart.toFloat()).isWithin(TOLERANCE).of(24f) + assertThat(textView.marginEnd.toFloat()).isWithin(TOLERANCE).of(40f) + + val layoutParams = textView.layoutParams as ViewGroup.MarginLayoutParams + assertThat(layoutParams.topMargin.toFloat()).isWithin(TOLERANCE).of(16f) + assertThat(layoutParams.bottomMargin.toFloat()).isWithin(TOLERANCE).of(32f) + } + private fun testMarginBindableAdapters_topAndBottomIsCorrect() { activityRule.scenario.runWithActivity { val textView: TextView = it.findViewById(R.id.test_margin_text_view) From 498b7d190d43609d098a91e803f34b102ed8ee97 Mon Sep 17 00:00:00 2001 From: TanishMoral11 Date: Tue, 17 Dec 2024 04:23:35 +0530 Subject: [PATCH 3/5] Improve Formatting --- .../org/oppia/android/app/databinding/MarginBindingAdapters.java | 1 - 1 file changed, 1 deletion(-) diff --git a/app/src/main/java/org/oppia/android/app/databinding/MarginBindingAdapters.java b/app/src/main/java/org/oppia/android/app/databinding/MarginBindingAdapters.java index 49c1a87ecf3..86e9006bf21 100644 --- a/app/src/main/java/org/oppia/android/app/databinding/MarginBindingAdapters.java +++ b/app/src/main/java/org/oppia/android/app/databinding/MarginBindingAdapters.java @@ -2,7 +2,6 @@ import android.view.View; import android.view.ViewGroup.MarginLayoutParams; - import androidx.annotation.NonNull; import androidx.core.view.MarginLayoutParamsCompat; import androidx.databinding.BindingAdapter; From daac2b31a5e27828d029c19f231480179612dc54 Mon Sep 17 00:00:00 2001 From: TanishMoral11 Date: Wed, 18 Dec 2024 14:14:00 +0530 Subject: [PATCH 4/5] Resolved Issues --- .../databinding/MarginBindingAdapters.java | 20 +++++-------------- .../databinding/MarginBindingAdaptersTest.kt | 6 +++--- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/databinding/MarginBindingAdapters.java b/app/src/main/java/org/oppia/android/app/databinding/MarginBindingAdapters.java index 86e9006bf21..a94e1deb30e 100644 --- a/app/src/main/java/org/oppia/android/app/databinding/MarginBindingAdapters.java +++ b/app/src/main/java/org/oppia/android/app/databinding/MarginBindingAdapters.java @@ -6,14 +6,10 @@ import androidx.core.view.MarginLayoutParamsCompat; import androidx.databinding.BindingAdapter; -/** - * Holds all custom binding adapters that set margin values. - */ +/** Holds all custom binding adapters that set margin values. */ public final class MarginBindingAdapters { - /** - * Sets the start margin for a view, accounting for RTL scenarios. - */ + /** Sets the start margin for a view, accounting for RTL scenarios. */ @BindingAdapter("layoutMarginStart") public static void setLayoutMarginStart(@NonNull View view, float marginStart) { if (view.getLayoutParams() instanceof MarginLayoutParams) { @@ -23,9 +19,7 @@ public static void setLayoutMarginStart(@NonNull View view, float marginStart) { } } - /** - * Sets the end margin for a view, accounting for RTL scenarios. - */ + /** Sets the end margin for a view, accounting for RTL scenarios. */ @BindingAdapter("layoutMarginEnd") public static void setLayoutMarginEnd(@NonNull View view, float marginEnd) { if (view.getLayoutParams() instanceof MarginLayoutParams) { @@ -35,9 +29,7 @@ public static void setLayoutMarginEnd(@NonNull View view, float marginEnd) { } } - /** - * Used to set a margin-top for views. - */ + /** Used to set a margin-top for views. */ @BindingAdapter("layoutMarginTop") public static void setLayoutMarginTop(@NonNull View view, float marginTop) { if (view.getLayoutParams() instanceof MarginLayoutParams) { @@ -47,9 +39,7 @@ public static void setLayoutMarginTop(@NonNull View view, float marginTop) { } } - /** - * Used to set a margin-bottom for views. - */ + /** Used to set a margin-bottom for views. */ @BindingAdapter("layoutMarginBottom") public static void setLayoutMarginBottom(@NonNull View view, float marginBottom) { if (view.getLayoutParams() instanceof MarginLayoutParams) { diff --git a/app/src/sharedTest/java/org/oppia/android/app/databinding/MarginBindingAdaptersTest.kt b/app/src/sharedTest/java/org/oppia/android/app/databinding/MarginBindingAdaptersTest.kt index 735fe45c506..41caa3690f6 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/databinding/MarginBindingAdaptersTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/databinding/MarginBindingAdaptersTest.kt @@ -298,7 +298,7 @@ class MarginBindingAdaptersTest { @Config(qualifiers = "port") @Test - fun testMarginBindableAdapters_layoutParamsSettingPreservesMargins() { + fun testMarginBindableAdapters_setLayoutParams_preservesMargins() { val textView = activityRule.scenario.runWithActivity { val textView: TextView = it.findViewById(R.id.test_margin_text_view) @@ -322,7 +322,7 @@ class MarginBindingAdaptersTest { @Config(qualifiers = "land") @Test - fun testMarginBindableAdapters_layoutParamsSettingPreservesMargins_landscape() { + fun testMarginBindableAdapters_landscapeMode_setLayoutParams_preservesMargins() { val textView = activityRule.scenario.runWithActivity { val textView: TextView = it.findViewById(R.id.test_margin_text_view) @@ -346,7 +346,7 @@ class MarginBindingAdaptersTest { @Config(qualifiers = "sw600dp-port") @Test - fun testMarginBindableAdapters_layoutParamsSettingPreservesMargins_tablet() { + fun testMarginBindableAdapters_tabletMode_setLayoutParams_preservesMargins() { val textView = activityRule.scenario.runWithActivity { val textView: TextView = it.findViewById(R.id.test_margin_text_view) From 3bd9bd08d541f2549a7d7f6c1dca7c7c9642943c Mon Sep 17 00:00:00 2001 From: TanishMoral11 Date: Wed, 18 Dec 2024 22:01:55 +0530 Subject: [PATCH 5/5] Remove Unnecessary Comment --- .../oppia/android/app/databinding/MarginBindingAdaptersTest.kt | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/src/sharedTest/java/org/oppia/android/app/databinding/MarginBindingAdaptersTest.kt b/app/src/sharedTest/java/org/oppia/android/app/databinding/MarginBindingAdaptersTest.kt index 41caa3690f6..a8d68ef8997 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/databinding/MarginBindingAdaptersTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/databinding/MarginBindingAdaptersTest.kt @@ -311,7 +311,6 @@ class MarginBindingAdaptersTest { return@runWithActivity textView } - // Verify that the margins are correctly set after method calls assertThat(textView.marginStart.toFloat()).isWithin(TOLERANCE).of(24f) assertThat(textView.marginEnd.toFloat()).isWithin(TOLERANCE).of(40f) @@ -335,7 +334,6 @@ class MarginBindingAdaptersTest { return@runWithActivity textView } - // Verify that the margins are correctly set after method calls assertThat(textView.marginStart.toFloat()).isWithin(TOLERANCE).of(24f) assertThat(textView.marginEnd.toFloat()).isWithin(TOLERANCE).of(40f) @@ -359,7 +357,6 @@ class MarginBindingAdaptersTest { return@runWithActivity textView } - // Verify that the margins are correctly set after method calls assertThat(textView.marginStart.toFloat()).isWithin(TOLERANCE).of(24f) assertThat(textView.marginEnd.toFloat()).isWithin(TOLERANCE).of(40f)