Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #3169: Item selection interaction text change and checkboxes should be disabled on selection #4777

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
d8ac261
initial commit
Akshatkamboj14 Nov 30, 2022
d9fb426
corrected ktlint issues
Akshatkamboj14 Nov 30, 2022
469c523
corrected ktlint issues
Akshatkamboj14 Nov 30, 2022
bc78baf
correcting error
Akshatkamboj14 Dec 1, 2022
3700c5c
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
Akshatkamboj14 Dec 6, 2022
81dbc16
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
Akshatkamboj14 Dec 9, 2022
7d76b55
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
Akshatkamboj14 Dec 12, 2022
4b2817f
loop for the enabled items
Akshatkamboj14 Dec 16, 2022
ea472ab
loop for the enabled items
Akshatkamboj14 Dec 16, 2022
6794495
Merge branch 'develop' into ItemSelectionInteraction-text-change-and-…
BenHenning Dec 23, 2022
99bf73c
Ensure items disable when hitting max.
BenHenning Dec 23, 2022
3568c27
Merge pull request #1 from BenHenning/suggested-fixes-for-4777
Akshatkamboj14 Jan 3, 2023
ee7eb03
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
Akshatkamboj14 Jan 3, 2023
16f4d35
Merge branch 'develop' into ItemSelectionInteraction-text-change-and-…
Akshatkamboj14 Jan 3, 2023
b1a525f
Merge branch 'ItemSelectionInteraction-text-change-and-checkboxes-sho…
Akshatkamboj14 Jan 6, 2023
e8afa83
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
Akshatkamboj14 Jan 8, 2023
f2719dc
added instrumentation tests
Akshatkamboj14 Jan 12, 2023
c443862
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
Akshatkamboj14 Jan 12, 2023
ea2a05f
corrected k-lint issues
Akshatkamboj14 Jan 12, 2023
5830d25
added instrumentation tests
Akshatkamboj14 Jan 12, 2023
90173c6
merged-with-develop
Akshatkamboj14 Jan 14, 2023
d7382aa
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
Akshatkamboj14 Jan 17, 2023
d126277
added the previous enabled color
Akshatkamboj14 Jan 21, 2023
5181714
added AppCompatBindingAdapterTest
Akshatkamboj14 Jan 22, 2023
bcc9339
lint issue
Akshatkamboj14 Jan 22, 2023
10a7337
syntax error
Akshatkamboj14 Jan 22, 2023
0cad374
lint issue
Akshatkamboj14 Jan 22, 2023
c47ee56
lint issue
Akshatkamboj14 Jan 22, 2023
f288ffb
added AppCompatCheckBoxBindingAdaptersTestActivity into test_file_exe…
Akshatkamboj14 Jan 23, 2023
8a92cc5
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
Akshatkamboj14 Feb 7, 2023
299f0bf
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
Akshatkamboj14 Feb 14, 2023
1bb78f6
corrected failing test
Akshatkamboj14 Feb 15, 2023
18a424a
corrected ci check
Akshatkamboj14 Feb 17, 2023
a9a3e26
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
Akshatkamboj14 Feb 21, 2023
e534511
added requested changes
Akshatkamboj14 Feb 24, 2023
c5ce0a5
solved k-lint issues
Akshatkamboj14 Feb 24, 2023
f5fedea
solved k-lint issues
Akshatkamboj14 Feb 24, 2023
a1e46bd
added requested changes
Akshatkamboj14 Feb 24, 2023
8dfdaaf
solved k-lint issues
Akshatkamboj14 Feb 24, 2023
a849a53
fixes issue
Akshatkamboj14 Feb 24, 2023
2cb0d53
uncommented line
Akshatkamboj14 Feb 25, 2023
80c435a
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
Akshatkamboj14 Feb 25, 2023
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
3 changes: 2 additions & 1 deletion app/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ VIEW_MODELS_WITH_RESOURCE_IMPORTS = [
"src/main/java/org/oppia/android/app/player/state/itemviewmodel/MathExpressionInteractionsViewModel.kt",
"src/main/java/org/oppia/android/app/player/state/itemviewmodel/PreviousResponsesHeaderViewModel.kt",
"src/main/java/org/oppia/android/app/player/state/itemviewmodel/RatioExpressionInputInteractionViewModel.kt",
"src/main/java/org/oppia/android/app/player/state/itemviewmodel/SelectionInteractionViewModel.kt",
"src/main/java/org/oppia/android/app/player/state/itemviewmodel/SubmittedAnswerViewModel.kt",
"src/main/java/org/oppia/android/app/player/state/itemviewmodel/TextInputViewModel.kt",
"src/main/java/org/oppia/android/app/profile/AddProfileViewModel.kt",
Expand Down Expand Up @@ -316,7 +317,6 @@ VIEW_MODELS = [
"src/main/java/org/oppia/android/app/player/state/itemviewmodel/ReplayButtonViewModel.kt",
"src/main/java/org/oppia/android/app/player/state/itemviewmodel/ReturnToTopicButtonViewModel.kt",
"src/main/java/org/oppia/android/app/player/state/itemviewmodel/SelectionInteractionContentViewModel.kt",
"src/main/java/org/oppia/android/app/player/state/itemviewmodel/SelectionInteractionViewModel.kt",
"src/main/java/org/oppia/android/app/player/state/itemviewmodel/StateItemViewModel.kt",
"src/main/java/org/oppia/android/app/player/state/itemviewmodel/SubmitButtonViewModel.kt",
"src/main/java/org/oppia/android/app/player/state/StateViewModel.kt",
Expand Down Expand Up @@ -469,6 +469,7 @@ BINDING_ADAPTERS_WITH_RESOURCE_IMPORTS = [

# keep sorted
BINDING_ADAPTERS = [
"src/main/java/org/oppia/android/app/databinding/AppCompatCheckBoxBindingAdapters.java",
"src/main/java/org/oppia/android/app/databinding/ConstraintLayoutAdapters.java",
"src/main/java/org/oppia/android/app/databinding/EditTextBindingAdapters.java",
"src/main/java/org/oppia/android/app/databinding/GuidelineBindingAdapters.java",
Expand Down
1 change: 1 addition & 0 deletions app/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@
android:name=".app.story.StoryActivity"
android:label="@string/story_activity_title"
android:theme="@style/OppiaThemeWithoutActionBar" />
<activity android:name=".app.testing.AppCompatCheckBoxBindingAdaptersTestActivity" />
<activity android:name=".app.testing.AudioFragmentTestActivity" />
<activity android:name=".app.testing.BindableAdapterTestActivity" />
<activity android:name=".app.testing.ConceptCardFragmentTestActivity" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ import org.oppia.android.app.settings.profile.ProfileResetPinActivity
import org.oppia.android.app.splash.SplashActivity
import org.oppia.android.app.story.StoryActivity
import org.oppia.android.app.testing.AdministratorControlsFragmentTestActivity
import org.oppia.android.app.testing.AppCompatCheckBoxBindingAdaptersTestActivity
import org.oppia.android.app.testing.AudioFragmentTestActivity
import org.oppia.android.app.testing.ConceptCardFragmentTestActivity
import org.oppia.android.app.testing.DragDropTestActivity
Expand Down Expand Up @@ -114,6 +115,10 @@ interface ActivityComponentImpl :
fun inject(administratorControlsActivity: AdministratorControlsActivity)
fun inject(administratorControlsFragmentTestActivity: AdministratorControlsFragmentTestActivity)
fun inject(adminPinActivity: AdminPinActivity)
fun inject(
appCompatCheckBoxBindingAdaptersTestActivity:
AppCompatCheckBoxBindingAdaptersTestActivity
)
fun inject(appLanguageActivity: AppLanguageActivity)
fun inject(appVersionActivity: AppVersionActivity)
fun inject(audioFragmentTestActivity: AudioFragmentTestActivity)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package org.oppia.android.app.databinding;

import android.content.res.ColorStateList;
import androidx.annotation.ColorInt;
import androidx.annotation.NonNull;
import androidx.appcompat.widget.AppCompatCheckBox;
import androidx.databinding.BindingAdapter;

/**
* Custom data-binding adapters for {@link AppCompatCheckBox}s.
*/
BenHenning marked this conversation as resolved.
Show resolved Hide resolved
public final class AppCompatCheckBoxBindingAdapters {
/** Sets the button tint for the specified checkbox, via data-binding. */
@BindingAdapter("app:buttonTint")
public static void setButtonTint(@NonNull AppCompatCheckBox checkBox, @ColorInt int colorRgb) {
checkBox.setSupportButtonTintList(ColorStateList.valueOf(colorRgb));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ class SelectionInteractionContentViewModel(
val htmlContent: SubtitledHtml,
val hasConversationView: Boolean,
private val itemIndex: Int,
private val selectionInteractionViewModel: SelectionInteractionViewModel
private val selectionInteractionViewModel: SelectionInteractionViewModel,
val isEnabled: ObservableBoolean
) : ObservableViewModel() {
var isAnswerSelected = ObservableBoolean()

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package org.oppia.android.app.player.state.itemviewmodel

import androidx.databinding.Observable
import androidx.databinding.ObservableBoolean
import androidx.databinding.ObservableField
import androidx.databinding.ObservableList
import org.oppia.android.R
import org.oppia.android.app.model.Interaction
import org.oppia.android.app.model.InteractionObject
import org.oppia.android.app.model.SetOfTranslatableHtmlContentIds
Expand All @@ -13,6 +15,7 @@ import org.oppia.android.app.model.WrittenTranslationContext
import org.oppia.android.app.player.state.answerhandling.InteractionAnswerErrorOrAvailabilityCheckReceiver
import org.oppia.android.app.player.state.answerhandling.InteractionAnswerHandler
import org.oppia.android.app.player.state.answerhandling.InteractionAnswerReceiver
import org.oppia.android.app.translation.AppLanguageResourceHandler
import org.oppia.android.app.viewmodel.ObservableArrayList
import org.oppia.android.domain.translation.TranslationController
import javax.inject.Inject
Expand All @@ -31,7 +34,8 @@ class SelectionInteractionViewModel private constructor(
private val interactionAnswerErrorOrAvailabilityCheckReceiver: InteractionAnswerErrorOrAvailabilityCheckReceiver, // ktlint-disable max-line-length
val isSplitView: Boolean,
val writtenTranslationContext: WrittenTranslationContext,
private val translationController: TranslationController
private val translationController: TranslationController,
private val resourceHandler: AppLanguageResourceHandler
) : StateItemViewModel(ViewType.SELECTION_INTERACTION), InteractionAnswerHandler {
private val interactionId: String = interaction.id

Expand All @@ -52,10 +56,21 @@ class SelectionInteractionViewModel private constructor(
?: minAllowableSelectionCount
}
private val selectedItems: MutableList<Int> = mutableListOf()
private val enabledItemsList by lazy {
List(choiceSubtitledHtmls.size) {
ObservableBoolean(true)
}
}
val choiceItems: ObservableList<SelectionInteractionContentViewModel> =
computeChoiceItems(choiceSubtitledHtmls, hasConversationView, this)
computeChoiceItems(choiceSubtitledHtmls, hasConversationView, this, enabledItemsList)

private val isAnswerAvailable = ObservableField(false)
val selectedItemText =
ObservableField(
resourceHandler.getStringInLocale(
R.string.state_fragment_item_selection_no_items_selected_hint_text
)
)

init {
val callback: Observable.OnPropertyChangedCallback =
Expand Down Expand Up @@ -124,21 +139,23 @@ class SelectionInteractionViewModel private constructor(
isCurrentlySelected -> {
selectedItems -= itemIndex
updateIsAnswerAvailable()
updateSelectionText()
updateItemSelectability()
false
}
!areCheckboxesBound() -> {
// Disable all items to simulate a radio button group.
// De-select all other items to simulate a radio button group.
choiceItems.forEach { item -> item.isAnswerSelected.set(false) }
selectedItems.clear()
selectedItems += itemIndex
updateIsAnswerAvailable()
true
}
selectedItems.size < maxAllowableSelectionCount -> {
// TODO(#3624): Add warning to user when they exceed the number of allowable selections or are under the minimum
// number required.
selectedItems += itemIndex
updateIsAnswerAvailable()
updateSelectionText()
updateItemSelectability()
true
}
else -> {
Expand All @@ -148,6 +165,38 @@ class SelectionInteractionViewModel private constructor(
}
}

private fun updateSelectionText() {
if (selectedItems.size < maxAllowableSelectionCount) {
selectedItemText.set(
resourceHandler.getStringInLocale(
R.string.state_fragment_item_selection_some_items_selected_hint_text
)
)
}
if (selectedItems.size == 0) {
selectedItemText.set(
resourceHandler.getStringInLocale(
R.string.state_fragment_item_selection_no_items_selected_hint_text
)
)
}
if (selectedItems.size == maxAllowableSelectionCount) {
selectedItemText.set(
resourceHandler.getStringInLocaleWithWrapping(
R.string.state_fragment_item_selection_max_items_selected_hint_text,
maxAllowableSelectionCount.toString()
)
)
}
}

private fun updateItemSelectability() {
if (selectedItems.size == maxAllowableSelectionCount) {
// All non-selected items should be disabled when the limit is reached.
enabledItemsList.filterIndexed { idx, _ -> idx !in selectedItems }.forEach { it.set(false) }
} else enabledItemsList.forEach { it.set(true) } // Otherwise, all items are available.
}

private fun areCheckboxesBound(): Boolean {
return interactionId == "ItemSelectionInput" && maxAllowableSelectionCount > 1
}
Expand All @@ -161,7 +210,8 @@ class SelectionInteractionViewModel private constructor(

/** Implementation of [StateItemViewModel.InteractionItemFactory] for this view model. */
class FactoryImpl @Inject constructor(
private val translationController: TranslationController
private val translationController: TranslationController,
private val resourceHandler: AppLanguageResourceHandler
) : InteractionItemFactory {
override fun create(
entityId: String,
Expand All @@ -181,7 +231,8 @@ class SelectionInteractionViewModel private constructor(
answerErrorReceiver,
isSplitView,
writtenTranslationContext,
translationController
translationController,
resourceHandler
)
}
}
Expand All @@ -190,15 +241,17 @@ class SelectionInteractionViewModel private constructor(
private fun computeChoiceItems(
choiceSubtitledHtmls: List<SubtitledHtml>,
hasConversationView: Boolean,
selectionInteractionViewModel: SelectionInteractionViewModel
selectionInteractionViewModel: SelectionInteractionViewModel,
enabledItemsList: List<ObservableBoolean>
): ObservableArrayList<SelectionInteractionContentViewModel> {
val observableList = ObservableArrayList<SelectionInteractionContentViewModel>()
observableList += choiceSubtitledHtmls.mapIndexed { index, subtitledHtml ->
SelectionInteractionContentViewModel(
htmlContent = subtitledHtml,
hasConversationView = hasConversationView,
itemIndex = index,
selectionInteractionViewModel = selectionInteractionViewModel
selectionInteractionViewModel = selectionInteractionViewModel,
isEnabled = enabledItemsList[index]
)
}
return observableList
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package org.oppia.android.app.testing

import android.os.Bundle
import org.oppia.android.R
import org.oppia.android.app.activity.ActivityComponentImpl
import org.oppia.android.app.activity.InjectableAppCompatActivity

/** Test activity for [org.oppia.android.app.databinding.AppCompatCheckBoxBindingAdapters]. */
class AppCompatCheckBoxBindingAdaptersTestActivity : InjectableAppCompatActivity() {
override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
(activityComponent as ActivityComponentImpl).inject(this)
setContentView(R.layout.test_app_compat_check_box_bindable_adapter_activity)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,10 @@
android:layout_height="wrap_content"
android:checked="@{viewModel.answerSelected}"
android:clickable="false"
android:enabled="@{viewModel.isEnabled}"
android:focusable="false"
android:labelFor="@id/item_selection_contents_text_view"
app:buttonTint="@color/shared_interaction_selector" />
app:buttonTint="@{viewModel.isEnabled ? @color/component_color_shared_item_selection_interaction_enabled_color : @color/component_color_shared_item_selection_interaction_disabled_color}" />

<TextView
android:id="@+id/item_selection_contents_text_view"
Expand All @@ -43,7 +44,7 @@
android:layout_toEndOf="@+id/item_selection_checkbox"
android:fontFamily="sans-serif"
android:text="@{htmlContent}"
android:textColor="@{viewModel.isAnswerSelected() ? @color/component_color_shared_selection_interaction_selected_text_color : @color/component_color_shared_selection_interaction_unselected_text_color}"
android:textColor="@{viewModel.isEnabled ? @color/component_color_shared_item_selection_interaction_enabled_color : @color/component_color_shared_item_selection_interaction_disabled_color}"
android:textSize="16sp" />
</RelativeLayout>
</layout>
2 changes: 1 addition & 1 deletion app/src/main/res/layout/selection_interaction_item.xml
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
android:layout_marginTop="4dp"
android:layout_marginBottom="4dp"
android:fontFamily="sans-serif-light"
android:text="@string/item_selection_text"
android:text="@{viewModel.selectedItemText}"
android:textColor="@color/component_color_shared_primary_text_color"
android:textSize="14sp"
android:textStyle="italic"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?xml version="1.0" encoding="utf-8"?>
<layout xmlns:android="http://schemas.android.com/apk/res/android">

<androidx.appcompat.widget.AppCompatCheckBox
android:id="@+id/test_check_box"
android:layout_width="wrap_content"
android:layout_height="wrap_content" />
</layout>
2 changes: 2 additions & 0 deletions app/src/main/res/values/component_colors.xml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@
<color name="component_color_shared_item_selection_interaction_unselected_color">@color/color_palette_item_selection_unselected_color</color>
<color name="component_color_shared_selection_interaction_selected_text_color">@color/color_palette_item_selection_unselected_text_color</color>
<color name="component_color_shared_selection_interaction_unselected_text_color">@color/color_palette_item_selection_selected_text_color</color>
<color name="component_color_shared_item_selection_interaction_enabled_color">@color/color_palette_item_selection_selected_text_color</color>
<color name="component_color_shared_item_selection_interaction_disabled_color">@color/color_palette_disabled_button_background_color</color>
<color name="component_color_shared_back_forward_arrow_button_color">@color/color_palette_back_forward_arrow_button_color</color>
<color name="component_color_shared_replay_button_color">@color/color_palette_replay_button_color</color>
<color name="component_color_shared_progress_bar_solid_color">@color/color_palette_progress_bar_solid_color</color>
Expand Down
3 changes: 3 additions & 0 deletions app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -584,4 +584,7 @@
<string name="administrator_controls_fragment_test_activity_label">Administrator Controls Fragment Test Activity</string>
<string name="revision_navigation_cards_header">Continue Studying</string>
<string name="state_fragment_hint_bar_forced_announcement_text">Go to the bottom of the screen for a hint.</string>
<string name="state_fragment_item_selection_no_items_selected_hint_text">Please select all correct choices.</string>
<string name="state_fragment_item_selection_some_items_selected_hint_text">You may select more choices.</string>
<string name="state_fragment_item_selection_max_items_selected_hint_text">No more than %s choices may be selected.</string>
</resources>
Loading