From e695446b48dd06016f14b29c78c47601d5a34f61 Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Thu, 14 Jul 2022 21:46:02 -0700 Subject: [PATCH 01/57] Update CODEOWNERS to consolidate test-related ownership (#4434) * Update CODEOWNERS Remove anandwana001 from codeowners. * Update CODEOWNERS Correct instrumentation codeowner pattern. This is needed now that broad test ownership is no longer defined. --- .github/CODEOWNERS | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index bec340490e4..a9aaf5ce694 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -50,9 +50,6 @@ gradlew.bat @BenHenning # Devbots configurations. /.devbots/ @BenHenning -# All tests. -*Test.kt @anandwana001 - # All resource files. /app/src/main/res/**/*.xml @rt4914 /utility/src/main/res/**/*.xml @rt4914 @@ -72,7 +69,7 @@ gradlew.bat @BenHenning *Manifest.xml @BenHenning # Linter configuration. -buf.yaml @anandwana001 +buf.yaml @BenHenning # Third-party dependencies. /third_party/ @BenHenning @@ -144,7 +141,7 @@ config/kitkat_main_dex_class_list.txt @BenHenning /app/src/*/java/org/oppia/android/app/viewmodel/ @BenHenning # App testing infrastructure. -/app/src/*/java/org/oppia/android/app/testing/ @anandwana001 +/app/src/*/java/org/oppia/android/app/testing/ @BenHenning ##################################################################################### # domain module # @@ -167,7 +164,7 @@ config/kitkat_main_dex_class_list.txt @BenHenning ##################################################################################### # Global testing module code ownership. -/testing/**/*.kt @anandwana001 @BenHenning +/testing/**/*.kt @BenHenning ##################################################################################### # data module # @@ -200,9 +197,6 @@ config/kitkat_main_dex_class_list.txt @BenHenning # Global scripts code ownership. /scripts/ @BenHenning -# Shell file ownership. -/scripts/**/*.sh @anandwana001 @BenHenning - # Script proto ownership. /scripts/**/*.proto @BenHenning @@ -225,7 +219,7 @@ config/kitkat_main_dex_class_list.txt @BenHenning ##################################################################################### # End-to-end test utilities and modules. -/instrumentation/src/java/**/*.kt @anandwana001 @BenHenning +/instrumentation/**/*.kt @BenHenning ##################################################################################### # global overrides # From eeafdf58535854d5080ccfff89b9bcf5bb9a1d13 Mon Sep 17 00:00:00 2001 From: Jishnu <64526117+JishnuGoyal@users.noreply.github.com> Date: Tue, 19 Jul 2022 08:59:00 +0530 Subject: [PATCH 02/57] Fix #4006: Hide edit accounts option from administrator controls (#4204) * add enableEditAccountsOptions feature flag * add flag to module; add companion object to be used in test to force platform parameter values * use enableEditAccount flag to control visibilty * add specific tests to check visibility by forcing feature flag values * remove test module and directly force feature flag value using companion object * add PlatformParameterForceMode class to explicitly define force method * add forceMode parameter to functions * optimise import after merge with develop * move companion object to TestPlatformParameterModule.kt * copy platform parameters for this test module * simple code changes since older implementation was removed * optimise imports and lint * add enable edit account parameter explicity * update changes to the other OptionsFragmentTest.kt * testplatformparametermodule is a dependency for administratorcontrolactivitytest and optionsFragmentTest * nits * revert all changes to TestPlatformParameterModule * Use PlatformParameterModule instead, with visibility annotation for testing * Use PlatformParameterModule instead in tests like before * lint * change no longer needed * add kdoc * add accidently removed line * move companion object back to TestPlatformParameterModule.kt. * use testplatform parameter module as suggested * lint * remove extra resources * lint * fix bazel * adjust for merge updates from develop * lint * resolve bazel issues * add force method for learner study analytics and use it * lint * lint * remove test parameter platform module (not required) * nits * nit --- app/BUILD.bazel | 1 + .../AdministratorControlsViewModel.kt | 15 +- .../AdministratorControlsActivityTest.kt | 93 ++++++++++- .../AdministratorControlsFragmentTest.kt | 5 +- .../learneranalytics/BUILD.bazel | 1 + .../ProfileAndDeviceIdFragmentTest.kt | 42 +---- .../app/options/OptionsFragmentTest.kt | 70 +------- .../testing/options/OptionsFragmentTest.kt | 72 +-------- .../PlatformParameterModule.kt | 10 ++ .../TestPlatformParameterModule.kt | 152 ++++++++++++++++++ .../PlatformParameterConstants.kt | 7 + 11 files changed, 295 insertions(+), 173 deletions(-) diff --git a/app/BUILD.bazel b/app/BUILD.bazel index 7c9941d90a8..83841648a29 100644 --- a/app/BUILD.bazel +++ b/app/BUILD.bazel @@ -891,6 +891,7 @@ TEST_DEPS = [ "//testing/src/main/java/org/oppia/android/testing/mockito", "//testing/src/main/java/org/oppia/android/testing/network", "//testing/src/main/java/org/oppia/android/testing/network:test_module", + "//testing/src/main/java/org/oppia/android/testing/platformparameter:test_module", "//testing/src/main/java/org/oppia/android/testing/robolectric:is_on_robolectric", "//testing/src/main/java/org/oppia/android/testing/robolectric:test_module", "//testing/src/main/java/org/oppia/android/testing/threading:coroutine_executor_service", diff --git a/app/src/main/java/org/oppia/android/app/administratorcontrols/AdministratorControlsViewModel.kt b/app/src/main/java/org/oppia/android/app/administratorcontrols/AdministratorControlsViewModel.kt index 2917cffe6ed..0fc6ef415cc 100644 --- a/app/src/main/java/org/oppia/android/app/administratorcontrols/AdministratorControlsViewModel.kt +++ b/app/src/main/java/org/oppia/android/app/administratorcontrols/AdministratorControlsViewModel.kt @@ -20,6 +20,7 @@ import org.oppia.android.domain.oppialogger.OppiaLogger import org.oppia.android.domain.profile.ProfileManagementController import org.oppia.android.util.data.AsyncResult import org.oppia.android.util.data.DataProviders.Companion.toLiveData +import org.oppia.android.util.platformparameter.EnableEditAccountsOptionsUi import org.oppia.android.util.platformparameter.LearnerStudyAnalytics import org.oppia.android.util.platformparameter.PlatformParameterValue import javax.inject.Inject @@ -31,6 +32,8 @@ class AdministratorControlsViewModel @Inject constructor( private val fragment: Fragment, private val oppiaLogger: OppiaLogger, private val profileManagementController: ProfileManagementController, + @EnableEditAccountsOptionsUi + private val enableEditAccountsOptionsUi: PlatformParameterValue, @LearnerStudyAnalytics private val learnerStudyAnalytics: PlatformParameterValue ) { private val routeToProfileListListener = activity as RouteToProfileListListener @@ -71,9 +74,13 @@ class AdministratorControlsViewModel @Inject constructor( private fun processAdministratorControlsList( deviceSettings: DeviceSettings ): List { - val itemViewModelList: MutableList = mutableListOf( - AdministratorControlsGeneralViewModel() - ) + + val itemViewModelList = mutableListOf() + + if (enableEditAccountsOptionsUi.value) { + itemViewModelList.add(AdministratorControlsGeneralViewModel()) + } + itemViewModelList.add( AdministratorControlsProfileViewModel( routeToProfileListListener, @@ -84,6 +91,7 @@ class AdministratorControlsViewModel @Inject constructor( if (learnerStudyAnalytics.value) { itemViewModelList.add(AdministratorControlsProfileAndDeviceIdViewModel(activity)) } + itemViewModelList.add( AdministratorControlsDownloadPermissionsViewModel( fragment, @@ -93,6 +101,7 @@ class AdministratorControlsViewModel @Inject constructor( deviceSettings ) ) + itemViewModelList.add(AdministratorControlsAppInformationViewModel(activity)) itemViewModelList.add( AdministratorControlsAccountActionsViewModel( diff --git a/app/src/sharedTest/java/org/oppia/android/app/administratorcontrols/AdministratorControlsActivityTest.kt b/app/src/sharedTest/java/org/oppia/android/app/administratorcontrols/AdministratorControlsActivityTest.kt index ba2a841558c..1b46cb78cb4 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/administratorcontrols/AdministratorControlsActivityTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/administratorcontrols/AdministratorControlsActivityTest.kt @@ -21,6 +21,7 @@ import androidx.test.espresso.UiController import androidx.test.espresso.ViewAction import androidx.test.espresso.action.ViewActions.click import androidx.test.espresso.action.ViewActions.scrollTo +import androidx.test.espresso.assertion.ViewAssertions.doesNotExist import androidx.test.espresso.assertion.ViewAssertions.matches import androidx.test.espresso.contrib.RecyclerViewActions.scrollToPosition import androidx.test.espresso.intent.Intents @@ -89,7 +90,6 @@ import org.oppia.android.domain.oppialogger.LogStorageModule import org.oppia.android.domain.oppialogger.LoggingIdentifierModule import org.oppia.android.domain.oppialogger.analytics.ApplicationLifecycleModule import org.oppia.android.domain.oppialogger.loguploader.LogUploadWorkerModule -import org.oppia.android.domain.platformparameter.PlatformParameterModule import org.oppia.android.domain.platformparameter.PlatformParameterSingletonModule import org.oppia.android.domain.question.QuestionModule import org.oppia.android.domain.topic.PrimeTopicAssetsControllerModule @@ -97,6 +97,7 @@ import org.oppia.android.domain.workmanager.WorkManagerConfigurationModule import org.oppia.android.testing.OppiaTestRule import org.oppia.android.testing.TestLogReportingModule import org.oppia.android.testing.junit.InitializeDefaultLocaleRule +import org.oppia.android.testing.platformparameter.TestPlatformParameterModule import org.oppia.android.testing.profile.ProfileTestHelper import org.oppia.android.testing.robolectric.RobolectricModule import org.oppia.android.testing.threading.TestCoroutineDispatchers @@ -164,6 +165,7 @@ class AdministratorControlsActivityTest { @Before fun setUp() { + TestPlatformParameterModule.forceEnableEditAccountsOptionsUi(true) Intents.init() setUpTestApplicationComponent() testCoroutineDispatchers.registerIdlingResource() @@ -193,6 +195,67 @@ class AdministratorControlsActivityTest { } } + @Test + fun testAdministratorControlsFragment_editAccountOptionsEnabled_generalOptionsIsDisplayed() { + launch( + createAdministratorControlsActivityIntent( + profileId = internalProfileId + ) + ).use { + testCoroutineDispatchers.runCurrent() + verifyItemDisplayedOnAdministratorControlListItem( + itemPosition = 0, + targetView = R.id.general_text_view + ) + verifyTextOnAdministratorListItemAtPosition( + itemPosition = 0, + targetViewId = R.id.edit_account_text_view, + stringIdToMatch = R.string.administrator_controls_edit_account + ) + } + } + + @Test + fun testAdministratorControlsFragment_editAccountOptionsDisabled_generalOptionsIsNotDisplayed() { + TestPlatformParameterModule.forceEnableEditAccountsOptionsUi(false) + + launch( + createAdministratorControlsActivityIntent( + profileId = internalProfileId + ) + ).use { + testCoroutineDispatchers.runCurrent() + verifyItemDisplayedOnAdministratorControlListItemDoesNotExist( + itemPosition = 0, + targetView = R.id.general_text_view + ) + verifyTextViewOnAdministratorListItemAtPositionDoesNotExist( + itemPosition = 0, + targetViewId = R.id.edit_account_text_view, + ) + } + } + + @Test + fun testAdministratorControlsFragment_profileManagementIsDisplayed() { + launch( + createAdministratorControlsActivityIntent( + profileId = internalProfileId + ) + ).use { + testCoroutineDispatchers.runCurrent() + verifyItemDisplayedOnAdministratorControlListItem( + itemPosition = 1, + targetView = R.id.profile_management_text_view + ) + verifyTextOnAdministratorListItemAtPosition( + itemPosition = 1, + targetViewId = R.id.edit_profiles_text_view, + stringIdToMatch = R.string.administrator_controls_edit_profiles + ) + } + } + // TODO(#762): Replace [ProfileChooserActivity] to [LoginActivity] once it is added. @Test fun testAdministratorControlsFragment_clickOkButtonInLogoutDialog_opensProfileChooserActivity() { @@ -691,6 +754,19 @@ class AdministratorControlsActivityTest { ).check(matches(isDisplayed())) } + private fun verifyItemDisplayedOnAdministratorControlListItemDoesNotExist( + itemPosition: Int, + targetView: Int + ) { + onView( + atPositionOnView( + recyclerViewId = R.id.administrator_controls_list, + position = itemPosition, + targetViewId = targetView + ) + ).check(doesNotExist()) + } + private fun verifyTextOnAdministratorListItemAtPosition( itemPosition: Int, targetViewId: Int, @@ -705,6 +781,19 @@ class AdministratorControlsActivityTest { ).check(matches(withText(context.getString(stringIdToMatch)))) } + private fun verifyTextViewOnAdministratorListItemAtPositionDoesNotExist( + itemPosition: Int, + targetViewId: Int, + ) { + onView( + atPositionOnView( + recyclerViewId = R.id.administrator_controls_list, + position = itemPosition, + targetViewId = targetViewId + ) + ).check(doesNotExist()) + } + private fun scrollToPosition(position: Int) { onView(withId(R.id.administrator_controls_list)).perform( scrollToPosition( @@ -724,7 +813,7 @@ class AdministratorControlsActivityTest { @Component( modules = [ RobolectricModule::class, - PlatformParameterModule::class, PlatformParameterSingletonModule::class, + TestPlatformParameterModule::class, PlatformParameterSingletonModule::class, TestDispatcherModule::class, ApplicationModule::class, LoggerModule::class, ContinueModule::class, FractionInputModule::class, ItemSelectionInputModule::class, MultipleChoiceInputModule::class, diff --git a/app/src/sharedTest/java/org/oppia/android/app/administratorcontrols/AdministratorControlsFragmentTest.kt b/app/src/sharedTest/java/org/oppia/android/app/administratorcontrols/AdministratorControlsFragmentTest.kt index c7694223169..2f737fe499b 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/administratorcontrols/AdministratorControlsFragmentTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/administratorcontrols/AdministratorControlsFragmentTest.kt @@ -81,7 +81,6 @@ import org.oppia.android.domain.oppialogger.LogStorageModule import org.oppia.android.domain.oppialogger.LoggingIdentifierModule import org.oppia.android.domain.oppialogger.analytics.ApplicationLifecycleModule import org.oppia.android.domain.oppialogger.loguploader.LogUploadWorkerModule -import org.oppia.android.domain.platformparameter.PlatformParameterModule import org.oppia.android.domain.platformparameter.PlatformParameterSingletonModule import org.oppia.android.domain.question.QuestionModule import org.oppia.android.domain.topic.PrimeTopicAssetsControllerModule @@ -89,6 +88,7 @@ import org.oppia.android.domain.workmanager.WorkManagerConfigurationModule import org.oppia.android.testing.OppiaTestRule import org.oppia.android.testing.TestLogReportingModule import org.oppia.android.testing.junit.InitializeDefaultLocaleRule +import org.oppia.android.testing.platformparameter.TestPlatformParameterModule import org.oppia.android.testing.profile.ProfileTestHelper import org.oppia.android.testing.robolectric.RobolectricModule import org.oppia.android.testing.threading.TestCoroutineDispatchers @@ -143,6 +143,7 @@ class AdministratorControlsFragmentTest { setUpTestApplicationComponent() profileTestHelper.initializeProfiles() testCoroutineDispatchers.registerIdlingResource() + TestPlatformParameterModule.forceEnableEditAccountsOptionsUi(true) } @After @@ -590,7 +591,7 @@ class AdministratorControlsFragmentTest { @Component( modules = [ RobolectricModule::class, - PlatformParameterModule::class, PlatformParameterSingletonModule::class, + TestPlatformParameterModule::class, PlatformParameterSingletonModule::class, TestDispatcherModule::class, ApplicationModule::class, LoggerModule::class, ContinueModule::class, FractionInputModule::class, ItemSelectionInputModule::class, MultipleChoiceInputModule::class, diff --git a/app/src/sharedTest/java/org/oppia/android/app/administratorcontrols/learneranalytics/BUILD.bazel b/app/src/sharedTest/java/org/oppia/android/app/administratorcontrols/learneranalytics/BUILD.bazel index a62f39766e0..383d73493df 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/administratorcontrols/learneranalytics/BUILD.bazel +++ b/app/src/sharedTest/java/org/oppia/android/app/administratorcontrols/learneranalytics/BUILD.bazel @@ -38,6 +38,7 @@ app_test( "//app/src/main/java/org/oppia/android/app/translation/testing:test_module", "//testing", "//testing/src/main/java/org/oppia/android/testing/junit:initialize_default_locale_rule", + "//testing/src/main/java/org/oppia/android/testing/platformparameter:test_module", "//testing/src/main/java/org/oppia/android/testing/robolectric:test_module", "//testing/src/main/java/org/oppia/android/testing/threading:test_module", "//testing/src/main/java/org/oppia/android/testing/time:test_module", diff --git a/app/src/sharedTest/java/org/oppia/android/app/administratorcontrols/learneranalytics/ProfileAndDeviceIdFragmentTest.kt b/app/src/sharedTest/java/org/oppia/android/app/administratorcontrols/learneranalytics/ProfileAndDeviceIdFragmentTest.kt index d6f4bc57c84..affe4a867b5 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/administratorcontrols/learneranalytics/ProfileAndDeviceIdFragmentTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/administratorcontrols/learneranalytics/ProfileAndDeviceIdFragmentTest.kt @@ -86,6 +86,7 @@ import org.oppia.android.domain.workmanager.WorkManagerConfigurationModule import org.oppia.android.testing.OppiaTestRule import org.oppia.android.testing.TestLogReportingModule import org.oppia.android.testing.junit.InitializeDefaultLocaleRule +import org.oppia.android.testing.platformparameter.TestPlatformParameterModule import org.oppia.android.testing.profile.ProfileTestHelper import org.oppia.android.testing.robolectric.RobolectricModule import org.oppia.android.testing.threading.TestCoroutineDispatchers @@ -106,16 +107,6 @@ import org.oppia.android.util.networking.NetworkConnectionUtilDebugModule import org.oppia.android.util.parser.html.HtmlParserEntityTypeModule import org.oppia.android.util.parser.image.GlideImageLoaderModule import org.oppia.android.util.parser.image.ImageParsingModule -import org.oppia.android.util.platformparameter.CACHE_LATEX_RENDERING_DEFAULT_VALUE -import org.oppia.android.util.platformparameter.CacheLatexRendering -import org.oppia.android.util.platformparameter.ENABLE_LANGUAGE_SELECTION_UI_DEFAULT_VALUE -import org.oppia.android.util.platformparameter.EnableLanguageSelectionUi -import org.oppia.android.util.platformparameter.LearnerStudyAnalytics -import org.oppia.android.util.platformparameter.PlatformParameterValue -import org.oppia.android.util.platformparameter.SPLASH_SCREEN_WELCOME_MSG_DEFAULT_VALUE -import org.oppia.android.util.platformparameter.SYNC_UP_WORKER_TIME_PERIOD_IN_HOURS_DEFAULT_VALUE -import org.oppia.android.util.platformparameter.SplashScreenWelcomeMsg -import org.oppia.android.util.platformparameter.SyncUpWorkerTimePeriodHours import org.oppia.android.util.system.OppiaClock import org.robolectric.annotation.Config import org.robolectric.annotation.LooperMode @@ -162,6 +153,8 @@ class ProfileAndDeviceIdFragmentTest { @Before fun setUp() { + TestPlatformParameterModule.forceEnableEditAccountsOptionsUi(true) + TestPlatformParameterModule.forceEnableLearnerStudyAnalytics(true) setUpTestApplicationComponent() testCoroutineDispatchers.registerIdlingResource() profileTestHelper.addOnlyAdminProfile() @@ -604,40 +597,13 @@ class ProfileAndDeviceIdFragmentTest { @Provides @ApplicationIdSeed fun provideFakeApplicationIdSeed(): Long = fixedApplicationId - - @Provides - @SplashScreenWelcomeMsg - fun provideSplashScreenWelcomeMsgParam(): PlatformParameterValue = - PlatformParameterValue.createDefaultParameter(SPLASH_SCREEN_WELCOME_MSG_DEFAULT_VALUE) - - @Provides - @SyncUpWorkerTimePeriodHours - fun provideSyncUpWorkerTimePeriod(): PlatformParameterValue { - return PlatformParameterValue.createDefaultParameter( - SYNC_UP_WORKER_TIME_PERIOD_IN_HOURS_DEFAULT_VALUE - ) - } - - @Provides - @EnableLanguageSelectionUi - fun provideEnableLanguageSelectionUi(): PlatformParameterValue = - PlatformParameterValue.createDefaultParameter(ENABLE_LANGUAGE_SELECTION_UI_DEFAULT_VALUE) - - @Provides - @LearnerStudyAnalytics - fun provideLearnerStudyAnalytics(): PlatformParameterValue = - PlatformParameterValue.createDefaultParameter(true) - - @Provides - @CacheLatexRendering - fun provideCacheLatexRendering(): PlatformParameterValue = - PlatformParameterValue.createDefaultParameter(CACHE_LATEX_RENDERING_DEFAULT_VALUE) } // TODO(#59): Figure out a way to reuse modules instead of needing to re-declare them. @Singleton @Component( modules = [ + TestPlatformParameterModule::class, TestModule::class, RobolectricModule::class, PlatformParameterSingletonModule::class, TestDispatcherModule::class, ApplicationModule::class, LoggerModule::class, ContinueModule::class, FractionInputModule::class, ItemSelectionInputModule::class, diff --git a/app/src/sharedTest/java/org/oppia/android/app/options/OptionsFragmentTest.kt b/app/src/sharedTest/java/org/oppia/android/app/options/OptionsFragmentTest.kt index 27884619dd4..8d8dd411b1b 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/options/OptionsFragmentTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/options/OptionsFragmentTest.kt @@ -25,8 +25,6 @@ import androidx.test.espresso.matcher.ViewMatchers.withText import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.rule.ActivityTestRule import dagger.Component -import dagger.Module -import dagger.Provides import org.hamcrest.Matchers.allOf import org.junit.After import org.junit.Before @@ -80,6 +78,7 @@ import org.oppia.android.domain.workmanager.WorkManagerConfigurationModule import org.oppia.android.testing.OppiaTestRule import org.oppia.android.testing.TestLogReportingModule import org.oppia.android.testing.junit.InitializeDefaultLocaleRule +import org.oppia.android.testing.platformparameter.TestPlatformParameterModule import org.oppia.android.testing.profile.ProfileTestHelper import org.oppia.android.testing.robolectric.RobolectricModule import org.oppia.android.testing.threading.TestCoroutineDispatchers @@ -98,19 +97,6 @@ import org.oppia.android.util.networking.NetworkConnectionUtilDebugModule import org.oppia.android.util.parser.html.HtmlParserEntityTypeModule import org.oppia.android.util.parser.image.GlideImageLoaderModule import org.oppia.android.util.parser.image.ImageParsingModule -import org.oppia.android.util.platformparameter.CACHE_LATEX_RENDERING -import org.oppia.android.util.platformparameter.CACHE_LATEX_RENDERING_DEFAULT_VALUE -import org.oppia.android.util.platformparameter.CacheLatexRendering -import org.oppia.android.util.platformparameter.EnableLanguageSelectionUi -import org.oppia.android.util.platformparameter.LEARNER_STUDY_ANALYTICS -import org.oppia.android.util.platformparameter.LEARNER_STUDY_ANALYTICS_DEFAULT_VALUE -import org.oppia.android.util.platformparameter.LearnerStudyAnalytics -import org.oppia.android.util.platformparameter.PlatformParameterSingleton -import org.oppia.android.util.platformparameter.PlatformParameterValue -import org.oppia.android.util.platformparameter.SPLASH_SCREEN_WELCOME_MSG_DEFAULT_VALUE -import org.oppia.android.util.platformparameter.SYNC_UP_WORKER_TIME_PERIOD_IN_HOURS_DEFAULT_VALUE -import org.oppia.android.util.platformparameter.SplashScreenWelcomeMsg -import org.oppia.android.util.platformparameter.SyncUpWorkerTimePeriodHours import org.robolectric.annotation.Config import org.robolectric.annotation.LooperMode import javax.inject.Inject @@ -141,7 +127,7 @@ class OptionsFragmentTest { @Before fun setUp() { - TestModule.forceEnableLanguageSelectionUi = true + TestPlatformParameterModule.forceEnableLanguageSelectionUi(true) Intents.init() setUpTestApplicationComponent() testCoroutineDispatchers.registerIdlingResource() @@ -352,7 +338,6 @@ class OptionsFragmentTest { @Test fun testOptionsFragment_featureEnabled_appLanguageOptionIsDisplayed() { - TestModule.forceEnableLanguageSelectionUi = true launch( createOptionActivityIntent( internalProfileId = 0, @@ -366,7 +351,8 @@ class OptionsFragmentTest { @Test fun testOptionsFragment_featureDisabled_appLanguageOptionIsNotDisplayed() { - TestModule.forceEnableLanguageSelectionUi = false + TestPlatformParameterModule.forceEnableLanguageSelectionUi(false) + launch( createOptionActivityIntent( internalProfileId = 0, @@ -640,56 +626,12 @@ class OptionsFragmentTest { testCoroutineDispatchers.runCurrent() } - @Module - class TestModule { - companion object { - var forceEnableLanguageSelectionUi: Boolean = true - } - - @Provides - @SplashScreenWelcomeMsg - fun provideSplashScreenWelcomeMsgParam(): PlatformParameterValue { - return PlatformParameterValue.createDefaultParameter(SPLASH_SCREEN_WELCOME_MSG_DEFAULT_VALUE) - } - - @Provides - @SyncUpWorkerTimePeriodHours - fun provideSyncUpWorkerTimePeriod(): PlatformParameterValue { - return PlatformParameterValue.createDefaultParameter( - SYNC_UP_WORKER_TIME_PERIOD_IN_HOURS_DEFAULT_VALUE - ) - } - - @Provides - @EnableLanguageSelectionUi - fun provideEnableLanguageSelectionUi(): PlatformParameterValue { - return PlatformParameterValue.createDefaultParameter(forceEnableLanguageSelectionUi) - } - - @Provides - @LearnerStudyAnalytics - fun provideLearnerStudyAnalytics( - platformParameterSingleton: PlatformParameterSingleton - ): PlatformParameterValue { - return platformParameterSingleton.getBooleanPlatformParameter(LEARNER_STUDY_ANALYTICS) - ?: PlatformParameterValue.createDefaultParameter(LEARNER_STUDY_ANALYTICS_DEFAULT_VALUE) - } - - @Provides - @CacheLatexRendering - fun provideCacheLatexRendering( - platformParameterSingleton: PlatformParameterSingleton - ): PlatformParameterValue { - return platformParameterSingleton.getBooleanPlatformParameter(CACHE_LATEX_RENDERING) - ?: PlatformParameterValue.createDefaultParameter(CACHE_LATEX_RENDERING_DEFAULT_VALUE) - } - } - // TODO(#59): Figure out a way to reuse modules instead of needing to re-declare them. @Singleton @Component( modules = [ - TestModule::class, RobolectricModule::class, PlatformParameterSingletonModule::class, + TestPlatformParameterModule::class, + RobolectricModule::class, PlatformParameterSingletonModule::class, TestDispatcherModule::class, ApplicationModule::class, LoggerModule::class, ContinueModule::class, FractionInputModule::class, ItemSelectionInputModule::class, MultipleChoiceInputModule::class, diff --git a/app/src/test/java/org/oppia/android/app/testing/options/OptionsFragmentTest.kt b/app/src/test/java/org/oppia/android/app/testing/options/OptionsFragmentTest.kt index da8a16bdcd9..0c9b6968ccd 100644 --- a/app/src/test/java/org/oppia/android/app/testing/options/OptionsFragmentTest.kt +++ b/app/src/test/java/org/oppia/android/app/testing/options/OptionsFragmentTest.kt @@ -14,8 +14,6 @@ import androidx.test.espresso.matcher.ViewMatchers.withId import androidx.test.ext.junit.runners.AndroidJUnit4 import com.google.common.truth.Truth.assertThat import dagger.Component -import dagger.Module -import dagger.Provides import org.junit.After import org.junit.Before import org.junit.Rule @@ -70,6 +68,7 @@ import org.oppia.android.domain.topic.PrimeTopicAssetsControllerModule import org.oppia.android.domain.workmanager.WorkManagerConfigurationModule import org.oppia.android.testing.TestLogReportingModule import org.oppia.android.testing.junit.InitializeDefaultLocaleRule +import org.oppia.android.testing.platformparameter.TestPlatformParameterModule import org.oppia.android.testing.robolectric.RobolectricModule import org.oppia.android.testing.threading.TestCoroutineDispatchers import org.oppia.android.testing.threading.TestDispatcherModule @@ -87,19 +86,7 @@ import org.oppia.android.util.networking.NetworkConnectionUtilDebugModule import org.oppia.android.util.parser.html.HtmlParserEntityTypeModule import org.oppia.android.util.parser.image.GlideImageLoaderModule import org.oppia.android.util.parser.image.ImageParsingModule -import org.oppia.android.util.platformparameter.CACHE_LATEX_RENDERING -import org.oppia.android.util.platformparameter.CACHE_LATEX_RENDERING_DEFAULT_VALUE -import org.oppia.android.util.platformparameter.CacheLatexRendering -import org.oppia.android.util.platformparameter.EnableLanguageSelectionUi -import org.oppia.android.util.platformparameter.LEARNER_STUDY_ANALYTICS -import org.oppia.android.util.platformparameter.LEARNER_STUDY_ANALYTICS_DEFAULT_VALUE -import org.oppia.android.util.platformparameter.LearnerStudyAnalytics -import org.oppia.android.util.platformparameter.PlatformParameterSingleton -import org.oppia.android.util.platformparameter.PlatformParameterValue -import org.oppia.android.util.platformparameter.SPLASH_SCREEN_WELCOME_MSG_DEFAULT_VALUE -import org.oppia.android.util.platformparameter.SYNC_UP_WORKER_TIME_PERIOD_IN_HOURS_DEFAULT_VALUE -import org.oppia.android.util.platformparameter.SplashScreenWelcomeMsg -import org.oppia.android.util.platformparameter.SyncUpWorkerTimePeriodHours +import org.oppia.android.util.platformparameter.ENABLE_EDIT_ACCOUNTS_OPTIONS_UI_DEFAULT_VALUE import org.robolectric.annotation.Config import org.robolectric.annotation.LooperMode import javax.inject.Inject @@ -117,7 +104,10 @@ class OptionsFragmentTest { @Before fun setUp() { - TestModule.forceEnableLanguageSelectionUi = true + TestPlatformParameterModule.forceEnableLanguageSelectionUi(true) + TestPlatformParameterModule.forceEnableEditAccountsOptionsUi( + ENABLE_EDIT_ACCOUNTS_OPTIONS_UI_DEFAULT_VALUE + ) setUpTestApplicationComponent() testCoroutineDispatchers.registerIdlingResource() } @@ -162,7 +152,6 @@ class OptionsFragmentTest { @Test fun testOptionsFragment_featureEnabled_appLanguageItemIsDisplayed() { - TestModule.forceEnableLanguageSelectionUi = true launch(createOptionActivityIntent(0, true)).use { testCoroutineDispatchers.runCurrent() @@ -172,7 +161,7 @@ class OptionsFragmentTest { @Test fun testOptionsFragment_featureDisabled_appLanguageItemIsNotDisplayed() { - TestModule.forceEnableLanguageSelectionUi = false + TestPlatformParameterModule.forceEnableLanguageSelectionUi(false) launch(createOptionActivityIntent(0, true)).use { testCoroutineDispatchers.runCurrent() @@ -237,57 +226,12 @@ class OptionsFragmentTest { ApplicationProvider.getApplicationContext().inject(this) } - @Module - class TestModule { - companion object { - var forceEnableLanguageSelectionUi: Boolean = true - } - - @Provides - @SplashScreenWelcomeMsg - fun provideSplashScreenWelcomeMsgParam(): PlatformParameterValue { - return PlatformParameterValue.createDefaultParameter(SPLASH_SCREEN_WELCOME_MSG_DEFAULT_VALUE) - } - - @Provides - @SyncUpWorkerTimePeriodHours - fun provideSyncUpWorkerTimePeriod(): PlatformParameterValue { - return PlatformParameterValue.createDefaultParameter( - SYNC_UP_WORKER_TIME_PERIOD_IN_HOURS_DEFAULT_VALUE - ) - } - - @Provides - @EnableLanguageSelectionUi - fun provideEnableLanguageSelectionUi(): PlatformParameterValue { - return PlatformParameterValue.createDefaultParameter(forceEnableLanguageSelectionUi) - } - - @Provides - @LearnerStudyAnalytics - fun provideLearnerStudyAnalytics( - platformParameterSingleton: PlatformParameterSingleton - ): PlatformParameterValue { - return platformParameterSingleton.getBooleanPlatformParameter(LEARNER_STUDY_ANALYTICS) - ?: PlatformParameterValue.createDefaultParameter(LEARNER_STUDY_ANALYTICS_DEFAULT_VALUE) - } - - @Provides - @CacheLatexRendering - fun provideCacheLatexRendering( - platformParameterSingleton: PlatformParameterSingleton - ): PlatformParameterValue { - return platformParameterSingleton.getBooleanPlatformParameter(CACHE_LATEX_RENDERING) - ?: PlatformParameterValue.createDefaultParameter(CACHE_LATEX_RENDERING_DEFAULT_VALUE) - } - } - // TODO(#59): Figure out a way to reuse modules instead of needing to re-declare them. @Singleton @Component( modules = [ TestDispatcherModule::class, ApplicationModule::class, RobolectricModule::class, - TestModule::class, PlatformParameterSingletonModule::class, + TestPlatformParameterModule::class, PlatformParameterSingletonModule::class, LoggerModule::class, ContinueModule::class, FractionInputModule::class, ItemSelectionInputModule::class, MultipleChoiceInputModule::class, NumberWithUnitsRuleModule::class, NumericInputRuleModule::class, TextInputRuleModule::class, diff --git a/domain/src/main/java/org/oppia/android/domain/platformparameter/PlatformParameterModule.kt b/domain/src/main/java/org/oppia/android/domain/platformparameter/PlatformParameterModule.kt index 41f2abfacb0..f11a055f512 100644 --- a/domain/src/main/java/org/oppia/android/domain/platformparameter/PlatformParameterModule.kt +++ b/domain/src/main/java/org/oppia/android/domain/platformparameter/PlatformParameterModule.kt @@ -5,9 +5,11 @@ import dagger.Provides import org.oppia.android.util.platformparameter.CACHE_LATEX_RENDERING import org.oppia.android.util.platformparameter.CACHE_LATEX_RENDERING_DEFAULT_VALUE import org.oppia.android.util.platformparameter.CacheLatexRendering +import org.oppia.android.util.platformparameter.ENABLE_EDIT_ACCOUNTS_OPTIONS_UI_DEFAULT_VALUE import org.oppia.android.util.platformparameter.ENABLE_LANGUAGE_SELECTION_UI_DEFAULT_VALUE import org.oppia.android.util.platformparameter.ENABLE_PERFORMANCE_METRICS_COLLECTION import org.oppia.android.util.platformparameter.ENABLE_PERFORMANCE_METRICS_COLLECTION_DEFAULT_VALUE +import org.oppia.android.util.platformparameter.EnableEditAccountsOptionsUi import org.oppia.android.util.platformparameter.EnableLanguageSelectionUi import org.oppia.android.util.platformparameter.EnablePerformanceMetricsCollection import org.oppia.android.util.platformparameter.LEARNER_STUDY_ANALYTICS @@ -63,6 +65,14 @@ class PlatformParameterModule { ) } + @Provides + @EnableEditAccountsOptionsUi + fun provideEnableEditAccountsOptionsUi(): PlatformParameterValue { + return PlatformParameterValue.createDefaultParameter( + ENABLE_EDIT_ACCOUNTS_OPTIONS_UI_DEFAULT_VALUE + ) + } + @Provides @LearnerStudyAnalytics fun provideLearnerStudyAnalytics( diff --git a/testing/src/main/java/org/oppia/android/testing/platformparameter/TestPlatformParameterModule.kt b/testing/src/main/java/org/oppia/android/testing/platformparameter/TestPlatformParameterModule.kt index 41974278205..08bc692557a 100644 --- a/testing/src/main/java/org/oppia/android/testing/platformparameter/TestPlatformParameterModule.kt +++ b/testing/src/main/java/org/oppia/android/testing/platformparameter/TestPlatformParameterModule.kt @@ -1,9 +1,37 @@ package org.oppia.android.testing.platformparameter +import androidx.annotation.VisibleForTesting import dagger.Module import dagger.Provides +import org.oppia.android.util.platformparameter.CACHE_LATEX_RENDERING +import org.oppia.android.util.platformparameter.CACHE_LATEX_RENDERING_DEFAULT_VALUE +import org.oppia.android.util.platformparameter.CacheLatexRendering +import org.oppia.android.util.platformparameter.ENABLE_EDIT_ACCOUNTS_OPTIONS_UI_DEFAULT_VALUE +import org.oppia.android.util.platformparameter.ENABLE_LANGUAGE_SELECTION_UI_DEFAULT_VALUE +import org.oppia.android.util.platformparameter.ENABLE_PERFORMANCE_METRICS_COLLECTION +import org.oppia.android.util.platformparameter.ENABLE_PERFORMANCE_METRICS_COLLECTION_DEFAULT_VALUE +import org.oppia.android.util.platformparameter.EnableEditAccountsOptionsUi +import org.oppia.android.util.platformparameter.EnableLanguageSelectionUi +import org.oppia.android.util.platformparameter.EnablePerformanceMetricsCollection +import org.oppia.android.util.platformparameter.LEARNER_STUDY_ANALYTICS_DEFAULT_VALUE +import org.oppia.android.util.platformparameter.LearnerStudyAnalytics +import org.oppia.android.util.platformparameter.PERFORMANCE_METRICS_COLLECTION_HIGH_FREQUENCY_TIME_INTERVAL_IN_MINUTES +import org.oppia.android.util.platformparameter.PERFORMANCE_METRICS_COLLECTION_HIGH_FREQUENCY_TIME_INTERVAL_IN_MINUTES_DEFAULT_VAL +import org.oppia.android.util.platformparameter.PERFORMANCE_METRICS_COLLECTION_LOW_FREQUENCY_TIME_INTERVAL_IN_MINUTES +import org.oppia.android.util.platformparameter.PERFORMANCE_METRICS_COLLECTION_LOW_FREQUENCY_TIME_INTERVAL_IN_MINUTES_DEFAULT_VAL +import org.oppia.android.util.platformparameter.PERFORMANCE_METRICS_COLLECTION_UPLOAD_TIME_INTERVAL_IN_MINUTES +import org.oppia.android.util.platformparameter.PERFORMANCE_METRICS_COLLECTION_UPLOAD_TIME_INTERVAL_IN_MINUTES_DEFAULT_VAL +import org.oppia.android.util.platformparameter.PerformanceMetricsCollectionHighFrequencyTimeIntervalInMinutes +import org.oppia.android.util.platformparameter.PerformanceMetricsCollectionLowFrequencyTimeIntervalInMinutes +import org.oppia.android.util.platformparameter.PerformanceMetricsCollectionUploadTimeIntervalInMinutes import org.oppia.android.util.platformparameter.PlatformParameterSingleton import org.oppia.android.util.platformparameter.PlatformParameterValue +import org.oppia.android.util.platformparameter.SPLASH_SCREEN_WELCOME_MSG +import org.oppia.android.util.platformparameter.SPLASH_SCREEN_WELCOME_MSG_DEFAULT_VALUE +import org.oppia.android.util.platformparameter.SYNC_UP_WORKER_TIME_PERIOD_IN_HOURS +import org.oppia.android.util.platformparameter.SYNC_UP_WORKER_TIME_PERIOD_IN_HOURS_DEFAULT_VALUE +import org.oppia.android.util.platformparameter.SplashScreenWelcomeMsg +import org.oppia.android.util.platformparameter.SyncUpWorkerTimePeriodHours import javax.inject.Singleton /* Fake Platform Parameter Module that provides individual Platform Parameters for testing. */ @@ -39,4 +67,128 @@ class TestPlatformParameterModule { return platformParameterSingleton.getBooleanPlatformParameter(TEST_BOOLEAN_PARAM_NAME) ?: PlatformParameterValue.createDefaultParameter(TEST_BOOLEAN_PARAM_DEFAULT_VALUE) } + + @Provides + @SplashScreenWelcomeMsg + fun provideSplashScreenWelcomeMsgParam( + platformParameterSingleton: PlatformParameterSingleton + ): PlatformParameterValue { + return platformParameterSingleton.getBooleanPlatformParameter(SPLASH_SCREEN_WELCOME_MSG) + ?: PlatformParameterValue.createDefaultParameter(SPLASH_SCREEN_WELCOME_MSG_DEFAULT_VALUE) + } + + @Provides + @SyncUpWorkerTimePeriodHours + fun provideSyncUpWorkerTimePeriod( + platformParameterSingleton: PlatformParameterSingleton + ): PlatformParameterValue { + return platformParameterSingleton.getIntegerPlatformParameter( + SYNC_UP_WORKER_TIME_PERIOD_IN_HOURS + ) ?: PlatformParameterValue.createDefaultParameter( + SYNC_UP_WORKER_TIME_PERIOD_IN_HOURS_DEFAULT_VALUE + ) + } + + @Provides + @EnableLanguageSelectionUi + fun provideEnableLanguageSelectionUi(): PlatformParameterValue { + return PlatformParameterValue.createDefaultParameter( + enableLanguageSelectionUi + ) + } + + @Provides + @EnableEditAccountsOptionsUi + fun provideEnableEditAccountsOptionsUi(): PlatformParameterValue { + return PlatformParameterValue.createDefaultParameter( + enableEditAccountsOptionsUi + ) + } + + @Provides + @LearnerStudyAnalytics + fun provideLearnerStudyAnalytics(): PlatformParameterValue { + return PlatformParameterValue.createDefaultParameter(enableLearnerStudyAnalytics) + } + + @Provides + @CacheLatexRendering + fun provideCacheLatexRendering( + platformParameterSingleton: PlatformParameterSingleton + ): PlatformParameterValue { + return platformParameterSingleton.getBooleanPlatformParameter(CACHE_LATEX_RENDERING) + ?: PlatformParameterValue.createDefaultParameter(CACHE_LATEX_RENDERING_DEFAULT_VALUE) + } + + @Provides + @EnablePerformanceMetricsCollection + fun provideEnablePerformanceMetricCollection( + platformParameterSingleton: PlatformParameterSingleton + ): PlatformParameterValue { + return platformParameterSingleton.getBooleanPlatformParameter( + ENABLE_PERFORMANCE_METRICS_COLLECTION + ) ?: PlatformParameterValue.createDefaultParameter( + ENABLE_PERFORMANCE_METRICS_COLLECTION_DEFAULT_VALUE + ) + } + + @Provides + @PerformanceMetricsCollectionUploadTimeIntervalInMinutes + fun providePerformanceMetricsCollectionUploadTimeIntervalInMinutes( + platformParameterSingleton: PlatformParameterSingleton + ): PlatformParameterValue { + return platformParameterSingleton.getIntegerPlatformParameter( + PERFORMANCE_METRICS_COLLECTION_UPLOAD_TIME_INTERVAL_IN_MINUTES + ) ?: PlatformParameterValue.createDefaultParameter( + PERFORMANCE_METRICS_COLLECTION_UPLOAD_TIME_INTERVAL_IN_MINUTES_DEFAULT_VAL + ) + } + + @Provides + @PerformanceMetricsCollectionHighFrequencyTimeIntervalInMinutes + fun providePerformanceMetricsCollectionHighFrequencyTimeIntervalInMinutes( + platformParameterSingleton: PlatformParameterSingleton + ): PlatformParameterValue { + return platformParameterSingleton.getIntegerPlatformParameter( + PERFORMANCE_METRICS_COLLECTION_HIGH_FREQUENCY_TIME_INTERVAL_IN_MINUTES + ) ?: PlatformParameterValue.createDefaultParameter( + PERFORMANCE_METRICS_COLLECTION_HIGH_FREQUENCY_TIME_INTERVAL_IN_MINUTES_DEFAULT_VAL + ) + } + + @Provides + @PerformanceMetricsCollectionLowFrequencyTimeIntervalInMinutes + fun providePerformanceMetricsCollectionLowFrequencyTimeIntervalInMinutes( + platformParameterSingleton: PlatformParameterSingleton + ): PlatformParameterValue { + return platformParameterSingleton.getIntegerPlatformParameter( + PERFORMANCE_METRICS_COLLECTION_LOW_FREQUENCY_TIME_INTERVAL_IN_MINUTES + ) ?: PlatformParameterValue.createDefaultParameter( + PERFORMANCE_METRICS_COLLECTION_LOW_FREQUENCY_TIME_INTERVAL_IN_MINUTES_DEFAULT_VAL + ) + } + + companion object { + private var enableLanguageSelectionUi = ENABLE_LANGUAGE_SELECTION_UI_DEFAULT_VALUE + private var enableEditAccountsOptionsUi = ENABLE_EDIT_ACCOUNTS_OPTIONS_UI_DEFAULT_VALUE + private var enableLearnerStudyAnalytics = LEARNER_STUDY_ANALYTICS_DEFAULT_VALUE + + /** Enables forcing [EnableLanguageSelectionUi] platform parameter flag from tests. */ + @VisibleForTesting(otherwise = VisibleForTesting.NONE) + fun forceEnableLanguageSelectionUi(value: Boolean) { + enableLanguageSelectionUi = value + } + + /** Enables forcing [EnableEditAccountsOptionsUI] platform parameter flag from tests. */ + @VisibleForTesting(otherwise = VisibleForTesting.NONE) + fun forceEnableEditAccountsOptionsUi(value: Boolean) { + enableEditAccountsOptionsUi = value + } + + /** Enables forcing [EnableLearnerStudyAnalytics] platform parameter flag from tests. */ + @VisibleForTesting(otherwise = VisibleForTesting.NONE) + fun forceEnableLearnerStudyAnalytics(value: Boolean) { + enableLearnerStudyAnalytics = value + } + } } diff --git a/utility/src/main/java/org/oppia/android/util/platformparameter/PlatformParameterConstants.kt b/utility/src/main/java/org/oppia/android/util/platformparameter/PlatformParameterConstants.kt index 385a5a20227..fa238e6f328 100644 --- a/utility/src/main/java/org/oppia/android/util/platformparameter/PlatformParameterConstants.kt +++ b/utility/src/main/java/org/oppia/android/util/platformparameter/PlatformParameterConstants.kt @@ -93,6 +93,13 @@ const val CACHE_LATEX_RENDERING = "cache_latex_rendering" /** Default value for whether to cache LaTeX rendering using Glide. */ const val CACHE_LATEX_RENDERING_DEFAULT_VALUE = true +/** Qualifier for the feature flag corresponding to enabling the edit accounts options. */ +@Qualifier +annotation class EnableEditAccountsOptionsUi + +/** Default value for the feature flag corresponding to [EnableEditAccountsOptionsUi]. */ +const val ENABLE_EDIT_ACCOUNTS_OPTIONS_UI_DEFAULT_VALUE = true + /** Qualifier for the platform parameter that controls whether to record performance metrics. */ @Qualifier annotation class EnablePerformanceMetricsCollection From c3b42a4c573e21d368d76d18b3b0584e351340aa Mon Sep 17 00:00:00 2001 From: bhaktideshmukh <78796264+bhaktideshmukh@users.noreply.github.com> Date: Tue, 19 Jul 2022 23:03:15 +0900 Subject: [PATCH 03/57] Fixes part of #4195 : Added dark mode support to QuestionPlayer and Exploration (#4382) * added dark mode for input interaction * updated naming convention * added error_color * added hint color * changes cursor color * changed cursor color * updated cursor_color * resolving failing checks * resolving failing checks * added new hint color --- app/src/main/res/drawable/color_cursor.xml | 2 +- .../drawable/edit_text_background_border.xml | 4 ++-- .../edit_text_background_border_blue.xml | 4 ++-- .../res/drawable/ic_arrow_down_grey_24dp.xml | 1 + .../res/drawable/ic_arrow_right_grey_24dp.xml | 1 + .../drawable/submitted_answer_background.xml | 4 ++-- .../activity_input_interaction_view_test.xml | 14 ++++++------- .../res/layout/fraction_interaction_item.xml | 2 ++ .../math_expression_interactions_item.xml | 2 ++ .../layout/numeric_input_interaction_item.xml | 2 ++ .../layout/ratio_input_interaction_item.xml | 2 ++ .../main/res/layout/submitted_answer_item.xml | 2 +- .../layout/text_input_interaction_item.xml | 1 + .../main/res/values-night/color_palette.xml | 21 +++++++++++++++++++ app/src/main/res/values-v21/themes.xml | 1 + app/src/main/res/values/color_defs.xml | 14 ++++++++++++- app/src/main/res/values/color_palette.xml | 21 +++++++++++++++++++ app/src/main/res/values/component_colors.xml | 14 +++++++++++-- app/src/main/res/values/styles.xml | 3 ++- app/src/main/res/values/themes.xml | 1 + 20 files changed, 97 insertions(+), 19 deletions(-) diff --git a/app/src/main/res/drawable/color_cursor.xml b/app/src/main/res/drawable/color_cursor.xml index 2f93b37ddcf..4573a4b96a4 100644 --- a/app/src/main/res/drawable/color_cursor.xml +++ b/app/src/main/res/drawable/color_cursor.xml @@ -1,5 +1,5 @@ - + diff --git a/app/src/main/res/drawable/edit_text_background_border.xml b/app/src/main/res/drawable/edit_text_background_border.xml index 3dd64225973..4bc3a0fd077 100644 --- a/app/src/main/res/drawable/edit_text_background_border.xml +++ b/app/src/main/res/drawable/edit_text_background_border.xml @@ -2,8 +2,8 @@ - + + android:color="@color/component_color_shared_input_interaction_edit_text_not_selected_border_color" /> diff --git a/app/src/main/res/drawable/edit_text_background_border_blue.xml b/app/src/main/res/drawable/edit_text_background_border_blue.xml index 852ab5a8df3..8291c80cc18 100644 --- a/app/src/main/res/drawable/edit_text_background_border_blue.xml +++ b/app/src/main/res/drawable/edit_text_background_border_blue.xml @@ -2,8 +2,8 @@ - + + android:color="@color/component_color_shared_input_interaction_edit_text_border_color" /> diff --git a/app/src/main/res/drawable/ic_arrow_down_grey_24dp.xml b/app/src/main/res/drawable/ic_arrow_down_grey_24dp.xml index 59518af0471..d96f89eef1e 100644 --- a/app/src/main/res/drawable/ic_arrow_down_grey_24dp.xml +++ b/app/src/main/res/drawable/ic_arrow_down_grey_24dp.xml @@ -1,6 +1,7 @@ + android:color="@color/component_color_shared_submitted_answer_item_stroke_color"/> + android:color="@color/component_color_shared_submitted_answer_item_solid_color"/> diff --git a/app/src/main/res/layout/activity_input_interaction_view_test.xml b/app/src/main/res/layout/activity_input_interaction_view_test.xml index fa9b7cb3920..d4a4ce03572 100644 --- a/app/src/main/res/layout/activity_input_interaction_view_test.xml +++ b/app/src/main/res/layout/activity_input_interaction_view_test.xml @@ -57,8 +57,8 @@ android:paddingBottom="8dp" android:singleLine="true" android:text="@={fractionInteractionViewModel.answerText}" - android:textColor="@color/oppia_primary_text" - android:textColorHint="@color/edit_text_hint" + android:textColor="@color/component_color_shared_input_interaction_edit_text_color" + android:textColorHint="@color/component_color_shared_edit_text_hint_color" android:textSize="14sp" android:textStyle="italic" app:textChangedListener="@{fractionInteractionViewModel.answerTextWatcher}" /> @@ -73,7 +73,7 @@ android:fontFamily="sans-serif" android:minHeight="32dp" android:text="@{fractionInteractionViewModel.errorMessage}" - android:textColor="@color/edit_text_error" + android:textColor="@color/component_color_shared_input_error_color" android:textSize="12sp" android:visibility="@{fractionInteractionViewModel.errorMessage.length() > 0 ? View.VISIBLE : View.INVISIBLE}" /> @@ -94,8 +94,8 @@ android:paddingBottom="8dp" android:singleLine="true" android:text="@={ratioInteractionInputViewModel.answerText}" - android:textColor="@color/oppia_primary_text" - android:textColorHint="@color/edit_text_hint" + android:textColor="@color/component_color_shared_input_interaction_edit_text_color" + android:textColorHint="@color/component_color_shared_edit_text_hint_color" android:textSize="14sp" android:textStyle="italic" app:textChangedListener="@{ratioInteractionInputViewModel.answerTextWatcher}" /> @@ -110,7 +110,7 @@ android:fontFamily="sans-serif" android:minHeight="32dp" android:text="@{ratioInteractionInputViewModel.errorMessage}" - android:textColor="@color/edit_text_error" + android:textColor="@color/component_color_shared_input_error_color" android:textSize="12sp" android:visibility="@{ratioInteractionInputViewModel.errorMessage.length() > 0 ? View.VISIBLE : View.INVISIBLE}" /> @@ -140,7 +140,7 @@ android:fontFamily="sans-serif" android:minHeight="32dp" android:text="@{numericInputViewModel.errorMessage}" - android:textColor="@color/edit_text_error" + android:textColor="@color/component_color_shared_input_error_color" android:textSize="12sp" android:visibility="@{numericInputViewModel.errorMessage.length() > 0 ? View.VISIBLE : View.INVISIBLE}" /> diff --git a/app/src/main/res/layout/fraction_interaction_item.xml b/app/src/main/res/layout/fraction_interaction_item.xml index f361f591c1a..4ac40582241 100644 --- a/app/src/main/res/layout/fraction_interaction_item.xml +++ b/app/src/main/res/layout/fraction_interaction_item.xml @@ -26,6 +26,7 @@ android:hint="@{viewModel.hintText}" android:inputType="text" android:text="@={viewModel.answerText}" + android:textColorHint="@color/component_color_shared_edit_text_hint_color" app:textChangedListener="@{viewModel.answerTextWatcher}" app:layout_constraintEnd_toEndOf="parent" app:layout_constraintStart_toStartOf="parent" @@ -35,6 +36,7 @@ android:id="@+id/fraction_input_error" style="@style/InputInteractionErrorTextView" android:text="@{viewModel.errorMessage}" + android:textColor="@color/component_color_shared_input_error_color" app:layout_constraintEnd_toEndOf="parent" app:layout_constraintStart_toStartOf="parent" app:layout_constraintTop_toBottomOf="@+id/fraction_input_interaction_view" diff --git a/app/src/main/res/layout/math_expression_interactions_item.xml b/app/src/main/res/layout/math_expression_interactions_item.xml index 23bc09f680a..6676f721cc8 100644 --- a/app/src/main/res/layout/math_expression_interactions_item.xml +++ b/app/src/main/res/layout/math_expression_interactions_item.xml @@ -26,6 +26,7 @@ app:placeholder="@{viewModel.hintText}" android:inputType="text" android:text="@={viewModel.answerText}" + android:textColorHint="@color/component_color_shared_edit_text_hint_color" app:textChangedListener="@{viewModel.answerTextWatcher}" app:layout_constraintEnd_toEndOf="parent" app:layout_constraintStart_toStartOf="parent" @@ -36,6 +37,7 @@ android:id="@+id/math_expression_input_error" style="@style/InputInteractionErrorTextView" android:text="@{viewModel.errorMessage}" + android:textColor="@color/component_color_shared_input_error_color" app:layout_constraintEnd_toEndOf="parent" app:layout_constraintStart_toStartOf="parent" app:layout_constraintTop_toBottomOf="@+id/math_expression_input_interaction_view" diff --git a/app/src/main/res/layout/numeric_input_interaction_item.xml b/app/src/main/res/layout/numeric_input_interaction_item.xml index 542881cbedf..bea2d287176 100644 --- a/app/src/main/res/layout/numeric_input_interaction_item.xml +++ b/app/src/main/res/layout/numeric_input_interaction_item.xml @@ -27,6 +27,7 @@ android:hint="@string/numeric_input_hint" android:inputType="numberDecimal" android:text="@={viewModel.answerText}" + android:textColorHint="@color/component_color_shared_edit_text_hint_color" app:layout_constraintEnd_toEndOf="parent" app:layout_constraintStart_toStartOf="parent" app:layout_constraintTop_toTopOf="parent" @@ -35,6 +36,7 @@ @color/color_def_white @color/color_def_oppia_light_black @color/color_def_accessible_light_grey + @color/color_def_white @color/color_def_oppia_light_black @color/color_def_accessible_grey @color/color_def_white @@ -33,6 +34,12 @@ @color/color_def_oppia_light_black @color/color_def_oppia_grayish_black @color/color_def_dark_blue + @color/color_def_oppia_pink + @color/color_def_oppia_dark_grey + @color/color_def_white + @color/color_def_white + @color/color_def_white + @color/color_def_white @color/color_def_oppia_grayish_black @color/color_def_white @color/color_def_white @@ -45,6 +52,20 @@ @color/color_def_oppia_pink @color/color_def_forest_green @color/color_def_forest_green + @color/color_def_white + @color/color_def_oppia_silver + @color/color_def_oppia_turquoise + @color/color_def_oppia_dark_grey + @color/color_def_maastricht_blue + @color/color_def_forest_green + @color/color_def_oppia_light_black + @color/color_def_dark_pink + @color/color_def_oppia_pink + @color/color_def_persian_blue + @color/color_def_bright_green + @color/color_def_white + @color/color_def_oppia_metallic_blue + @color/color_def_dark_purple @color/color_def_black_grey @color/color_def_oppia_turquoise @color/color_def_dark_pink diff --git a/app/src/main/res/values-v21/themes.xml b/app/src/main/res/values-v21/themes.xml index 6836b964a9e..2fcb8536985 100644 --- a/app/src/main/res/values-v21/themes.xml +++ b/app/src/main/res/values-v21/themes.xml @@ -15,6 +15,7 @@ @color/color_palette_accent_color false true + @style/InputInteractionEditText @style/Animation.Activity @color/component_color_shared_activity_status_bar_color diff --git a/app/src/main/res/values/color_defs.xml b/app/src/main/res/values/color_defs.xml index ea5c596b144..078786856cd 100644 --- a/app/src/main/res/values/color_defs.xml +++ b/app/src/main/res/values/color_defs.xml @@ -54,12 +54,24 @@ #395FD0 #DDDDDD #707070 + #923026 #FFFFF0 #999999 #02655c + #666666 + #923026 + #0e162f + #081661 + #8A3D69 + #00609C + #009C8A + #674172 + #7659B6 + #2F6687 + #A6503A + #441530 #923026 #33999999 - #8A3D69 #ACFFF8 #61999999 #00E4F2F1 diff --git a/app/src/main/res/values/color_palette.xml b/app/src/main/res/values/color_palette.xml index 492407e7d56..5db583630ea 100644 --- a/app/src/main/res/values/color_palette.xml +++ b/app/src/main/res/values/color_palette.xml @@ -20,6 +20,7 @@ @color/color_def_accessible_light_grey_2 @color/color_def_white @color/color_def_spanish_grey + @color/color_def_spanish_grey @color/color_def_bright_grey @color/color_def_white @color/color_def_black @@ -33,6 +34,12 @@ @color/color_def_oppia_light_green @color/color_def_oppia_solid_blue @color/color_def_oppia_stroke_blue + @color/color_def_input_error + @color/color_def_white + @color/color_def_black + @color/color_def_oppia_dark_blue + @color/color_def_mid_grey + @color/color_def_mid_grey @color/color_def_white @color/color_def_oppia_dark_blue @color/color_def_oppia_dark_blue @@ -45,6 +52,20 @@ @color/color_def_error_text @color/color_def_accessible_grey @color/color_def_accessible_grey + @color/color_def_oppia_dark_grey_border + @color/color_def_oppia_dark_blue + @color/color_def_white + @color/color_def_oppia_dark_blue + @color/color_def_ocean_blue + @color/color_def_oppia_green + @color/color_def_oppia_light_green + @color/color_def_oppia_brown_dark + @color/color_def_oppia_brown_dark + @color/color_def_persian_blue + @color/color_def_oppia_light_brown + @color/color_def_black + @color/color_def_teal_blue + @color/color_def_dark_red @color/color_def_black_grey @color/color_def_oppia_dark_blue @color/color_def_oppia_brown_dark diff --git a/app/src/main/res/values/component_colors.xml b/app/src/main/res/values/component_colors.xml index 71baadc2772..0e5a42c1e79 100644 --- a/app/src/main/res/values/component_colors.xml +++ b/app/src/main/res/values/component_colors.xml @@ -1,12 +1,22 @@ - @color/color_palette_primary_text_color + @color/color_palette_cursor_color @color/color_palette_toolbar_color @color/color_palette_highlighted_text_color @color/color_palette_highlighted_text_color @color/color_palette_exploration_background_color @color/color_palette_highlighted_text_color @color/color_palette_highlighted_text_color + @color/color_palette_highlighted_text_color + @color/color_palette_input_error_color + @color/color_palette_submitted_answer_item_solid_color + @color/color_palette_submitted_answer_item_stroke_color + @color/color_palette_highlighted_text_color + @color/color_palette_hint_text_input_interaction_color + @color/color_palette_input_interaction_edit_text_border_color + @color/color_palette_input_interaction_edit_text_not_selected_border_color + @color/color_palette_arrow_down_grey_color + @color/color_palette_arrow_right_grey_color @color/color_palette_rounded_rect_background_color @color/color_palette_multiple_choice_color @color/color_palette_multiple_choice_text_color @@ -40,7 +50,7 @@ @color/color_palette_primary_text_color @color/color_palette_primary_text_color @color/color_palette_primary_text_color - @color/color_palette_text_input_background_color + @color/color_palette_text_input_background_color @color/color_palette_error_color @color/color_palette_container_background_color @color/color_palette_dark_background_color diff --git a/app/src/main/res/values/styles.xml b/app/src/main/res/values/styles.xml index d7237b66bd2..06b21244d8e 100644 --- a/app/src/main/res/values/styles.xml +++ b/app/src/main/res/values/styles.xml @@ -164,7 +164,7 @@ From ee8fdc694f7ed94925f7b72208a80340719c355a Mon Sep 17 00:00:00 2001 From: Sarthak Agarwal Date: Tue, 26 Jul 2022 12:40:10 +0530 Subject: [PATCH 04/57] Fixes #4328: Proto definitions for performance metrics collection (#4336) * qualifiers and constants for metric record and upload times * comments * dagger provides for flags * rename to enablePerformanceMetricCollection * initial proto * nit fixes * nit fixes * comments. * nits * nit * updates. * updates. * nits. * metric log inclusion. * name correction. * nits. * storage comment * pss comment * network usage comment. * network usage comment - part 2. * metric addition in proto definitions. * metricLog --> loggableMetric * wording update for transmission * memory and storage tier updates --- .../src/main/proto/performance_metrics.proto | 187 ++++++++++++++++++ 1 file changed, 187 insertions(+) create mode 100644 model/src/main/proto/performance_metrics.proto diff --git a/model/src/main/proto/performance_metrics.proto b/model/src/main/proto/performance_metrics.proto new file mode 100644 index 00000000000..a6019cfb772 --- /dev/null +++ b/model/src/main/proto/performance_metrics.proto @@ -0,0 +1,187 @@ +syntax = "proto3"; + +package model; + +option java_package = "org.oppia.android.app.model"; +option java_multiple_files = true; + +// Structure for an oppia metric log. +message OppiaMetricLog { + // The UTC timestamp of the event in long/millis. + int64 timestamp_millis = 1; + + // The priority of the event. + Priority priority = 2; + + // Indicates whether the application is in the foreground. + bool is_app_in_foreground = 3; + + // The memory tier of the current device. + MemoryTier memory_tier = 4; + + // The storage tier of the current device. + StorageTier storage_tier = 5; + + // The current network type of the device. + NetworkType network_type = 6; + + // The metric log of the event. + LoggableMetric loggable_metric = 7; + + // The current screen where the event is getting logged. + CurrentScreen current_screen = 8; + + // Structure of a loggable metric. + message LoggableMetric { + oneof loggable_metric_type { + // The metric being logged is related to the apk size of the application. + ApkSizeMetric apk_size_metric = 1; + + // The metric being logged is related to the storage usage of the application. + StorageUsageMetric storage_usage_metric = 2; + + // The metric being logged is related to the startup latency of the application. + StartupLatencyMetric startup_latency_metric = 3; + + // The metric being logged is related to the memory usage of the application. + MemoryUsageMetric memory_usage_metric = 4; + + // The metric being logged is related to the network usage of the application. + NetworkUsageMetric network_usage_metric = 5; + + // The metric being logged is related to the cpu usage of the application. + CpuUsageMetric cpu_usage_metric = 6; + } + } + + // Structure of the apk size metric. + message ApkSizeMetric { + // Size of the app's installed APK file, in bytes. + int64 apk_size_bytes = 1; + } + + // Structure of the storage usage metric. + message StorageUsageMetric { + // Amount of storage usage by the app on user's device in bytes. This storage size is the cumulative + // size of app-specific files which include the application cache but not the apk size. + int64 storage_usage_bytes = 1; + } + + // Structure of the startup latency metric. + message StartupLatencyMetric { + // Number of milliseconds required to start up the application from a cold start. + int64 startup_latency_millis = 1; + } + + // Structure of the memory usage metric. + message MemoryUsageMetric { + // Amount of memory used by the application on the device in bytes. + // Here, PSS refers to Proportional Set Size which is the number of non-shared pages used by the + // app. It is majorly useful when we want to know how much memory is used by all processes since + // processes don't get counted multiple times. For more information, refer to the following link: + // https://developer.android.com/topic/performance/memory-management#calculating_memory_footprint + int64 total_pss_bytes = 1; + } + + // Structure of the network usage metric. + message NetworkUsageMetric { + // Usage data is collected in discrete bins of time called 'Buckets'. The time interval of this + // bucket is customizable and depends on the query that we are making. This data is logged after + // the bucket's timeframe. + + // This field denotes the number of bytes received by the application on the device during + // this bucket's time interval. + int64 bytes_received = 1; + + // This field denotes the number of bytes sent by the application on the device during + // this bucket's time interval. + int64 bytes_sent = 2; + } + + // Structure of the cpu usage metric. + message CpuUsageMetric { + // Amount of cpu used by the application. + // TODO(#4466): Add correct proto definitions and comments for cpu usage metric logging. + int64 cpu_usage_metric = 1; + } + + // Supported priority of events for performance metric logging. + enum Priority { + // The undefined priority of an event. + PRIORITY_UNSPECIFIED = 0; + + // The priority of events whose logs will be the first to be removed from the storage if the + // size exceeds a certain limit + LOW_PRIORITY = 1; + + // The priority of events whose logs will be the ones to be removed from the storage if the + // size exceeds a certain limit and no low priority logs exist. + MEDIUM_PRIORITY = 2; + + // The priority of events whose logs should not be removed from the storage on most occasions + // but can be removed if they're the only ones there and size limit exceeds. + HIGH_PRIORITY = 3; + } + + // Supported screens for indicating the current screen where the event logging is occurring. + enum CurrentScreen { + // The undefined current screen of an event. + SCREEN_UNSPECIFIED = 0; + + // The metric log was taken when home screen was opened. + HOME_SCREEN = 1; + // TODO(#4340): Add support for all screens which are going to be used for metric logging. + } + + // Supported memory tiers for indicating the total memory of user's device. + enum MemoryTier { + // The undefined memory tier of a device. + MEMORY_TIER_UNSPECIFIED = 0; + + // The memory tier where the amount of memory is less than 2GB. + LOW_MEMORY_TIER = 1; // <2GB + + // The memory tier where the amount of memory is less than or equal to 3GB and more than or + // equal to 2GB. + MEDIUM_MEMORY_TIER = 2; // =<3GB && >2GB + + // The memory tier where the amount of memory is greater than 3GB. + HIGH_MEMORY_TIER = 3; // >3GB + } + + // Supported storage tiers for indicating the total storage space on user's device. + enum StorageTier { + // The undefined storage tier of a device. + STORAGE_TIER_UNSPECIFIED = 0; + + // The storage tier where the amount of storage is less than 32GB. + LOW_STORAGE = 1; // <32GB + + // The storage tier where the amount of storage is less than or equal to 64GB and more than or + // equal to 32GB. + MEDIUM_STORAGE = 2; // =<64GB && >=32GB + + // The storage tier where the amount of storage is greater than 64GB. + HIGH_STORAGE = 3; // >64GB + } + + // Supported network types for indicating the current network situation on user's device. + enum NetworkType { + // The undefined network type of a device. + NETWORK_UNSPECIFIED = 0; + + // Indicates that the device is not connected to any network. + NONE = 1; + + // Indicates that the device is connected to wifi or ethernet. + WIFI = 2; + + // Indicates that the device is connected to mobile or WiMax. + CELLULAR = 3; + } +} + +// Structure for a container that contains metric log reports. +message OppiaMetricLogs { + repeated OppiaMetricLog oppia_metric_log = 1; +} From 8dd171649b8813556e8ec09603d9c192a77750de Mon Sep 17 00:00:00 2001 From: Jishnu <64526117+JishnuGoyal@users.noreply.github.com> Date: Wed, 27 Jul 2022 07:29:29 +0530 Subject: [PATCH 05/57] Fix #4438: Add profile login counter (#4426) * add number_of_logins field to profile proto * add increment login count function * add tests * init number of logins to 0 when new profile is added * shorten test name for lint issue * shorten test name for lint issue * use suggested approach for login count increment * use suggested approach for login count increment + lint fix * fix test name and logic as suggested * modify test to check for login count = 2 after 2 logins * nit * lint * rename test * nit - add full stop --- .../profile/ProfileManagementController.kt | 33 +++++++++++-------- .../ProfileManagementControllerTest.kt | 23 +++++++++++-- model/src/main/proto/profile.proto | 3 ++ 3 files changed, 43 insertions(+), 16 deletions(-) diff --git a/domain/src/main/java/org/oppia/android/domain/profile/ProfileManagementController.kt b/domain/src/main/java/org/oppia/android/domain/profile/ProfileManagementController.kt index d7ec152f38e..ea47447f48c 100644 --- a/domain/src/main/java/org/oppia/android/domain/profile/ProfileManagementController.kt +++ b/domain/src/main/java/org/oppia/android/domain/profile/ProfileManagementController.kt @@ -231,6 +231,7 @@ class ProfileManagementController @Inject constructor( readingTextSize = ReadingTextSize.MEDIUM_TEXT_SIZE appLanguage = AppLanguage.ENGLISH_APP_LANGUAGE audioLanguage = AudioLanguage.ENGLISH_AUDIO_LANGUAGE + numberOfLogins = 0 if (learnerStudyAnalytics.value) { // Only set a learner ID if there's an ongoing user study. @@ -607,7 +608,8 @@ class ProfileManagementController @Inject constructor( } /** - * Log in to the user's Profile by setting the current profile Id and updating profile's last logged in time. + * Log in to the user's Profile by setting the current profile Id, updating profile's last logged in + * time and updating the total number of logins for the current profile Id. * * @param profileId the ID corresponding to the profile being logged into. * @return a [DataProvider] that indicates the success/failure of this login operation. @@ -617,7 +619,7 @@ class ProfileManagementController @Inject constructor( return@transformAsync getDeferredResult( profileId, null, - updateLastLoggedInAsync(profileId) + updateLastLoggedInAsyncAndNumberOfLogins(profileId) ) } } @@ -638,19 +640,22 @@ class ProfileManagementController @Inject constructor( } } - private fun updateLastLoggedInAsync(profileId: ProfileId): Deferred { - return profileDataStore.storeDataWithCustomChannelAsync(updateInMemoryCache = true) { - val profile = it.profilesMap[profileId.internalId] - ?: return@storeDataWithCustomChannelAsync Pair(it, ProfileActionStatus.PROFILE_NOT_FOUND) - val updatedProfile = - profile.toBuilder().setLastLoggedInTimestampMs(oppiaClock.getCurrentTimeMs()).build() - val profileDatabaseBuilder = it.toBuilder().putProfiles( - profileId.internalId, - updatedProfile - ) - Pair(profileDatabaseBuilder.build(), ProfileActionStatus.SUCCESS) + private fun updateLastLoggedInAsyncAndNumberOfLogins(profileId: ProfileId): + Deferred { + return profileDataStore.storeDataWithCustomChannelAsync(updateInMemoryCache = true) { + val profile = it.profilesMap[profileId.internalId] + ?: return@storeDataWithCustomChannelAsync Pair(it, ProfileActionStatus.PROFILE_NOT_FOUND) + val updatedProfile = profile.toBuilder() + .setLastLoggedInTimestampMs(oppiaClock.getCurrentTimeMs()) + .setNumberOfLogins(profile.numberOfLogins + 1) + .build() + val profileDatabaseBuilder = it.toBuilder().putProfiles( + profileId.internalId, + updatedProfile + ) + Pair(profileDatabaseBuilder.build(), ProfileActionStatus.SUCCESS) + } } - } /** * Deletes an existing profile. diff --git a/domain/src/test/java/org/oppia/android/domain/profile/ProfileManagementControllerTest.kt b/domain/src/test/java/org/oppia/android/domain/profile/ProfileManagementControllerTest.kt index 1239147acc7..9bc31adf13d 100644 --- a/domain/src/test/java/org/oppia/android/domain/profile/ProfileManagementControllerTest.kt +++ b/domain/src/test/java/org/oppia/android/domain/profile/ProfileManagementControllerTest.kt @@ -59,7 +59,6 @@ import org.robolectric.annotation.Config import org.robolectric.annotation.LooperMode import java.io.File import java.io.FileInputStream -import java.lang.IllegalStateException import javax.inject.Inject import javax.inject.Singleton @@ -121,6 +120,7 @@ class ProfileManagementControllerTest { assertThat(profile.readingTextSize).isEqualTo(MEDIUM_TEXT_SIZE) assertThat(profile.appLanguage).isEqualTo(AppLanguage.ENGLISH_APP_LANGUAGE) assertThat(profile.audioLanguage).isEqualTo(AudioLanguage.ENGLISH_AUDIO_LANGUAGE) + assertThat(profile.numberOfLogins).isEqualTo(0) assertThat(File(getAbsoluteDirPath("0")).isDirectory).isTrue() } @@ -521,7 +521,7 @@ class ProfileManagementControllerTest { } @Test - fun testLoginToProfile_addProfiles_loginToProfile_checkGetProfileIdAndLoginTimestampIsCorrect() { + fun testLoginProfile_addedProfile_profileIdTimestampAndNumberOfLoginsIsCorrectlyUpdated() { setUpTestApplicationComponent() addTestProfiles() @@ -532,6 +532,25 @@ class ProfileManagementControllerTest { val profile = monitorFactory.waitForNextSuccessfulResult(profileProvider) assertThat(profileManagementController.getCurrentProfileId().internalId).isEqualTo(2) assertThat(profile.lastLoggedInTimestampMs).isNotEqualTo(0) + assertThat(profile.numberOfLogins).isEqualTo(1) + } + + @Test + fun testLoginToProfile_addProfile_loginToProfileTwice_checkNumberOfLoginsIsTwo() { + setUpTestApplicationComponent() + addTestProfiles() + var loginProvider = profileManagementController.loginToProfile(PROFILE_ID_2) + monitorFactory.waitForNextSuccessfulResult(loginProvider) + + // log out of profile 2 + loginProvider = profileManagementController.loginToProfile(PROFILE_ID_3) + monitorFactory.waitForNextSuccessfulResult(loginProvider) + + loginProvider = profileManagementController.loginToProfile(PROFILE_ID_2) + monitorFactory.waitForNextSuccessfulResult(loginProvider) + val profileProvider = profileManagementController.getProfile(PROFILE_ID_2) + val profile = monitorFactory.waitForNextSuccessfulResult(profileProvider) + assertThat(profile.numberOfLogins).isEqualTo(2) } @Test diff --git a/model/src/main/proto/profile.proto b/model/src/main/proto/profile.proto index 6285450ede5..535e1103fb1 100644 --- a/model/src/main/proto/profile.proto +++ b/model/src/main/proto/profile.proto @@ -70,6 +70,9 @@ message Profile { // are enabled as part of a user study. In non-user study cases this is expected to either be // empty or a meaningless value. string learner_id = 13; + + // Represents the number of times a user has successfully logged into their profile. + int32 number_of_logins = 14; } // Represents a profile avatar image. From f446fe8ab94de43187cf19895be683882ccfcf3f Mon Sep 17 00:00:00 2001 From: bhaktideshmukh <78796264+bhaktideshmukh@users.noreply.github.com> Date: Thu, 28 Jul 2022 05:03:22 +0900 Subject: [PATCH 06/57] Fixes #4192 : Added dark mode support to MarkChaptersCompletedActivity, MarkStoriesCompletedActivity and MarkTopicsCompletedActivity (#4442) * dark_mode * added mark_completed_text color --- .../main/res/color/checkbox_text_color.xml | 6 ++--- .../res/drawable/ic_check_box_checked.xml | 2 +- .../mark_chapters_completed_fragment.xml | 10 ++++---- ..._chapters_completed_story_summary_view.xml | 4 ++-- .../mark_stories_completed_fragment.xml | 10 ++++---- ...k_stories_completed_story_summary_view.xml | 2 +- .../layout/mark_topics_completed_fragment.xml | 10 ++++---- .../mark_topics_completed_topic_view.xml | 2 +- .../main/res/values-night/color_palette.xml | 6 +++++ app/src/main/res/values/color_defs.xml | 2 ++ app/src/main/res/values/color_palette.xml | 6 +++++ app/src/main/res/values/component_colors.xml | 23 +++++++++++++++++++ 12 files changed, 60 insertions(+), 23 deletions(-) diff --git a/app/src/main/res/color/checkbox_text_color.xml b/app/src/main/res/color/checkbox_text_color.xml index dff1ece24e9..0224c0f546d 100644 --- a/app/src/main/res/color/checkbox_text_color.xml +++ b/app/src/main/res/color/checkbox_text_color.xml @@ -1,12 +1,12 @@ - - - diff --git a/app/src/main/res/drawable/ic_check_box_checked.xml b/app/src/main/res/drawable/ic_check_box_checked.xml index 85ca608c033..f18efa940af 100644 --- a/app/src/main/res/drawable/ic_check_box_checked.xml +++ b/app/src/main/res/drawable/ic_check_box_checked.xml @@ -1,4 +1,4 @@ - diff --git a/app/src/main/res/layout/mark_chapters_completed_fragment.xml b/app/src/main/res/layout/mark_chapters_completed_fragment.xml index 3389d5e652b..e7bbd5fe6fb 100644 --- a/app/src/main/res/layout/mark_chapters_completed_fragment.xml +++ b/app/src/main/res/layout/mark_chapters_completed_fragment.xml @@ -32,7 +32,7 @@ android:id="@+id/mark_chapters_completed_toolbar" android:layout_width="match_parent" android:layout_height="?attr/actionBarSize" - android:background="@color/oppia_primary" + android:background="@color/component_color_mark_chapters_completed_primary_toolbar_color" android:textSize="20sp" app:navigationContentDescription="@string/navigate_up" app:navigationIcon="?attr/homeAsUpIndicator" @@ -44,7 +44,7 @@ + android:background="@color/component_color_mark_chapters_completed_container_background_color"> @@ -93,7 +93,7 @@ android:minWidth="48dp" android:minHeight="48dp" android:text="@string/modify_lesson_progress_mark_completed_uppercase" - android:textColor="@color/color_def_white" + android:textColor="@color/component_color_mark_chapters_completed_text_mark_completed_color" app:layout_constraintBottom_toBottomOf="parent" app:layout_constraintEnd_toEndOf="parent" app:layout_constraintTop_toTopOf="parent" /> @@ -112,7 +112,7 @@ android:id="@+id/mark_chapters_completed_recycler_view" android:layout_width="match_parent" android:layout_height="match_parent" - android:background="@color/profile_edit_background" + android:background="@color/component_color_mark_chapters_completed_background_color" android:clipToPadding="false" android:overScrollMode="never" android:paddingBottom="40dp" diff --git a/app/src/main/res/layout/mark_chapters_completed_story_summary_view.xml b/app/src/main/res/layout/mark_chapters_completed_story_summary_view.xml index fa69fa12717..186998e2ba1 100644 --- a/app/src/main/res/layout/mark_chapters_completed_story_summary_view.xml +++ b/app/src/main/res/layout/mark_chapters_completed_story_summary_view.xml @@ -18,13 +18,13 @@ android:id="@+id/mark_chapters_completed_story_name_text_view" style="@style/Subtitle1ViewStart" android:layout_width="match_parent" - android:background="@color/color_def_white" + android:background="@color/component_color_mark_chapters_completed_subtext_heading_background_color" android:paddingStart="16dp" android:paddingTop="20dp" android:paddingEnd="16dp" android:paddingBottom="20dp" android:text="@{viewModel.storyName}" - android:textColor="@color/oppia_primary_text_dark" + android:textColor="@color/component_color_mark_chapters_completed_subtext_heading_color" app:layout_constraintEnd_toEndOf="parent" app:layout_constraintStart_toStartOf="parent" app:layout_constraintTop_toTopOf="parent" /> diff --git a/app/src/main/res/layout/mark_stories_completed_fragment.xml b/app/src/main/res/layout/mark_stories_completed_fragment.xml index d657f17009d..1bae6f2cfff 100644 --- a/app/src/main/res/layout/mark_stories_completed_fragment.xml +++ b/app/src/main/res/layout/mark_stories_completed_fragment.xml @@ -32,7 +32,7 @@ android:id="@+id/mark_stories_completed_toolbar" android:layout_width="match_parent" android:layout_height="?attr/actionBarSize" - android:background="@color/oppia_primary" + android:background="@color/component_color_mark_stories_completed_primary_toolbar_color" android:textSize="20sp" app:navigationContentDescription="@string/navigate_up" app:navigationIcon="?attr/homeAsUpIndicator" @@ -44,7 +44,7 @@ + android:background="@color/component_color_mark_stories_completed_container_background_color"> @@ -93,7 +93,7 @@ android:minWidth="48dp" android:minHeight="48dp" android:text="@string/modify_lesson_progress_mark_completed_uppercase" - android:textColor="@color/color_def_white" + android:textColor="@color/component_color_mark_stories_completed_text_mark_completed_color" app:layout_constraintBottom_toBottomOf="parent" app:layout_constraintEnd_toEndOf="parent" app:layout_constraintTop_toTopOf="parent" /> @@ -112,7 +112,7 @@ android:id="@+id/mark_stories_completed_recycler_view" android:layout_width="match_parent" android:layout_height="match_parent" - android:background="@color/profile_edit_background" + android:background="@color/component_color_mark_stories_completed_background_color" android:clipToPadding="false" android:overScrollMode="never" android:paddingVertical="40dp" diff --git a/app/src/main/res/layout/mark_stories_completed_story_summary_view.xml b/app/src/main/res/layout/mark_stories_completed_story_summary_view.xml index 94b41e448d3..e9205d0c07f 100644 --- a/app/src/main/res/layout/mark_stories_completed_story_summary_view.xml +++ b/app/src/main/res/layout/mark_stories_completed_story_summary_view.xml @@ -33,7 +33,7 @@ android:paddingBottom="26dp" android:text="@{viewModel.storySummary.storyName}" android:textAlignment="viewStart" - android:textColor="@color/oppia_primary_text" + android:textColor="@color/checkbox_text_color" android:textSize="16sp" app:layout_constraintBottom_toBottomOf="parent" app:layout_constraintEnd_toEndOf="parent" diff --git a/app/src/main/res/layout/mark_topics_completed_fragment.xml b/app/src/main/res/layout/mark_topics_completed_fragment.xml index 01ec032cc88..00ea5dda2ba 100644 --- a/app/src/main/res/layout/mark_topics_completed_fragment.xml +++ b/app/src/main/res/layout/mark_topics_completed_fragment.xml @@ -32,7 +32,7 @@ android:id="@+id/mark_topics_completed_toolbar" android:layout_width="match_parent" android:layout_height="?attr/actionBarSize" - android:background="@color/oppia_primary" + android:background="@color/component_color_mark_topics_completed_primary_toolbar_color" android:textSize="20sp" app:navigationContentDescription="@string/navigate_up" app:navigationIcon="?attr/homeAsUpIndicator" @@ -44,7 +44,7 @@ + android:background="@color/component_color_mark_topics_completed_container_background_color"> @@ -93,7 +93,7 @@ android:minWidth="48dp" android:minHeight="48dp" android:text="@string/modify_lesson_progress_mark_completed_uppercase" - android:textColor="@color/color_def_white" + android:textColor="@color/component_color_mark_topics_completed_text_mark_completed_color" app:layout_constraintBottom_toBottomOf="parent" app:layout_constraintEnd_toEndOf="parent" app:layout_constraintTop_toTopOf="parent" /> @@ -112,7 +112,7 @@ android:id="@+id/mark_topics_completed_recycler_view" android:layout_width="match_parent" android:layout_height="match_parent" - android:background="@color/profile_edit_background" + android:background="@color/component_color_mark_topics_completed_background_color" android:clipToPadding="false" android:overScrollMode="never" android:paddingVertical="40dp" diff --git a/app/src/main/res/layout/mark_topics_completed_topic_view.xml b/app/src/main/res/layout/mark_topics_completed_topic_view.xml index 846260a5647..15eb7c66fbf 100644 --- a/app/src/main/res/layout/mark_topics_completed_topic_view.xml +++ b/app/src/main/res/layout/mark_topics_completed_topic_view.xml @@ -33,7 +33,7 @@ android:paddingBottom="26dp" android:text="@{viewModel.topic.name}" android:textAlignment="viewStart" - android:textColor="@color/oppia_primary_text" + android:textColor="@color/checkbox_text_color" android:textSize="16sp" app:layout_constraintBottom_toBottomOf="parent" app:layout_constraintEnd_toEndOf="parent" diff --git a/app/src/main/res/values-night/color_palette.xml b/app/src/main/res/values-night/color_palette.xml index 8778915452d..822352d053f 100644 --- a/app/src/main/res/values-night/color_palette.xml +++ b/app/src/main/res/values-night/color_palette.xml @@ -82,4 +82,10 @@ @color/color_def_dark_mode_question_player_progress_bar_gradient_center @color/color_def_dark_mode_question_player_progress_bar_gradient_end @color/color_def_white + @color/color_def_oppia_light_black + @color/color_def_white + @color/color_def_oppia_turquoise + @color/color_def_oppia_turquoise + @color/color_def_white + @color/color_def_white diff --git a/app/src/main/res/values/color_defs.xml b/app/src/main/res/values/color_defs.xml index 078786856cd..3f420995410 100644 --- a/app/src/main/res/values/color_defs.xml +++ b/app/src/main/res/values/color_defs.xml @@ -80,4 +80,6 @@ #23272A00 #23272A #00C7B6 + #E5E5E5 + #008577 diff --git a/app/src/main/res/values/color_palette.xml b/app/src/main/res/values/color_palette.xml index 5db583630ea..c97e06cfef9 100644 --- a/app/src/main/res/values/color_palette.xml +++ b/app/src/main/res/values/color_palette.xml @@ -82,4 +82,10 @@ @color/color_def_question_player_progress_bar_gradient_center @color/color_def_question_player_progress_bar_gradient_end @color/color_def_white + @color/color_def_whitish_grey + @color/color_def_accessible_grey + @color/color_def_accessible_grey + @color/color_def_checkbox_green + @color/color_def_white + @color/color_def_white diff --git a/app/src/main/res/values/component_colors.xml b/app/src/main/res/values/component_colors.xml index 0e5a42c1e79..638e760ae1a 100644 --- a/app/src/main/res/values/component_colors.xml +++ b/app/src/main/res/values/component_colors.xml @@ -186,4 +186,27 @@ @color/color_palette_dark_text_color @color/color_palette_dark_text_color @color/color_palette_hint_text_color + + @color/color_palette_toolbar_color + @color/color_palette_secondary_toolbar_color + @color/color_palette_mark_chapters_background_color + @color/color_palette_checkbox_text_unselected_state_color + @color/color_palette_checkbox_text_selected_state_color + @color/color_palette_checkbox_selected_state_color + @color/color_palette_container_background_color + @color/color_palette_highlighted_text_color + @color/color_palette_mark_completed_text_color + @color/color_palette_mark_completed_all_text_color + + @color/color_palette_toolbar_color + @color/color_palette_secondary_toolbar_color + @color/color_palette_mark_chapters_background_color + @color/color_palette_mark_completed_text_color + @color/color_palette_mark_completed_all_text_color + + @color/color_palette_toolbar_color + @color/color_palette_secondary_toolbar_color + @color/color_palette_mark_chapters_background_color + @color/color_palette_mark_completed_text_color + @color/color_palette_mark_completed_all_text_color From 3496373d2342cad53b19067ff057f43c7758460b Mon Sep 17 00:00:00 2001 From: Jishnu <64526117+JishnuGoyal@users.noreply.github.com> Date: Thu, 28 Jul 2022 12:18:44 +0530 Subject: [PATCH 07/57] Fix #4439: Change speaker icon to headset icon (#4429) * change speaker icon to headset icon * add headset-off icon and delete old icons * use hex value instead of resource * use hex value instead of resource because CI was failing * refactor name of the drawable resource to match others * reformat --- .../main/res/drawable/ic_audio_streaming_off_24dp.xml | 8 -------- .../main/res/drawable/ic_audio_streaming_on_24dp.xml | 8 -------- app/src/main/res/drawable/ic_baseline_headset_24.xml | 11 +++++++++++ .../main/res/drawable/ic_baseline_headset_off_24.xml | 10 ++++++++++ app/src/main/res/layout/exploration_activity.xml | 2 +- 5 files changed, 22 insertions(+), 17 deletions(-) delete mode 100644 app/src/main/res/drawable/ic_audio_streaming_off_24dp.xml delete mode 100644 app/src/main/res/drawable/ic_audio_streaming_on_24dp.xml create mode 100644 app/src/main/res/drawable/ic_baseline_headset_24.xml create mode 100644 app/src/main/res/drawable/ic_baseline_headset_off_24.xml diff --git a/app/src/main/res/drawable/ic_audio_streaming_off_24dp.xml b/app/src/main/res/drawable/ic_audio_streaming_off_24dp.xml deleted file mode 100644 index 8d0980bfa12..00000000000 --- a/app/src/main/res/drawable/ic_audio_streaming_off_24dp.xml +++ /dev/null @@ -1,8 +0,0 @@ - - - diff --git a/app/src/main/res/drawable/ic_audio_streaming_on_24dp.xml b/app/src/main/res/drawable/ic_audio_streaming_on_24dp.xml deleted file mode 100644 index 3d1f3cd9db2..00000000000 --- a/app/src/main/res/drawable/ic_audio_streaming_on_24dp.xml +++ /dev/null @@ -1,8 +0,0 @@ - - - diff --git a/app/src/main/res/drawable/ic_baseline_headset_24.xml b/app/src/main/res/drawable/ic_baseline_headset_24.xml new file mode 100644 index 00000000000..2e70e369c18 --- /dev/null +++ b/app/src/main/res/drawable/ic_baseline_headset_24.xml @@ -0,0 +1,11 @@ + + + diff --git a/app/src/main/res/drawable/ic_baseline_headset_off_24.xml b/app/src/main/res/drawable/ic_baseline_headset_off_24.xml new file mode 100644 index 00000000000..57fc8016896 --- /dev/null +++ b/app/src/main/res/drawable/ic_baseline_headset_off_24.xml @@ -0,0 +1,10 @@ + + + diff --git a/app/src/main/res/layout/exploration_activity.xml b/app/src/main/res/layout/exploration_activity.xml index d17a705b995..acbd1a98a67 100755 --- a/app/src/main/res/layout/exploration_activity.xml +++ b/app/src/main/res/layout/exploration_activity.xml @@ -53,7 +53,7 @@ android:layout_gravity="end" android:contentDescription="@{viewModel.isAudioStreamingOn ? @string/audio_player_on : @string/audio_player_off}" android:scaleType="center" - app:srcCompat="@{viewModel.isAudioStreamingOn ? @drawable/ic_audio_streaming_on_24dp : @drawable/ic_audio_streaming_off_24dp}" + app:srcCompat="@{viewModel.isAudioStreamingOn ? @drawable/ic_baseline_headset_24 : @drawable/ic_baseline_headset_off_24}" android:visibility="@{viewModel.showAudioButton ? View.VISIBLE : View.GONE}" /> From ec9a891c3e321f116f9183f13f834be3aac5418d Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Fri, 29 Jul 2022 02:11:41 -0700 Subject: [PATCH 08/57] Revert "Fixes part of #4195 : Added dark mode support to QuestionPlayer and Exploration (#4382)" (#4478) This reverts commit c3b42a4c573e21d368d76d18b3b0584e351340aa. --- app/src/main/res/drawable/color_cursor.xml | 2 +- .../drawable/edit_text_background_border.xml | 4 ++-- .../edit_text_background_border_blue.xml | 4 ++-- .../res/drawable/ic_arrow_down_grey_24dp.xml | 1 - .../res/drawable/ic_arrow_right_grey_24dp.xml | 1 - .../drawable/submitted_answer_background.xml | 4 ++-- .../activity_input_interaction_view_test.xml | 14 ++++++------- .../res/layout/fraction_interaction_item.xml | 2 -- .../math_expression_interactions_item.xml | 2 -- .../layout/numeric_input_interaction_item.xml | 2 -- .../layout/ratio_input_interaction_item.xml | 2 -- .../main/res/layout/submitted_answer_item.xml | 2 +- .../layout/text_input_interaction_item.xml | 1 - .../main/res/values-night/color_palette.xml | 21 ------------------- app/src/main/res/values-v21/themes.xml | 1 - app/src/main/res/values/color_defs.xml | 14 +------------ app/src/main/res/values/color_palette.xml | 21 ------------------- app/src/main/res/values/component_colors.xml | 14 ++----------- app/src/main/res/values/styles.xml | 3 +-- app/src/main/res/values/themes.xml | 1 - 20 files changed, 19 insertions(+), 97 deletions(-) diff --git a/app/src/main/res/drawable/color_cursor.xml b/app/src/main/res/drawable/color_cursor.xml index 4573a4b96a4..2f93b37ddcf 100644 --- a/app/src/main/res/drawable/color_cursor.xml +++ b/app/src/main/res/drawable/color_cursor.xml @@ -1,5 +1,5 @@ - + diff --git a/app/src/main/res/drawable/edit_text_background_border.xml b/app/src/main/res/drawable/edit_text_background_border.xml index 4bc3a0fd077..3dd64225973 100644 --- a/app/src/main/res/drawable/edit_text_background_border.xml +++ b/app/src/main/res/drawable/edit_text_background_border.xml @@ -2,8 +2,8 @@ - + + android:color="@color/oppia_grey_border_dark" /> diff --git a/app/src/main/res/drawable/edit_text_background_border_blue.xml b/app/src/main/res/drawable/edit_text_background_border_blue.xml index 8291c80cc18..852ab5a8df3 100644 --- a/app/src/main/res/drawable/edit_text_background_border_blue.xml +++ b/app/src/main/res/drawable/edit_text_background_border_blue.xml @@ -2,8 +2,8 @@ - + + android:color="@color/color_def_oppia_dark_blue" /> diff --git a/app/src/main/res/drawable/ic_arrow_down_grey_24dp.xml b/app/src/main/res/drawable/ic_arrow_down_grey_24dp.xml index d96f89eef1e..59518af0471 100644 --- a/app/src/main/res/drawable/ic_arrow_down_grey_24dp.xml +++ b/app/src/main/res/drawable/ic_arrow_down_grey_24dp.xml @@ -1,7 +1,6 @@ + android:color="@color/color_def_black"/> + android:color="@color/color_def_white"/> diff --git a/app/src/main/res/layout/activity_input_interaction_view_test.xml b/app/src/main/res/layout/activity_input_interaction_view_test.xml index d4a4ce03572..fa9b7cb3920 100644 --- a/app/src/main/res/layout/activity_input_interaction_view_test.xml +++ b/app/src/main/res/layout/activity_input_interaction_view_test.xml @@ -57,8 +57,8 @@ android:paddingBottom="8dp" android:singleLine="true" android:text="@={fractionInteractionViewModel.answerText}" - android:textColor="@color/component_color_shared_input_interaction_edit_text_color" - android:textColorHint="@color/component_color_shared_edit_text_hint_color" + android:textColor="@color/oppia_primary_text" + android:textColorHint="@color/edit_text_hint" android:textSize="14sp" android:textStyle="italic" app:textChangedListener="@{fractionInteractionViewModel.answerTextWatcher}" /> @@ -73,7 +73,7 @@ android:fontFamily="sans-serif" android:minHeight="32dp" android:text="@{fractionInteractionViewModel.errorMessage}" - android:textColor="@color/component_color_shared_input_error_color" + android:textColor="@color/edit_text_error" android:textSize="12sp" android:visibility="@{fractionInteractionViewModel.errorMessage.length() > 0 ? View.VISIBLE : View.INVISIBLE}" /> @@ -94,8 +94,8 @@ android:paddingBottom="8dp" android:singleLine="true" android:text="@={ratioInteractionInputViewModel.answerText}" - android:textColor="@color/component_color_shared_input_interaction_edit_text_color" - android:textColorHint="@color/component_color_shared_edit_text_hint_color" + android:textColor="@color/oppia_primary_text" + android:textColorHint="@color/edit_text_hint" android:textSize="14sp" android:textStyle="italic" app:textChangedListener="@{ratioInteractionInputViewModel.answerTextWatcher}" /> @@ -110,7 +110,7 @@ android:fontFamily="sans-serif" android:minHeight="32dp" android:text="@{ratioInteractionInputViewModel.errorMessage}" - android:textColor="@color/component_color_shared_input_error_color" + android:textColor="@color/edit_text_error" android:textSize="12sp" android:visibility="@{ratioInteractionInputViewModel.errorMessage.length() > 0 ? View.VISIBLE : View.INVISIBLE}" /> @@ -140,7 +140,7 @@ android:fontFamily="sans-serif" android:minHeight="32dp" android:text="@{numericInputViewModel.errorMessage}" - android:textColor="@color/component_color_shared_input_error_color" + android:textColor="@color/edit_text_error" android:textSize="12sp" android:visibility="@{numericInputViewModel.errorMessage.length() > 0 ? View.VISIBLE : View.INVISIBLE}" /> diff --git a/app/src/main/res/layout/fraction_interaction_item.xml b/app/src/main/res/layout/fraction_interaction_item.xml index 4ac40582241..f361f591c1a 100644 --- a/app/src/main/res/layout/fraction_interaction_item.xml +++ b/app/src/main/res/layout/fraction_interaction_item.xml @@ -26,7 +26,6 @@ android:hint="@{viewModel.hintText}" android:inputType="text" android:text="@={viewModel.answerText}" - android:textColorHint="@color/component_color_shared_edit_text_hint_color" app:textChangedListener="@{viewModel.answerTextWatcher}" app:layout_constraintEnd_toEndOf="parent" app:layout_constraintStart_toStartOf="parent" @@ -36,7 +35,6 @@ android:id="@+id/fraction_input_error" style="@style/InputInteractionErrorTextView" android:text="@{viewModel.errorMessage}" - android:textColor="@color/component_color_shared_input_error_color" app:layout_constraintEnd_toEndOf="parent" app:layout_constraintStart_toStartOf="parent" app:layout_constraintTop_toBottomOf="@+id/fraction_input_interaction_view" diff --git a/app/src/main/res/layout/math_expression_interactions_item.xml b/app/src/main/res/layout/math_expression_interactions_item.xml index 6676f721cc8..23bc09f680a 100644 --- a/app/src/main/res/layout/math_expression_interactions_item.xml +++ b/app/src/main/res/layout/math_expression_interactions_item.xml @@ -26,7 +26,6 @@ app:placeholder="@{viewModel.hintText}" android:inputType="text" android:text="@={viewModel.answerText}" - android:textColorHint="@color/component_color_shared_edit_text_hint_color" app:textChangedListener="@{viewModel.answerTextWatcher}" app:layout_constraintEnd_toEndOf="parent" app:layout_constraintStart_toStartOf="parent" @@ -37,7 +36,6 @@ android:id="@+id/math_expression_input_error" style="@style/InputInteractionErrorTextView" android:text="@{viewModel.errorMessage}" - android:textColor="@color/component_color_shared_input_error_color" app:layout_constraintEnd_toEndOf="parent" app:layout_constraintStart_toStartOf="parent" app:layout_constraintTop_toBottomOf="@+id/math_expression_input_interaction_view" diff --git a/app/src/main/res/layout/numeric_input_interaction_item.xml b/app/src/main/res/layout/numeric_input_interaction_item.xml index bea2d287176..542881cbedf 100644 --- a/app/src/main/res/layout/numeric_input_interaction_item.xml +++ b/app/src/main/res/layout/numeric_input_interaction_item.xml @@ -27,7 +27,6 @@ android:hint="@string/numeric_input_hint" android:inputType="numberDecimal" android:text="@={viewModel.answerText}" - android:textColorHint="@color/component_color_shared_edit_text_hint_color" app:layout_constraintEnd_toEndOf="parent" app:layout_constraintStart_toStartOf="parent" app:layout_constraintTop_toTopOf="parent" @@ -36,7 +35,6 @@ @color/color_def_white @color/color_def_oppia_light_black @color/color_def_accessible_light_grey - @color/color_def_white @color/color_def_oppia_light_black @color/color_def_accessible_grey @color/color_def_white @@ -34,12 +33,6 @@ @color/color_def_oppia_light_black @color/color_def_oppia_grayish_black @color/color_def_dark_blue - @color/color_def_oppia_pink - @color/color_def_oppia_dark_grey - @color/color_def_white - @color/color_def_white - @color/color_def_white - @color/color_def_white @color/color_def_oppia_grayish_black @color/color_def_white @color/color_def_white @@ -52,20 +45,6 @@ @color/color_def_oppia_pink @color/color_def_forest_green @color/color_def_forest_green - @color/color_def_white - @color/color_def_oppia_silver - @color/color_def_oppia_turquoise - @color/color_def_oppia_dark_grey - @color/color_def_maastricht_blue - @color/color_def_forest_green - @color/color_def_oppia_light_black - @color/color_def_dark_pink - @color/color_def_oppia_pink - @color/color_def_persian_blue - @color/color_def_bright_green - @color/color_def_white - @color/color_def_oppia_metallic_blue - @color/color_def_dark_purple @color/color_def_black_grey @color/color_def_oppia_turquoise @color/color_def_dark_pink diff --git a/app/src/main/res/values-v21/themes.xml b/app/src/main/res/values-v21/themes.xml index 2fcb8536985..6836b964a9e 100644 --- a/app/src/main/res/values-v21/themes.xml +++ b/app/src/main/res/values-v21/themes.xml @@ -15,7 +15,6 @@ @color/color_palette_accent_color false true - @style/InputInteractionEditText @style/Animation.Activity @color/component_color_shared_activity_status_bar_color diff --git a/app/src/main/res/values/color_defs.xml b/app/src/main/res/values/color_defs.xml index 3f420995410..74d331b6d52 100644 --- a/app/src/main/res/values/color_defs.xml +++ b/app/src/main/res/values/color_defs.xml @@ -54,24 +54,12 @@ #395FD0 #DDDDDD #707070 - #923026 #FFFFF0 #999999 #02655c - #666666 - #923026 - #0e162f - #081661 - #8A3D69 - #00609C - #009C8A - #674172 - #7659B6 - #2F6687 - #A6503A - #441530 #923026 #33999999 + #8A3D69 #ACFFF8 #61999999 #00E4F2F1 diff --git a/app/src/main/res/values/color_palette.xml b/app/src/main/res/values/color_palette.xml index c97e06cfef9..4626e3caa8c 100644 --- a/app/src/main/res/values/color_palette.xml +++ b/app/src/main/res/values/color_palette.xml @@ -20,7 +20,6 @@ @color/color_def_accessible_light_grey_2 @color/color_def_white @color/color_def_spanish_grey - @color/color_def_spanish_grey @color/color_def_bright_grey @color/color_def_white @color/color_def_black @@ -34,12 +33,6 @@ @color/color_def_oppia_light_green @color/color_def_oppia_solid_blue @color/color_def_oppia_stroke_blue - @color/color_def_input_error - @color/color_def_white - @color/color_def_black - @color/color_def_oppia_dark_blue - @color/color_def_mid_grey - @color/color_def_mid_grey @color/color_def_white @color/color_def_oppia_dark_blue @color/color_def_oppia_dark_blue @@ -52,20 +45,6 @@ @color/color_def_error_text @color/color_def_accessible_grey @color/color_def_accessible_grey - @color/color_def_oppia_dark_grey_border - @color/color_def_oppia_dark_blue - @color/color_def_white - @color/color_def_oppia_dark_blue - @color/color_def_ocean_blue - @color/color_def_oppia_green - @color/color_def_oppia_light_green - @color/color_def_oppia_brown_dark - @color/color_def_oppia_brown_dark - @color/color_def_persian_blue - @color/color_def_oppia_light_brown - @color/color_def_black - @color/color_def_teal_blue - @color/color_def_dark_red @color/color_def_black_grey @color/color_def_oppia_dark_blue @color/color_def_oppia_brown_dark diff --git a/app/src/main/res/values/component_colors.xml b/app/src/main/res/values/component_colors.xml index 638e760ae1a..c01ce15d7d5 100644 --- a/app/src/main/res/values/component_colors.xml +++ b/app/src/main/res/values/component_colors.xml @@ -1,22 +1,12 @@ - @color/color_palette_cursor_color + @color/color_palette_primary_text_color @color/color_palette_toolbar_color @color/color_palette_highlighted_text_color @color/color_palette_highlighted_text_color @color/color_palette_exploration_background_color @color/color_palette_highlighted_text_color @color/color_palette_highlighted_text_color - @color/color_palette_highlighted_text_color - @color/color_palette_input_error_color - @color/color_palette_submitted_answer_item_solid_color - @color/color_palette_submitted_answer_item_stroke_color - @color/color_palette_highlighted_text_color - @color/color_palette_hint_text_input_interaction_color - @color/color_palette_input_interaction_edit_text_border_color - @color/color_palette_input_interaction_edit_text_not_selected_border_color - @color/color_palette_arrow_down_grey_color - @color/color_palette_arrow_right_grey_color @color/color_palette_rounded_rect_background_color @color/color_palette_multiple_choice_color @color/color_palette_multiple_choice_text_color @@ -50,7 +40,7 @@ @color/color_palette_primary_text_color @color/color_palette_primary_text_color @color/color_palette_primary_text_color - @color/color_palette_text_input_background_color + @color/color_palette_text_input_background_color @color/color_palette_error_color @color/color_palette_container_background_color @color/color_palette_dark_background_color diff --git a/app/src/main/res/values/styles.xml b/app/src/main/res/values/styles.xml index 06b21244d8e..d7237b66bd2 100644 --- a/app/src/main/res/values/styles.xml +++ b/app/src/main/res/values/styles.xml @@ -164,7 +164,7 @@ From 2daaf811389eb9f315c5ee1656e097a1a1c74792 Mon Sep 17 00:00:00 2001 From: Vraj Desai <43074241+vrajdesai78@users.noreply.github.com> Date: Tue, 2 Aug 2022 21:50:14 +0530 Subject: [PATCH 09/57] Fixes #3895: Rich-text hyperlink text improvement (#4486) * Rich text changed for hyperlink text * Failing test fixed * klint issue fixed --- .../topic/revisioncard/RevisionCardFragmentTest.kt | 12 ++++++++---- domain/src/main/assets/GJ2rLXRKD5hw_2.json | 2 +- domain/src/main/assets/GJ2rLXRKD5hw_2.textproto | 2 +- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/app/src/sharedTest/java/org/oppia/android/app/topic/revisioncard/RevisionCardFragmentTest.kt b/app/src/sharedTest/java/org/oppia/android/app/topic/revisioncard/RevisionCardFragmentTest.kt index 445a72e33f0..e0f30bbef08 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/topic/revisioncard/RevisionCardFragmentTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/topic/revisioncard/RevisionCardFragmentTest.kt @@ -365,7 +365,7 @@ class RevisionCardFragmentTest { testCoroutineDispatchers.runCurrent() onView(withId(R.id.revision_card_explanation_text)).check( - matches(withText(containsString("Learn more"))) + matches(withText(containsString("Description of subtopic is here."))) ) } } @@ -386,7 +386,7 @@ class RevisionCardFragmentTest { testCoroutineDispatchers.runCurrent() onView(withId(R.id.revision_card_explanation_text)).check( - matches(withText(containsString("Learn more"))) + matches(withText(containsString("Description of subtopic is here."))) ) } } @@ -403,7 +403,9 @@ class RevisionCardFragmentTest { ).use { testCoroutineDispatchers.runCurrent() - onView(withId(R.id.revision_card_explanation_text)).perform(openClickableSpan("Learn more")) + onView(withId(R.id.revision_card_explanation_text)).perform( + openClickableSpan("This concept card demonstrates overall concept card functionality.") + ) testCoroutineDispatchers.runCurrent() onView(withText("Concept Card")).inRoot(isDialog()).check(matches(isDisplayed())) @@ -427,7 +429,9 @@ class RevisionCardFragmentTest { onView(isRoot()).perform(orientationLandscape()) testCoroutineDispatchers.runCurrent() - onView(withId(R.id.revision_card_explanation_text)).perform(openClickableSpan("Learn more")) + onView(withId(R.id.revision_card_explanation_text)).perform( + openClickableSpan("This concept card demonstrates overall concept card functionality.") + ) testCoroutineDispatchers.runCurrent() onView(withText("Concept Card")).inRoot(isDialog()).check(matches(isDisplayed())) diff --git a/domain/src/main/assets/GJ2rLXRKD5hw_2.json b/domain/src/main/assets/GJ2rLXRKD5hw_2.json index 2397eee1e6a..5faf47cf8f9 100644 --- a/domain/src/main/assets/GJ2rLXRKD5hw_2.json +++ b/domain/src/main/assets/GJ2rLXRKD5hw_2.json @@ -4,7 +4,7 @@ "page_contents": { "subtitled_html": { "content_id": "content", - "html": "

Description of subtopic is here.

" + "html": "

Description of subtopic is here.

" }, "recorded_voiceovers": { "voiceovers_mapping": { diff --git a/domain/src/main/assets/GJ2rLXRKD5hw_2.textproto b/domain/src/main/assets/GJ2rLXRKD5hw_2.textproto index 3b83337f4c5..c914a962e5e 100644 --- a/domain/src/main/assets/GJ2rLXRKD5hw_2.textproto +++ b/domain/src/main/assets/GJ2rLXRKD5hw_2.textproto @@ -1,6 +1,6 @@ subtopic_title: "Fractions of a group" page_contents { - html: "

Description of subtopic is here.

" + html: "

Description of subtopic is here.

" content_id: "content" } recorded_voiceover { From d8ab9e953f694e21fb6621ef7786516f9f068bce Mon Sep 17 00:00:00 2001 From: Vraj Desai <43074241+vrajdesai78@users.noreply.github.com> Date: Wed, 3 Aug 2022 19:05:36 +0530 Subject: [PATCH 10/57] Fixes #4443: Talkback reader reads blank on empty resume lesson summary (#4444) * Blank space removed by setting importantForAccessibility * Handled edge case when description is empty * updated other xml files and optimized code * import optimized * Changed logic * changed logic to handle empty description and added tests * removed unused import * Optimized imports --- .../ResumeLessonFragmentPresenter.kt | 8 +++++- .../resumelesson/ResumeLessonFragmentTest.kt | 27 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/org/oppia/android/app/resumelesson/ResumeLessonFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/resumelesson/ResumeLessonFragmentPresenter.kt index 9c223f25862..348db4bc672 100644 --- a/app/src/main/java/org/oppia/android/app/resumelesson/ResumeLessonFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/resumelesson/ResumeLessonFragmentPresenter.kt @@ -117,7 +117,7 @@ class ResumeLessonFragmentPresenter @Inject constructor( } private fun updateChapterDescription() { - binding.resumeLessonChapterDescriptionTextView.text = htmlParserFactory.create( + val chapterDescription = htmlParserFactory.create( resourceBucketName, resumeLessonViewModel.entityType, explorationId, @@ -126,6 +126,12 @@ class ResumeLessonFragmentPresenter @Inject constructor( resumeLessonViewModel.chapterSummary.get()!!.summary, binding.resumeLessonChapterDescriptionTextView ) + if (chapterDescription.isNotBlank()) { + binding.resumeLessonChapterDescriptionTextView.visibility = View.VISIBLE + binding.resumeLessonChapterDescriptionTextView.text = chapterDescription + } else { + binding.resumeLessonChapterDescriptionTextView.visibility = View.GONE + } } private fun getResumeLessonViewModel(): ResumeLessonViewModel { diff --git a/app/src/sharedTest/java/org/oppia/android/app/resumelesson/ResumeLessonFragmentTest.kt b/app/src/sharedTest/java/org/oppia/android/app/resumelesson/ResumeLessonFragmentTest.kt index 098001373d2..bb5a9228d51 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/resumelesson/ResumeLessonFragmentTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/resumelesson/ResumeLessonFragmentTest.kt @@ -11,6 +11,7 @@ import androidx.test.core.app.ActivityScenario.launch import androidx.test.core.app.ApplicationProvider import androidx.test.espresso.Espresso.onView import androidx.test.espresso.assertion.ViewAssertions.matches +import androidx.test.espresso.matcher.ViewMatchers.isDisplayed import androidx.test.espresso.matcher.ViewMatchers.isRoot import androidx.test.espresso.matcher.ViewMatchers.withId import androidx.test.espresso.matcher.ViewMatchers.withText @@ -18,6 +19,7 @@ import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.rule.ActivityTestRule import com.google.common.truth.Truth.assertThat import dagger.Component +import org.hamcrest.Matchers.not import org.junit.After import org.junit.Before import org.junit.Rule @@ -71,6 +73,9 @@ import org.oppia.android.domain.topic.FRACTIONS_EXPLORATION_ID_0 import org.oppia.android.domain.topic.FRACTIONS_STORY_ID_0 import org.oppia.android.domain.topic.FRACTIONS_TOPIC_ID import org.oppia.android.domain.topic.PrimeTopicAssetsControllerModule +import org.oppia.android.domain.topic.RATIOS_EXPLORATION_ID_0 +import org.oppia.android.domain.topic.RATIOS_STORY_ID_0 +import org.oppia.android.domain.topic.RATIOS_TOPIC_ID import org.oppia.android.domain.workmanager.WorkManagerConfigurationModule import org.oppia.android.testing.OppiaTestRule import org.oppia.android.testing.TestImageLoaderModule @@ -208,6 +213,16 @@ class ResumeLessonFragmentTest { } } + @Test + fun testResumeLessonFragment_emptyLessonDescriptionNotDisplayed() { + launch(createResumeRatiosLessonActivityIntent()).use { + testCoroutineDispatchers.runCurrent() + onView(withId(R.id.resume_lesson_chapter_description_text_view)).check( + matches(not(isDisplayed())) + ) + } + } + @Test fun testResumeLessonFragment_lessonDescriptionIsInRtl_isDisplayedCorrectly() { launch(createResumeLessonActivityIntent()).use { scenario -> @@ -260,6 +275,18 @@ class ResumeLessonFragmentTest { ) } + private fun createResumeRatiosLessonActivityIntent(): Intent { + return ResumeLessonActivity.createResumeLessonActivityIntent( + context, + internalProfileId, + RATIOS_TOPIC_ID, + RATIOS_STORY_ID_0, + RATIOS_EXPLORATION_ID_0, + backflowScreen = null, + explorationCheckpoint = ExplorationCheckpoint.getDefaultInstance() + ) + } + // TODO(#59): Figure out a way to reuse modules instead of needing to re-declare them. @Singleton @Component( From db57738b6e6686bb8f7fcce14cb9f6a8011a57c1 Mon Sep 17 00:00:00 2001 From: Vraj Desai <43074241+vrajdesai78@users.noreply.github.com> Date: Sun, 14 Aug 2022 03:23:43 +0530 Subject: [PATCH 11/57] Fixes #3896: Improve flow of lessons tab (#4441) * Removed repetitive content description * Updated topic_lessons_story_summary.xml for landscape * Fixed klint issues * KDoc added * Removed changed onclick * Updated tests * Updated testcases * Drop-down-icon clicklistener added * klint issue fixed * Optimized code for lessons flow * Added tests * Klint issue fixed * Nit changes * Updated logic and testcases * fixed error * Rerun ci checks --- .../topic/lessons/StorySummaryViewModel.kt | 15 ++ .../lessons/TopicLessonsFragmentPresenter.kt | 58 ++++--- .../topic_lessons_story_summary.xml | 6 +- .../main/res/layout/lessons_chapter_view.xml | 2 +- .../layout/topic_lessons_story_summary.xml | 8 +- .../topic/lessons/TopicLessonsFragmentTest.kt | 159 +++++++++--------- 6 files changed, 145 insertions(+), 103 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/topic/lessons/StorySummaryViewModel.kt b/app/src/main/java/org/oppia/android/app/topic/lessons/StorySummaryViewModel.kt index d7b0832e002..f4f57e584ae 100644 --- a/app/src/main/java/org/oppia/android/app/topic/lessons/StorySummaryViewModel.kt +++ b/app/src/main/java/org/oppia/android/app/topic/lessons/StorySummaryViewModel.kt @@ -49,6 +49,21 @@ class StorySummaryViewModel( ) } + /* + * Returns content description of progress container based on story percentage. + * + * @return a [String] representing content description for progress container + */ + fun computeProgressContainerContentDescription(): String { + return if (storyPercentage.get()!! < 100) { + "${storyProgressPercentageText.get()} " + + resourceHandler.getStringInLocale(R.string.status_in_progress) + } else { + "${storyProgressPercentageText.get()} " + + resourceHandler.getStringInLocale(R.string.status_completed) + } + } + private fun computeStoryProgressPercentageText(storyPercentage: Int): String { return resourceHandler.getStringInLocaleWithWrapping( R.string.topic_story_progress_percentage, storyPercentage.toString() diff --git a/app/src/main/java/org/oppia/android/app/topic/lessons/TopicLessonsFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/topic/lessons/TopicLessonsFragmentPresenter.kt index 5bb654e2550..316d1e5f174 100644 --- a/app/src/main/java/org/oppia/android/app/topic/lessons/TopicLessonsFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/topic/lessons/TopicLessonsFragmentPresenter.kt @@ -23,6 +23,7 @@ import org.oppia.android.databinding.TopicLessonsTitleBinding import org.oppia.android.domain.exploration.ExplorationDataController import org.oppia.android.domain.exploration.lightweightcheckpointing.ExplorationCheckpointController import org.oppia.android.domain.oppialogger.OppiaLogger +import org.oppia.android.util.accessibility.AccessibilityService import org.oppia.android.util.data.AsyncResult import org.oppia.android.util.data.DataProviders.Companion.toLiveData import javax.inject.Inject @@ -44,6 +45,9 @@ class TopicLessonsFragmentPresenter @Inject constructor( @Inject lateinit var topicLessonViewModel: TopicLessonViewModel + @Inject + lateinit var accessibilityService: AccessibilityService + private var currentExpandedChapterListIndex: Int? = null private lateinit var binding: TopicLessonsFragmentBinding @@ -180,28 +184,44 @@ class TopicLessonsFragmentPresenter @Inject constructor( ) binding.chapterRecyclerView.adapter = createChapterRecyclerViewAdapter() + binding.expandListIcon.setOnClickListener { + expandStoryList(position) + } + binding.root.setOnClickListener { - val previousIndex: Int? = currentExpandedChapterListIndex - currentExpandedChapterListIndex = - if (currentExpandedChapterListIndex != null && - currentExpandedChapterListIndex == position - ) { - null - } else { - position - } - expandedChapterListIndexListener.onExpandListIconClicked(currentExpandedChapterListIndex) - if (previousIndex != null && currentExpandedChapterListIndex != null && - previousIndex == currentExpandedChapterListIndex + expandStoryList(position) + } + + if (accessibilityService.isScreenReaderEnabled()) { + binding.root.isClickable = false + binding.expandListIcon.isClickable = true + } else { + binding.root.isClickable = true + binding.expandListIcon.isClickable = false + } + } + + private fun expandStoryList(position: Int) { + val previousIndex: Int? = currentExpandedChapterListIndex + currentExpandedChapterListIndex = + if (currentExpandedChapterListIndex != null && + currentExpandedChapterListIndex == position ) { - bindingAdapter.notifyItemChanged(currentExpandedChapterListIndex!!) + null } else { - previousIndex?.let { - bindingAdapter.notifyItemChanged(previousIndex) - } - currentExpandedChapterListIndex?.let { - bindingAdapter.notifyItemChanged(currentExpandedChapterListIndex!!) - } + position + } + expandedChapterListIndexListener.onExpandListIconClicked(currentExpandedChapterListIndex) + if (previousIndex != null && currentExpandedChapterListIndex != null && + previousIndex == currentExpandedChapterListIndex + ) { + bindingAdapter.notifyItemChanged(currentExpandedChapterListIndex!!) + } else { + previousIndex?.let { + bindingAdapter.notifyItemChanged(previousIndex) + } + currentExpandedChapterListIndex?.let { + bindingAdapter.notifyItemChanged(currentExpandedChapterListIndex!!) } } } diff --git a/app/src/main/res/layout-sw600dp-land/topic_lessons_story_summary.xml b/app/src/main/res/layout-sw600dp-land/topic_lessons_story_summary.xml index 8922aad9a63..23996aea2b2 100644 --- a/app/src/main/res/layout-sw600dp-land/topic_lessons_story_summary.xml +++ b/app/src/main/res/layout-sw600dp-land/topic_lessons_story_summary.xml @@ -26,6 +26,7 @@ android:layout_gravity="center" android:layout_marginTop="12dp" android:layout_marginBottom="12dp" + android:importantForAccessibility="no" app:cardBackgroundColor="@color/color_def_white" app:cardCornerRadius="4dp" app:cardElevation="4dp" @@ -49,13 +50,13 @@ android:layout_height="wrap_content" android:layout_marginTop="24dp" android:gravity="center" + android:contentDescription="@{viewModel.computeProgressContainerContentDescription()}" android:orientation="vertical"> + android:layout_height="wrap_content"> diff --git a/app/src/main/res/layout/lessons_chapter_view.xml b/app/src/main/res/layout/lessons_chapter_view.xml index 45d609923d0..7ddce56be57 100644 --- a/app/src/main/res/layout/lessons_chapter_view.xml +++ b/app/src/main/res/layout/lessons_chapter_view.xml @@ -31,7 +31,6 @@ android:layout_height="16dp" android:layout_marginStart="16dp" android:layout_marginEnd="8dp" - android:contentDescription="@{viewModel.computeChapterPlayStateIconContentDescription()}" app:srcCompat="@{viewModel.chapterPlayState == ChapterPlayState.COMPLETED?@drawable/ic_check_24dp:@drawable/ic_pending_24dp}" android:visibility="@{(viewModel.chapterPlayState == ChapterPlayState.COMPLETED || viewModel.chapterPlayState == ChapterPlayState.IN_PROGRESS_SAVED)?View.VISIBLE : View.INVISIBLE}" /> @@ -57,6 +56,7 @@ android:layout_marginBottom="28dp" android:fontFamily="sans-serif" android:importantForAccessibility="@{viewModel.chapterPlayState != ChapterPlayState.NOT_PLAYABLE_MISSING_PREREQUISITES ? View.IMPORTANT_FOR_ACCESSIBILITY_YES : View.IMPORTANT_FOR_ACCESSIBILITY_NO}" + android:contentDescription="@{viewModel.computeChapterPlayStateIconContentDescription()}" android:text="@{viewModel.chapterName}" android:textColor="@{viewModel.chapterPlayState != ChapterPlayState.NOT_PLAYABLE_MISSING_PREREQUISITES ? @color/oppia_primary_text : @color/oppia_primary_text_30}" android:textSize="14sp" /> diff --git a/app/src/main/res/layout/topic_lessons_story_summary.xml b/app/src/main/res/layout/topic_lessons_story_summary.xml index cd1501aae5e..1d69865eac9 100644 --- a/app/src/main/res/layout/topic_lessons_story_summary.xml +++ b/app/src/main/res/layout/topic_lessons_story_summary.xml @@ -23,6 +23,7 @@ android:layout_marginTop="12dp" android:layout_marginEnd="20dp" android:layout_marginBottom="12dp" + android:importantForAccessibility="no" app:cardBackgroundColor="@color/color_def_white" app:cardCornerRadius="4dp" app:cardElevation="4dp" @@ -42,17 +43,18 @@ android:orientation="horizontal"> + android:layout_height="wrap_content"> @@ -118,7 +119,6 @@ android:layout_width="wrap_content" android:layout_height="wrap_content" android:fontFamily="sans-serif" - android:importantForAccessibility="no" android:text="@{viewModel.computeChapterCountText()}" android:textColor="@color/oppia_primary_text" android:textSize="16sp" /> diff --git a/app/src/sharedTest/java/org/oppia/android/app/topic/lessons/TopicLessonsFragmentTest.kt b/app/src/sharedTest/java/org/oppia/android/app/topic/lessons/TopicLessonsFragmentTest.kt index d8036b9b35a..32b7a7086c4 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/topic/lessons/TopicLessonsFragmentTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/topic/lessons/TopicLessonsFragmentTest.kt @@ -18,6 +18,7 @@ import androidx.test.espresso.intent.Intents.intended import androidx.test.espresso.intent.matcher.IntentMatchers.hasComponent import androidx.test.espresso.intent.matcher.IntentMatchers.hasExtra import androidx.test.espresso.matcher.ViewMatchers.hasDescendant +import androidx.test.espresso.matcher.ViewMatchers.isClickable import androidx.test.espresso.matcher.ViewMatchers.isDescendantOfA import androidx.test.espresso.matcher.ViewMatchers.isDisplayed import androidx.test.espresso.matcher.ViewMatchers.isRoot @@ -106,6 +107,7 @@ import org.oppia.android.testing.threading.TestDispatcherModule import org.oppia.android.testing.time.FakeOppiaClock import org.oppia.android.testing.time.FakeOppiaClockModule import org.oppia.android.util.accessibility.AccessibilityTestModule +import org.oppia.android.util.accessibility.FakeAccessibilityService import org.oppia.android.util.caching.AssetModule import org.oppia.android.util.caching.testing.CachingTestModule import org.oppia.android.util.gcsresource.GcsResourceModule @@ -146,6 +148,9 @@ class TopicLessonsFragmentTest { @Inject lateinit var fakeOppiaClock: FakeOppiaClock + @Inject + lateinit var fakeAccessibilityService: FakeAccessibilityService + @Inject lateinit var explorationCheckpointTestHelper: ExplorationCheckpointTestHelper @@ -212,7 +217,7 @@ class TopicLessonsFragmentTest { ) launch(createTopicActivityIntent(internalProfileId, RATIOS_TOPIC_ID)).use { clickLessonTab() - verifyProgressContentDescriptionAtPosition(itemPosition = 1, stringToMatch = "100%") + verifyProgressContentDescriptionAtPosition(itemPosition = 1, stringToMatch = "100% Completed") } } @@ -224,7 +229,7 @@ class TopicLessonsFragmentTest { ) launch(createTopicActivityIntent(internalProfileId, RATIOS_TOPIC_ID)).use { clickLessonTab() - verifyProgressContentDescriptionAtPosition(itemPosition = 1, stringToMatch = "0%") + verifyProgressContentDescriptionAtPosition(itemPosition = 1, stringToMatch = "0% In Progress") } } @@ -240,7 +245,10 @@ class TopicLessonsFragmentTest { ) launch(createTopicActivityIntent(internalProfileId, RATIOS_TOPIC_ID)).use { clickLessonTab() - verifyProgressContentDescriptionAtPosition(itemPosition = 1, stringToMatch = "50%") + verifyProgressContentDescriptionAtPosition( + itemPosition = 1, + stringToMatch = "50% In Progress" + ) } } @@ -248,18 +256,7 @@ class TopicLessonsFragmentTest { fun testLessonsPlayFragment_loadRatiosTopic_noStoryProgress_contentDescriptionIsCorrect() { launch(createTopicActivityIntent(internalProfileId, RATIOS_TOPIC_ID)).use { clickLessonTab() - verifyProgressContentDescriptionAtPosition(itemPosition = 1, stringToMatch = "0%") - } - } - - @Test - fun testLessonsPlayFragment_loadFractionsTopic_storyChapterTextsContentDescriptionIsCorrect() { - launch(createTopicActivityIntent(internalProfileId, FRACTIONS_TOPIC_ID)).use { - clickLessonTab() - verifyStoryAndChapterCountContentDescriptionAtPosition( - itemPosition = 1, - stringToMatch = "2 Chapters in Matthew Goes to the Bakery" - ) + verifyProgressContentDescriptionAtPosition(itemPosition = 1, stringToMatch = "0% In Progress") } } @@ -676,40 +673,6 @@ class TopicLessonsFragmentTest { } } - @Test - fun testLessonPlayFrag_loadRatiosTopic_partialProg_verifyContentDescriptionIsCorrect() { - storyProgressTestHelper.markInProgressSavedRatiosStory0Exp0( - profileId, - timestampOlderThanOneWeek = false - ) - launch(createTopicActivityIntent(internalProfileId, RATIOS_TOPIC_ID)).use { - clickLessonTab() - clickStoryItem(position = 1, targetViewId = R.id.chapter_list_drop_down_icon) - scrollToPosition(position = 1) - verifyChapterPlayStateIconContentDescriptionIsCorrect( - itemPosition = 0, - contentDescription = "Chapter 1 with title What is a Ratio? is in progress" - ) - } - } - - @Test - fun testLessonPlayFrag_loadRatiosTopic_topicCompleted_verifyContentDescriptionIsCorrect() { - storyProgressTestHelper.markCompletedRatiosStory0Exp0( - profileId, - timestampOlderThanOneWeek = false - ) - launch(createTopicActivityIntent(internalProfileId, RATIOS_TOPIC_ID)).use { - clickLessonTab() - clickStoryItem(position = 1, targetViewId = R.id.chapter_list_drop_down_icon) - scrollToPosition(position = 1) - verifyChapterPlayStateIconContentDescriptionIsCorrect( - itemPosition = 0, - contentDescription = "Chapter 1 with title What is a Ratio? is completed" - ) - } - } - @Test fun testLessonPlayFrag_loadRatiosTopic_partialProg_partialProgIconIsDisplayed() { storyProgressTestHelper.markInProgressSavedRatiosStory0Exp0( @@ -849,6 +812,76 @@ class TopicLessonsFragmentTest { } } + @Test + fun testLessonsPlayFragment_loadRatiosTopic_checkDropDownIconWithScreenReader_isClickable() { + launch(createTopicActivityIntent(internalProfileId, FRACTIONS_TOPIC_ID)).use { + fakeAccessibilityService.setScreenReaderEnabled(true) + clickLessonTab() + testCoroutineDispatchers.runCurrent() + onView( + atPositionOnView( + recyclerViewId = R.id.story_summary_recycler_view, + position = 1, + targetViewId = R.id.expand_list_icon + ) + ).check( + matches(isClickable()) + ) + } + } + + @Test + fun testLessonPlayFragment_loadRatiosTopic_checkDropDownIconWithoutScreenReader_isNotClickable() { + launch(createTopicActivityIntent(internalProfileId, FRACTIONS_TOPIC_ID)).use { + clickLessonTab() + testCoroutineDispatchers.runCurrent() + onView( + atPositionOnView( + recyclerViewId = R.id.story_summary_recycler_view, + position = 1, + targetViewId = R.id.expand_list_icon + ) + ).check( + matches(not(isClickable())) + ) + } + } + + @Test + fun testLessonPlayFragment_loadRatiosTopic_checkStoryContainerWithScreenReader_isNotClickable() { + launch(createTopicActivityIntent(internalProfileId, FRACTIONS_TOPIC_ID)).use { + fakeAccessibilityService.setScreenReaderEnabled(true) + clickLessonTab() + testCoroutineDispatchers.runCurrent() + onView( + atPositionOnView( + recyclerViewId = R.id.story_summary_recycler_view, + position = 1, + targetViewId = R.id.story_container + ) + ).check( + matches(not(isClickable())) + ) + } + } + + @Test + fun testLessonPlayFragment_loadRatiosTopic_checkStoryContainerWithoutScreenReader_isClickable() { + launch(createTopicActivityIntent(internalProfileId, FRACTIONS_TOPIC_ID)).use { + clickLessonTab() + testCoroutineDispatchers.runCurrent() + onView( + atPositionOnView( + recyclerViewId = R.id.story_summary_recycler_view, + position = 1, + targetViewId = R.id.story_container + ) + ).check( + matches(isClickable()) + ) + } + } + private fun createTopicActivityIntent(internalProfileId: Int, topicId: String): Intent { return TopicActivity.createTopicActivityIntent( ApplicationProvider.getApplicationContext(), @@ -915,37 +948,11 @@ class TopicLessonsFragmentTest { atPositionOnView( recyclerViewId = R.id.story_summary_recycler_view, position = itemPosition, - targetViewId = R.id.story_progress_container + targetViewId = R.id.story_progress_linear_layout ) ).check(matches(withContentDescription(stringToMatch))) } - private fun verifyStoryAndChapterCountContentDescriptionAtPosition( - itemPosition: Int, - stringToMatch: String - ) { - onView( - atPositionOnView( - recyclerViewId = R.id.story_summary_recycler_view, - position = itemPosition, - targetViewId = R.id.story_name_chapter_count_container - ) - ).check(matches(withContentDescription(stringToMatch))) - } - - private fun verifyChapterPlayStateIconContentDescriptionIsCorrect( - itemPosition: Int, - contentDescription: String - ) { - onView( - atPositionOnView( - recyclerViewId = R.id.chapter_recycler_view, - position = itemPosition, - targetViewId = R.id.chapter_play_state_icon - ) - ).check(matches(withContentDescription(contentDescription))) - } - private fun verifyChapterPlayStateIconIsVisibleAtPosition(itemPosition: Int) { onView( atPositionOnView( From a55c8b25235e8d99ddbdef4e16a4fa36b4d22eb8 Mon Sep 17 00:00:00 2001 From: Joshua Murigi Date: Sun, 14 Aug 2022 00:57:45 +0300 Subject: [PATCH 12/57] Fix #4361: Continue button is not aligned properly when reading text size in extra large (#4385) * fixed_continue_action_button * fixed_continue_action_button * add layout width * removed continue_submit_button_width dimens --- app/src/main/res/layout/continue_interaction_item.xml | 2 +- app/src/main/res/layout/continue_navigation_button_item.xml | 2 +- app/src/main/res/values/dimens.xml | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/app/src/main/res/layout/continue_interaction_item.xml b/app/src/main/res/layout/continue_interaction_item.xml index 10c11624268..12a500b82cc 100644 --- a/app/src/main/res/layout/continue_interaction_item.xml +++ b/app/src/main/res/layout/continue_interaction_item.xml @@ -49,7 +49,7 @@