Skip to content

Commit

Permalink
Fix #4450: Add next and previous card options on revision screen (#4554)
Browse files Browse the repository at this point in the history
* add functionality for next/prev navigation for revision cards

* update according to changes from develop

* fix bad merge with develop

* make toolbar orange

* refactor code + add tests

* lint/shorten test name

* lint/shorten test name

* add all refactor jobs

* requested changes

* add 'scroll to' to tests

* add min height and width

* remove accidentally added code that led to test failure

* pre-load subtopic list size

* lint

* piping was not correct

* lint

* lint

* disable accessibility checks

* finish up small left bits

* lint

* commit before checkout

* partial: Add changes requested in review.

* lint

* partial: suggested changes

* kt to binding

* kt to xml binding

* change colors of status bar and toolbar color without using logic

* lint

* fix todo spacing

* wrap up with review comments and fix CI

* lint

* text alignment should be right when only next card is visible

* add kdocs

* finish up kdocs
  • Loading branch information
JishnuGoyal authored Oct 3, 2022
1 parent 87f0e98 commit f096b1d
Show file tree
Hide file tree
Showing 25 changed files with 670 additions and 139 deletions.
2 changes: 1 addition & 1 deletion app/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@
<activity
android:name=".app.topic.revisioncard.RevisionCardActivity"
android:label="@string/revision_card_activity_title"
android:theme="@style/OppiaThemeWithoutActionBar" />
android:theme="@style/RevisionCardActivityTheme" />
<activity android:name=".app.testing.SplashTestActivity" />
<activity
android:name=".app.topic.TopicActivity"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,11 @@ class LessonThumbnailImageView @JvmOverloads constructor(
checkIfLoadingIsPossible()
}

fun setLessonThumbnail(lessonThumbnail: LessonThumbnail) {
this.lessonThumbnail = lessonThumbnail
checkIfLoadingIsPossible()
fun setLessonThumbnail(lessonThumbnail: LessonThumbnail?) {
if (lessonThumbnail != null) {
this.lessonThumbnail = lessonThumbnail
checkIfLoadingIsPossible()
}
}

fun setIsBlurred(isBlurred: Boolean) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,19 @@ class TopicRevisionTestActivity : InjectableAppCompatActivity(), RouteToRevision
topicRevisionTestActivityPresenter.handleOnCreate()
}

