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.

Sign up for GitHub

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 on selection #4481

Closed
wants to merge 4 commits into from
Closed

Fixes #3169: Item selection interaction text change on selection #4481

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
09ae729
initial commit
Akshatkamboj14 Jul 31, 2022
f67fd8d
Merge branch 'develop' of https://github.com/oppia/oppia-android into…
Akshatkamboj14 Aug 2, 2022
96516f8
text updated upon selection
Akshatkamboj14 Aug 2, 2022
dedc527
updated indentation
Akshatkamboj14 Aug 2, 2022
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
15 changes: 14 additions & 1 deletion ...in/java/org/oppia/android/app/player/state/itemviewmodel/SelectionInteractionViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class SelectionInteractionViewModel private constructor(
computeChoiceItems(choiceSubtitledHtmls, hasConversationView, this)

private val isAnswerAvailable = ObservableField(false)

val selectedItemText = ObservableField("Please select atleast one choice.")
Copy link
Member

Choose a reason for hiding this comment

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

We need to use string resources for these strings, instead, since they need to be translatable.

init {
val callback: Observable.OnPropertyChangedCallback =
object : Observable.OnPropertyChangedCallback() {
Expand Down Expand Up @@ -138,6 +138,19 @@ class SelectionInteractionViewModel private constructor(
// TODO(#3624): Add warning to user when they exceed the number of allowable selections or are under the minimum
// number required.
selectedItems += itemIndex
selectedItemText.set("You may select more choices")
Copy link
Member

Choose a reason for hiding this comment

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

Could this be logically moved into updateSelectionText to avoid needing to set the item text multiple times here? It may also help with robustness since it ensures that updateSelectionText always uses the most correct text.

updateIsAnswerAvailable()
true
}
selectedItems.size == 0 -> {
selectedItems += itemIndex
selectedItemText.set("Please select one or more choices.")
updateIsAnswerAvailable()
true
}
selectedItems.size == maxAllowableSelectionCount -> {
selectedItems += itemIndex
selectedItemText.set("No more than $maxAllowableSelectionCount choices may be selected.")
updateIsAnswerAvailable()
true
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Create a separate function which accepts size and maxAllowableSelectionCount which you can use to set the correct string.

Now decide where do want to use that function.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, @rt4914 I have pushed the same, PTAL.

Expand Down
2 changes: 1 addition & 1 deletion app/src/main/res/layout/selection_interaction_item.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,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/oppia_primary_text"
android:textSize="14sp"
android:textStyle="italic"
Expand Down