Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
c851395
Add new feature flags
theMr17 Mar 1, 2025
2cc5137
Replace enableExtraTopicTabsUi usage with enableTopicInfoTab & enable…
theMr17 Mar 2, 2025
dd106e7
Enable practice tab for alpha builds
theMr17 Mar 2, 2025
8394a3f
Remove unused parameter
theMr17 Mar 2, 2025
ec425d5
Remove EXTRA_TOPIC_TABS_UI feature flag
theMr17 Mar 2, 2025
19b6bb7
Fix index of featureFlagItemMap in test
theMr17 Mar 2, 2025
e1cf476
Fix lint tests
theMr17 Mar 2, 2025
4c35b71
Fix tests
theMr17 Mar 2, 2025
b58eb88
Fix tests
theMr17 Mar 2, 2025
d7ecb4d
Fix tests
theMr17 Mar 2, 2025
f610e90
Fix tests
theMr17 Mar 2, 2025
59fc043
Fix tests
theMr17 Mar 2, 2025
a8e10c6
Merge branch 'develop' into support-practice-questions
theMr17 Mar 11, 2025
c1d994e
Fix tests
theMr17 Mar 12, 2025
b2796b7
Fix tests
theMr17 Mar 12, 2025
af87445
Fix tests
theMr17 Mar 12, 2025
8b0154d
Merge branch 'develop' into support-practice-questions
theMr17 Mar 20, 2025
4088c53
Merge branch 'develop' into support-practice-questions
theMr17 Mar 21, 2025
bae1331
Merge branch 'develop' into support-practice-questions
theMr17 Mar 28, 2025
80bf4b2
Merge branch 'develop' into support-practice-questions
theMr17 Apr 5, 2025
176ee96
Address review comments
theMr17 Apr 9, 2025
45ee60f
Merge remote-tracking branch 'origin/support-practice-questions' into…
theMr17 Apr 10, 2025
b09e3f5
Fix tests
theMr17 Apr 10, 2025
370177a
Fix tests
theMr17 Apr 10, 2025
ba5a5fe
Fix tests
theMr17 Apr 13, 2025
ae940ac
Introduce TabConfig
theMr17 Apr 13, 2025
de66aa3
Fix static checks
theMr17 Apr 13, 2025
3edff87
Merge branch 'develop' into support-practice-questions
theMr17 Apr 13, 2025
52a131c
Introduce enable_practice_tab field
theMr17 Apr 13, 2025
94c04b3
Merge remote-tracking branch 'origin/support-practice-questions' into…
theMr17 Apr 13, 2025
e932376
Hide practice tab based on enablePracticeTab
theMr17 Apr 13, 2025
070e02e
Merge branch 'develop' into support-practice-questions
theMr17 Apr 22, 2025
fd4388e
Merge branch 'develop' into support-practice-questions
theMr17 May 6, 2025
6faf94a
Rename enable_practice_tab to has_practice_questions for non-app level
theMr17 May 8, 2025
a9b7f9b
Merge branch 'develop' into support-practice-questions
theMr17 May 17, 2025
10c1ed1
Merge branch 'develop' into support-practice-questions
theMr17 May 22, 2025
79b4de9
Refactor getTabCount function
theMr17 May 25, 2025
302f696
Merge remote-tracking branch 'origin/support-practice-questions' into…
theMr17 May 25, 2025
505167e
Update FeatureFlagNameToNumericIdConverter
theMr17 May 25, 2025
0e1006f
Merge branch 'develop' into support-practice-questions
theMr17 Jun 1, 2025
fe034ee
Merge branch 'develop' into support-practice-questions
theMr17 Jun 9, 2025
1c91582
Add global variables for info and practice tab flag to maintain consi…
theMr17 Jun 13, 2025
13b80ba
Merge remote-tracking branch 'origin/support-practice-questions' into…
theMr17 Jun 13, 2025
ca3765f
Merge branch 'develop' into support-practice-questions
theMr17 Jun 13, 2025
f72b337
Merge branch 'develop' into support-practice-questions
theMr17 Jun 13, 2025
77920b5
Merge branch 'develop' into support-practice-questions
theMr17 Aug 27, 2025
134292c
Merge branch 'develop' into support-practice-questions
theMr17 Sep 3, 2025
ee9e2b9
Address review comments
theMr17 Sep 8, 2025
d43197c
Enable practice tab on alpha builds
theMr17 Sep 8, 2025
cabedae
Merge branch 'develop' into support-practice-questions
theMr17 Sep 8, 2025
4206099
Fix a few tests
theMr17 Sep 9, 2025
b3b792a
Fix ktlint issue
theMr17 Sep 9, 2025
183908c
Fix tests
theMr17 Sep 10, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ import org.oppia.android.app.ui.R
import org.oppia.android.domain.oppialogger.OppiaLogger
import org.oppia.android.domain.oppialogger.analytics.AnalyticsController
import org.oppia.android.util.accessibility.AccessibilityService
import org.oppia.android.util.platformparameter.EnableExtraTopicTabsUi
import org.oppia.android.util.platformparameter.EnableTopicInfoTab
import org.oppia.android.util.platformparameter.EnableTopicPracticeTab
import org.oppia.android.util.platformparameter.PlatformParameterValue
import javax.inject.Inject