override fun routeToRevisionCard(internalProfileId: Int, topicId: String, subtopicId: Int) {
override fun routeToRevisionCard(
internalProfileId: Int,
topicId: String,
subtopicId: Int,
subtopicListSize: Int
) {
startActivity(
RevisionCardActivity.createRevisionCardActivityIntent(
this,
internalProfileId,
topicId,
subtopicId
subtopicId,
subtopicListSize
)
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,15 @@ class TopicTestActivity :
)
}

override fun routeToRevisionCard(internalProfileId: Int, topicId: String, subtopicId: Int) {
override fun routeToRevisionCard(
internalProfileId: Int,
topicId: String,
subtopicId: Int,
subtopicListSize: Int
) {
startActivity(
RevisionCardActivity.createRevisionCardActivityIntent(
this, internalProfileId, topicId, subtopicId
this, internalProfileId, topicId, subtopicId, subtopicListSize
)
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,15 @@ class TopicTestActivityForStory :
)
}

override fun routeToRevisionCard(internalProfileId: Int, topicId: String, subtopicId: Int) {
override fun routeToRevisionCard(
internalProfileId: Int,
topicId: String,
subtopicId: Int,
subtopicListSize: Int
) {
startActivity(
RevisionCardActivity.createRevisionCardActivityIntent(
this, internalProfileId, topicId, subtopicId
this, internalProfileId, topicId, subtopicId, subtopicListSize
)
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,10 @@ package org.oppia.android.app.topic

/** Listener for when an [TopicActivity] should route to a [RevisionCardFragment]. */
interface RouteToRevisionCardListener {
fun routeToRevisionCard(internalProfileId: Int, topicId: String, subtopicId: Int)
fun routeToRevisionCard(
internalProfileId: Int,
topicId: String,
subtopicId: Int,
subtopicListSize: Int
)
}
10 changes: 8 additions & 2 deletions app/src/main/java/org/oppia/android/app/topic/TopicActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,19 @@ class TopicActivity :
)
}

override fun routeToRevisionCard(internalProfileId: Int, topicId: String, subtopicId: Int) {
override fun routeToRevisionCard(
internalProfileId: Int,
topicId: String,
subtopicId: Int,
subtopicListSize: Int
) {
startActivity(
RevisionCardActivity.createRevisionCardActivityIntent(
this,
internalProfileId,
topicId,
subtopicId
subtopicId,
subtopicListSize
)
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class TopicRevisionFragmentPresenter @Inject constructor(
private var internalProfileId: Int = -1
private lateinit var topicId: String
private val routeToReviewListener = activity as RouteToRevisionCardListener
private var subtopicListSize: Int? = null

fun handleCreateView(
inflater: LayoutInflater,
Expand Down Expand Up @@ -55,11 +56,20 @@ class TopicRevisionFragmentPresenter @Inject constructor(
this.viewModel = this@TopicRevisionFragmentPresenter.viewModel
lifecycleOwner = fragment
}

viewModel.subtopicLiveData.observe(fragment) {
this.subtopicListSize = it.size
}
return binding.root
}

override fun onTopicRevisionSummaryClicked(subtopic: Subtopic) {
routeToReviewListener.routeToRevisionCard(internalProfileId, topicId, subtopic.subtopicId)
routeToReviewListener.routeToRevisionCard(
internalProfileId,
topicId,
subtopic.subtopicId,
checkNotNull(subtopicListSize) { "Subtopic list size not found." }
)
}

private fun createRecyclerViewAdapter(): BindableAdapter<TopicRevisionItemViewModel> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import org.oppia.android.app.activity.ActivityComponentImpl
import org.oppia.android.app.activity.InjectableAppCompatActivity
import org.oppia.android.app.model.ScreenName.REVISION_CARD_ACTIVITY
import org.oppia.android.app.player.exploration.BottomSheetOptionsMenuItemClickListener
import org.oppia.android.app.topic.RouteToRevisionCardListener
import org.oppia.android.app.topic.conceptcard.ConceptCardListener
import org.oppia.android.util.logging.CurrentAppScreenNameIntentDecorator.decorateWithScreenName
import javax.inject.Inject
Expand All @@ -16,6 +17,7 @@ class RevisionCardActivity :
InjectableAppCompatActivity(),
ReturnToTopicClickListener,
ConceptCardListener,
RouteToRevisionCardListener,
BottomSheetOptionsMenuItemClickListener {

@Inject
Expand All @@ -30,7 +32,13 @@ class RevisionCardActivity :
"Expected topic ID to be included in intent for RevisionCardActivity."
}
val subtopicId = intent.getIntExtra(SUBTOPIC_ID_EXTRA_KEY, -1)
revisionCardActivityPresenter.handleOnCreate(internalProfileId, topicId, subtopicId)
val subtopicListSize = intent.getIntExtra(SUBTOPIC_LIST_SIZE_EXTRA_KEY, -1)
revisionCardActivityPresenter.handleOnCreate(
internalProfileId,
topicId,
subtopicId,
subtopicListSize
)
}
}

Expand All @@ -42,23 +50,44 @@ class RevisionCardActivity :
internal const val INTERNAL_PROFILE_ID_EXTRA_KEY = "RevisionCardActivity.internal_profile_id"
internal const val TOPIC_ID_EXTRA_KEY = "RevisionCardActivity.topic_id"
internal const val SUBTOPIC_ID_EXTRA_KEY = "RevisionCardActivity.subtopic_id"
internal const val SUBTOPIC_LIST_SIZE_EXTRA_KEY = "RevisionCardActivity.subtopic_list_size"

/** Returns a new [Intent] to route to [RevisionCardActivity]. */
fun createRevisionCardActivityIntent(
context: Context,
internalProfileId: Int,
topicId: String,
subtopicId: Int
subtopicId: Int,
subtopicListSize: Int
): Intent {
return Intent(context, RevisionCardActivity::class.java).apply {
putExtra(INTERNAL_PROFILE_ID_EXTRA_KEY, internalProfileId)
putExtra(TOPIC_ID_EXTRA_KEY, topicId)
putExtra(SUBTOPIC_ID_EXTRA_KEY, subtopicId)
putExtra(SUBTOPIC_LIST_SIZE_EXTRA_KEY, subtopicListSize)
decorateWithScreenName(REVISION_CARD_ACTIVITY)
}
}
}

override fun routeToRevisionCard(
internalProfileId: Int,
topicId: String,
subtopicId: Int,
subtopicListSize: Int
) {
startActivity(
createRevisionCardActivityIntent(
this,
internalProfileId,
topicId,
subtopicId,
subtopicListSize
)
)
this.finish()
}

override fun onReturnToTopicClicked() {
onBackPressed()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,12 @@ class RevisionCardActivityPresenter @Inject constructor(
private lateinit var topicId: String
private var subtopicId: Int = 0

fun handleOnCreate(internalProfileId: Int, topicId: String, subtopicId: Int) {
fun handleOnCreate(
internalProfileId: Int,
topicId: String,
subtopicId: Int,
subtopicListSize: Int
) {
val binding = DataBindingUtil.setContentView<RevisionCardActivityBinding>(
activity,
R.layout.revision_card_activity
Expand Down Expand Up @@ -72,7 +77,7 @@ class RevisionCardActivityPresenter @Inject constructor(
if (getReviewCardFragment() == null) {
activity.supportFragmentManager.beginTransaction().add(
R.id.revision_card_fragment_placeholder,
RevisionCardFragment.newInstance(topicId, subtopicId, profileId)
RevisionCardFragment.newInstance(topicId, subtopicId, profileId, subtopicListSize)
).commitNow()
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,24 @@ class RevisionCardFragment : InjectableDialogFragment() {
companion object {
private const val TOPIC_ID_ARGUMENT_KEY = "RevisionCardFragment.topic_id"
private const val SUBTOPIC_ID_ARGUMENT_KEY = "RevisionCardFragment.subtopic_id"
private const val SUBTOPIC_LIST_SIZE_ARGUMENT_KEY = "RevisionCardFragment.subtopic_list_size"
private const val PROFILE_ID_ARGUMENT_KEY = "RevisionCardFragment.profile_id"

/**
* Returns a new [RevisionCardFragment] to display the specific subtopic for the given topic &
* profile.
*/
fun newInstance(topicId: String, subtopicId: Int, profileId: ProfileId): RevisionCardFragment {
return RevisionCardFragment().apply {
arguments = Bundle().apply {
putString(TOPIC_ID_ARGUMENT_KEY, topicId)
putInt(SUBTOPIC_ID_ARGUMENT_KEY, subtopicId)
putProto(PROFILE_ID_ARGUMENT_KEY, profileId)
fun newInstance(topicId: String, subtopicId: Int, profileId: ProfileId, subtopicListSize: Int):
RevisionCardFragment {
return RevisionCardFragment().apply {
arguments = Bundle().apply {
putString(TOPIC_ID_ARGUMENT_KEY, topicId)
putInt(SUBTOPIC_ID_ARGUMENT_KEY, subtopicId)
putProto(PROFILE_ID_ARGUMENT_KEY, profileId)
putInt(SUBTOPIC_LIST_SIZE_ARGUMENT_KEY, subtopicListSize)
}
}
}
}
}

@Inject
Expand All @@ -58,8 +61,9 @@ class RevisionCardFragment : InjectableDialogFragment() {
}
val subtopicId = args.getInt(SUBTOPIC_ID_ARGUMENT_KEY, -1)
val profileId = args.getProto(PROFILE_ID_ARGUMENT_KEY, ProfileId.getDefaultInstance())
val subtopicListSize = args.getInt(SUBTOPIC_LIST_SIZE_ARGUMENT_KEY, -1)
return revisionCardFragmentPresenter.handleCreateView(
inflater, container, topicId, subtopicId, profileId
inflater, container, topicId, subtopicId, profileId, subtopicListSize
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import org.oppia.android.app.model.ProfileId
import org.oppia.android.app.topic.conceptcard.ConceptCardFragment
import org.oppia.android.app.topic.conceptcard.ConceptCardFragment.Companion.CONCEPT_CARD_DIALOG_FRAGMENT_TAG
import org.oppia.android.app.translation.AppLanguageResourceHandler
import org.oppia.android.app.viewmodel.ViewModelProvider
import org.oppia.android.databinding.RevisionCardFragmentBinding
import org.oppia.android.domain.oppialogger.OppiaLogger
import org.oppia.android.domain.translation.TranslationController
Expand All @@ -26,9 +25,9 @@ class RevisionCardFragmentPresenter @Inject constructor(
private val htmlParserFactory: HtmlParser.Factory,
@DefaultResourceBucketName private val resourceBucketName: String,
@TopicHtmlParserEntityType private val entityType: String,
private val viewModelProvider: ViewModelProvider<RevisionCardViewModel>,
private val translationController: TranslationController,
private val appLanguageResourceHandler: AppLanguageResourceHandler
private val appLanguageResourceHandler: AppLanguageResourceHandler,
private val revisionCardViewModelFactory: RevisionCardViewModel.Factory
) : HtmlParser.CustomOppiaTagActionListener {
private lateinit var profileId: ProfileId

Expand All @@ -37,7 +36,8 @@ class RevisionCardFragmentPresenter @Inject constructor(
container: ViewGroup?,
topicId: String,
subtopicId: Int,
profileId: ProfileId
profileId: ProfileId,
subtopicListSize: Int
): View? {
this.profileId = profileId

Expand All @@ -48,9 +48,13 @@ class RevisionCardFragmentPresenter @Inject constructor(
/* attachToRoot= */ false
)
val view = binding.revisionCardExplanationText
val viewModel = getReviewCardViewModel()
val viewModel = revisionCardViewModelFactory.create(
topicId,
subtopicId,
profileId,
subtopicListSize
)

viewModel.initialize(topicId, subtopicId, profileId)
logRevisionCardEvent(topicId, subtopicId)

binding.let {
Expand All @@ -59,22 +63,21 @@ class RevisionCardFragmentPresenter @Inject constructor(
}

viewModel.revisionCardLiveData.observe(
fragment,
{ ephemeralRevisionCard ->
val pageContentsHtml =
translationController.extractString(
ephemeralRevisionCard.revisionCard.pageContents,
ephemeralRevisionCard.writtenTranslationContext
)
view.text = htmlParserFactory.create(
resourceBucketName, entityType, topicId, imageCenterAlign = true,
customOppiaTagActionListener = this,
displayLocale = appLanguageResourceHandler.getDisplayLocale()
).parseOppiaHtml(
pageContentsHtml, view, supportsLinks = true, supportsConceptCards = true
fragment
) { ephemeralRevisionCard ->
val pageContentsHtml =
translationController.extractString(
ephemeralRevisionCard.revisionCard.pageContents,
ephemeralRevisionCard.writtenTranslationContext
)
}
)
view.text = htmlParserFactory.create(
resourceBucketName, entityType, topicId, imageCenterAlign = true,
customOppiaTagActionListener = this,
displayLocale = appLanguageResourceHandler.getDisplayLocale()
).parseOppiaHtml(
pageContentsHtml, view, supportsLinks = true, supportsConceptCards = true
)
}

return binding.root
}
Expand All @@ -88,10 +91,6 @@ class RevisionCardFragmentPresenter @Inject constructor(
}
}

private fun getReviewCardViewModel(): RevisionCardViewModel {
return viewModelProvider.getForFragment(fragment, RevisionCardViewModel::class.java)
}

private fun logRevisionCardEvent(topicId: String, subTopicId: Int) {
oppiaLogger.logImportantEvent(oppiaLogger.createOpenRevisionCardContext(topicId, subTopicId))
}
Expand Down
Loading

0 comments on commit f096b1d

Please sign in to comment.