Expand All @@ -33,7 +34,8 @@ class TopicFragmentPresenter @Inject constructor(
private val viewModel: TopicViewModel,
private val oppiaLogger: OppiaLogger,
private val analyticsController: AnalyticsController,
@EnableExtraTopicTabsUi private val enableExtraTopicTabsUi: PlatformParameterValue<Boolean>,
@EnableTopicInfoTab private val enableTopicInfoTabFlag: PlatformParameterValue<Boolean>,
@EnableTopicPracticeTab private val enableTopicPracticeTabFlag: PlatformParameterValue<Boolean>,
private val resourceHandler: AppLanguageResourceHandler
) {
@Inject
Expand All @@ -45,6 +47,12 @@ class TopicFragmentPresenter @Inject constructor(
private lateinit var storyId: String
private lateinit var viewPager: ViewPager2

private var enableTopicInfoTab: Boolean = enableTopicInfoTabFlag.value
private var enableTopicPracticeTab: Boolean = false
set(hasPracticeQuestions) {
field = enableTopicPracticeTabFlag.value && hasPracticeQuestions
}

fun handleCreateView(
inflater: LayoutInflater,
container: ViewGroup?,
Expand Down Expand Up @@ -76,7 +84,17 @@ class TopicFragmentPresenter @Inject constructor(
viewModel.setTopicId(topicId)
binding.viewModel = viewModel

setUpViewPager(viewPager, classroomId, topicId, isConfigChanged)
viewModel.hasPracticeQuestions.observe(fragment) { hasPracticeQuestions ->
enableTopicPracticeTab = hasPracticeQuestions

setUpViewPager(
viewPager,
classroomId,
topicId,
isConfigChanged
)
}

return binding.root
}

Expand Down Expand Up @@ -120,7 +138,7 @@ class TopicFragmentPresenter @Inject constructor(
}

private fun computeTabPosition(tab: TopicTab): Int {
return if (enableExtraTopicTabsUi.value) tab.positionWithFourTabs else tab.positionWithTwoTabs
return tab.getPosition(enableTopicInfoTab, enableTopicPracticeTab)
}

private fun setUpViewPager(
Expand All @@ -136,25 +154,36 @@ class TopicFragmentPresenter @Inject constructor(
classroomId,
topicId,
storyId,
enableExtraTopicTabsUi.value
enableTopicInfoTab,
enableTopicPracticeTab
)
viewPager2.adapter = adapter
TabLayoutMediator(tabLayout, viewPager2) { tab, position ->
val topicTab = TopicTab.getTabForPosition(position, enableExtraTopicTabsUi.value)
val topicTab = TopicTab.getTabForPosition(
position,
enableTopicInfoTab,
enableTopicPracticeTab
)
tab.text = resourceHandler.getStringInLocale(topicTab.tabLabelResId)
tab.icon = ContextCompat.getDrawable(activity, topicTab.tabIconResId)
tab.contentDescription = resourceHandler.getStringInLocale(topicTab.contentDescriptionResId)
}.attach()
if (!isConfigChanged && topicId.isNotEmpty()) {
if (enableExtraTopicTabsUi.value) {
if (enableTopicInfoTab) {
setCurrentTab(if (storyId.isNotEmpty()) TopicTab.LEARN else TopicTab.INFO)
} else {
setCurrentTab(TopicTab.LEARN)
}
}
viewPager2.registerOnPageChangeCallback(object : ViewPager2.OnPageChangeCallback() {
override fun onPageSelected(position: Int) {
logTopicEvents(TopicTab.getTabForPosition(position, enableExtraTopicTabsUi.value))
logTopicEvents(
TopicTab.getTabForPosition(
position,
enableTopicInfoTab,
enableTopicPracticeTab
)
)
}
})
}
Expand Down
98 changes: 71 additions & 27 deletions app/src/main/java/org/oppia/android/app/topic/TopicTab.kt
Original file line number Diff line number Diff line change
Expand Up @@ -6,58 +6,102 @@ import org.oppia.android.app.ui.R

/** Enum to store the tabs of [TopicFragment] and get tab by position. */
enum class TopicTab(
val positionWithTwoTabs: Int,
val positionWithFourTabs: Int,
@StringRes val tabLabelResId: Int,
@DrawableRes val tabIconResId: Int,
@StringRes val contentDescriptionResId: Int
@StringRes val contentDescriptionResId: Int,
val positions: Map<TabConfig, Int>
) {
INFO(
positionWithTwoTabs = -1,
positionWithFourTabs = 0,
tabLabelResId = R.string.info,
tabIconResId = R.drawable.ic_info_icon_24dp,
contentDescriptionResId = R.string.info_tab_content_description
contentDescriptionResId = R.string.info_tab_content_description,
positions = mapOf(
TabConfig.TwoTabs to -1,
TabConfig.ThreeTabsWithInfo to 0,
TabConfig.ThreeTabsWithPractice to -1,
TabConfig.FourTabs to 0
)
),
LEARN(
positionWithTwoTabs = 0,
positionWithFourTabs = 1,
tabLabelResId = R.string.learn,
tabIconResId = R.drawable.ic_lessons_icon_24dp,
contentDescriptionResId = R.string.lessons_tab_content_description
contentDescriptionResId = R.string.lessons_tab_content_description,
positions = mapOf(
TabConfig.TwoTabs to 0,
TabConfig.ThreeTabsWithInfo to 1,
TabConfig.ThreeTabsWithPractice to 0,
TabConfig.FourTabs to 1
)
),
PRACTICE(
positionWithTwoTabs = -1,
positionWithFourTabs = 2,
tabLabelResId = R.string.practice,
tabIconResId = R.drawable.ic_practice_icon_24dp,
contentDescriptionResId = R.string.practice_tab_content_description
contentDescriptionResId = R.string.practice_tab_content_description,
positions = mapOf(
TabConfig.TwoTabs to -1,
TabConfig.ThreeTabsWithInfo to -1,
TabConfig.ThreeTabsWithPractice to 1,
TabConfig.FourTabs to 2
)
),
STUDY(
positionWithTwoTabs = 1,
positionWithFourTabs = 3,
tabLabelResId = R.string.study,
tabIconResId = R.drawable.ic_revision_icon_24dp,
contentDescriptionResId = R.string.revision_tab_content_description
contentDescriptionResId = R.string.revision_tab_content_description,
positions = mapOf(
TabConfig.TwoTabs to 1,
TabConfig.ThreeTabsWithInfo to 2,
TabConfig.ThreeTabsWithPractice to 2,
TabConfig.FourTabs to 3
)
);

/** Enum representing different tab configurations. */
enum class TabConfig {
TwoTabs,
ThreeTabsWithInfo,
ThreeTabsWithPractice,
FourTabs;

companion object {
fun getConfig(enableInfo: Boolean, enablePractice: Boolean) = when {
enableInfo && enablePractice -> FourTabs
enableInfo -> ThreeTabsWithInfo
enablePractice -> ThreeTabsWithPractice
else -> TwoTabs
}
}
}

/** Returns the tab position based on enabled tabs. */
fun getPosition(enableTopicInfoTab: Boolean, enableTopicPracticeTab: Boolean): Int {
val config = TabConfig.getConfig(enableTopicInfoTab, enableTopicPracticeTab)
return positions[config] ?: -1 // -1 indicates that the tab is not available
}

companion object {
/**
* Returns the [TopicTab] corresponding to the specified tab position, considering whether the
* info and practice tabs are enabled per [enableExtraTopicTabsUi].
* info and practice tabs are enabled per [enableTopicInfoTab] and [enableTopicPracticeTab]
* respectively.
*/
fun getTabForPosition(position: Int, enableExtraTopicTabsUi: Boolean): TopicTab {
return checkNotNull(
values().find {
position == if (enableExtraTopicTabsUi) {
it.positionWithFourTabs
} else it.positionWithTwoTabs
}
) { "No tab corresponding to position: $position" }
fun getTabForPosition(
position: Int,
enableTopicInfoTab: Boolean,
enableTopicPracticeTab: Boolean
): TopicTab {
return values().find {
it.getPosition(enableTopicInfoTab, enableTopicPracticeTab) == position
} ?: error("No tab corresponding to position: $position")
}

/** Returns the number of active tabs considering [enableExtraTopicTabsUi]. */
fun getTabCount(enableExtraTopicTabsUi: Boolean) =
if (enableExtraTopicTabsUi) values().size else values().size - 2
/**
* Returns the number of active tabs considering [enableTopicInfoTab] and
* [enableTopicPracticeTab].
*/
fun getTabCount(enableTopicInfoTab: Boolean, enableTopicPracticeTab: Boolean): Int {
val config = TabConfig.getConfig(enableTopicInfoTab, enableTopicPracticeTab)
return values().count { it.positions[config] != -1 } // -1 indicates that the tab is disabled
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ class TopicViewModel @Inject constructor(
}
}

val hasPracticeQuestions: LiveData<Boolean> by lazy {
Transformations.map(topicLiveData) { ephemeralTopic ->
ephemeralTopic.topic.hasPracticeQuestions
}
}

fun setProfileId(profileId: ProfileId) {
this.profileId = profileId
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,15 @@ class ViewPagerAdapter(
private val classroomId: String,
private val topicId: String,
private val storyId: String,
private val enableExtraTopicTabsUi: Boolean
private val enableTopicInfoTab: Boolean,
private val enableTopicPracticeTab: Boolean
) : FragmentStateAdapter(fragment) {

override fun getItemCount(): Int = TopicTab.getTabCount(enableExtraTopicTabsUi)
override fun getItemCount(): Int =
TopicTab.getTabCount(enableTopicInfoTab, enableTopicPracticeTab)

override fun createFragment(position: Int): Fragment {
return when (TopicTab.getTabForPosition(position, enableExtraTopicTabsUi)) {
return when (TopicTab.getTabForPosition(position, enableTopicInfoTab, enableTopicPracticeTab)) {
TopicTab.INFO -> {
TopicInfoFragment.newInstance(profileId, topicId)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ class FeatureFlagsFragmentTest {

// Note to developers: if you add/remove a feature flag, please update the expected count.
onView(withId(R.id.feature_flags_recycler_view))
.check(RecyclerViewMatcher.hasItemCount(count = 14))
.check(RecyclerViewMatcher.hasItemCount(count = 15))
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ 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.EnableExtraTopicTabsUi
import org.oppia.android.util.platformparameter.EnableTopicInfoTab
import org.oppia.android.util.platformparameter.EnableTopicPracticeTab
import org.oppia.android.util.platformparameter.PlatformParameterValue
import org.robolectric.annotation.Config
import org.robolectric.annotation.LooperMode
Expand All @@ -116,12 +117,16 @@ class TopicTestActivityForStoryTest {
@Inject
lateinit var testCoroutineDispatchers: TestCoroutineDispatchers

@field:[Inject EnableExtraTopicTabsUi]
lateinit var enableExtraTopicTabsUiValue: PlatformParameterValue<Boolean>
@field:[Inject EnableTopicInfoTab]
lateinit var enableTopicInfoTab: PlatformParameterValue<Boolean>

@field:[Inject EnableTopicPracticeTab]
lateinit var enableTopicPracticeTab: PlatformParameterValue<Boolean>

@Before
fun setUp() {
TestPlatformParameterModule.forceEnableExtraTopicTabsUi(true)
TestPlatformParameterModule.forceEnableTopicInfoTab(true)
TestPlatformParameterModule.forceEnableTopicPracticeTab(true)
setUpTestApplicationComponent()
testCoroutineDispatchers.registerIdlingResource()
}
Expand All @@ -145,7 +150,8 @@ class TopicTestActivityForStoryTest {
matchCurrentTabTitle(
TopicTab.getTabForPosition(
position = 1,
enableExtraTopicTabsUiValue.value
enableTopicInfoTab.value,
enableTopicPracticeTab.value
).name
)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,8 @@ class TopicActivityTest {

@Test
fun testTopicActivity_hasCorrectActivityLabel() {
TestPlatformParameterModule.forceEnableExtraTopicTabsUi(true)
TestPlatformParameterModule.forceEnableTopicInfoTab(true)
TestPlatformParameterModule.forceEnableTopicPracticeTab(true)
launchTopicActivity(
profileId, TEST_CLASSROOM_ID_1, FRACTIONS_TOPIC_ID
).use { scenario ->
Expand All @@ -187,7 +188,8 @@ class TopicActivityTest {
@Test
@RunOn(TestPlatform.ROBOLECTRIC) // TODO(#3858): Enable for Espresso.
fun testTopicActivity_startPracticeSession_questionActivityStartedWithProfileId() {
TestPlatformParameterModule.forceEnableExtraTopicTabsUi(true)
TestPlatformParameterModule.forceEnableTopicInfoTab(true)
TestPlatformParameterModule.forceEnableTopicPracticeTab(true)
Comment on lines +191 to +192
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this test work with only the forceEnableTopicPracticeTab? It looks like it is only testing the practice tab.

Copy link
Collaborator Author

@theMr17 theMr17 Apr 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that should work, removed TestPlatformParameterModule.forceEnableTopicInfoTab(true)

Edit: We need to enable info tab as well since we are checking for the topic_name_text_view (present in topic_name_text_view.xml) in the launchTopicActivity() function.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You response is not clear with regards to my original question. Is the visibility of topic_name_text_view gated behind both flags? why?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are in the practice session, the status of the info tab should be irrelevant. Otherwise it indicates a problem with the implementation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You response is not clear with regards to my original question. Is the visibility of topic_name_text_view gated behind both flags? why?

The topic_name_text_view is gated behind ENABLE_TOPIC_INFO_TAB only. Note that topic_name_text_view is a text element inside the info tab. The test launches the TopicActivity and expects the info tab to be viewed. Then clicks on the practice tab and navigates away from the info tab to the practice tab.

onView(withText("Practice")).perform(click())

Considering the above setup of the test, we need to have both the feature flags enabled.

Copy link
Collaborator

@subhajitxyz subhajitxyz Sep 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are using separate feature flag for info tab and practice tab. I think we should remove this line onView(withId(R.id.topic_name_text_view)).check(matches(isDisplayed())) from fun launchTopicActivity. So that we can properly test with both feature flag on and off.

launchTopicActivity(profileId, TEST_CLASSROOM_ID_1, FRACTIONS_TOPIC_ID).use {
// Open the practice tab and select a skill.
onView(withText("Practice")).perform(click())
Expand Down
Loading
Loading