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

Fix part of #140: Topic play animation #486

Closed
wants to merge 11 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Next Next commit
Topic play animation
  • Loading branch information
Rajat Talesra committed Nov 28, 2019
commit ddc950a26660536d1ec95fa83bfdbdcb6af36359
86 changes: 75 additions & 11 deletions app/src/main/java/org/oppia/app/topic/play/StorySummaryAdapter.kt
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
package org.oppia.app.topic.play

import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import android.view.animation.Animation
import android.view.animation.Transformation
import android.widget.LinearLayout
import androidx.recyclerview.widget.RecyclerView
import org.oppia.app.databinding.TopicPlayStorySummaryBinding
import org.oppia.app.databinding.TopicPlayTitleBinding
Expand All @@ -12,6 +16,7 @@ import org.oppia.app.model.ChapterSummary

private const val VIEW_TYPE_TITLE_TEXT = 1
private const val VIEW_TYPE_STORY_ITEM = 2
private const val ANIMATION_DURATION: Long = 400

/** Adapter to bind StorySummary to [RecyclerView] inside [TopicPlayFragment]. */
class StorySummaryAdapter(
Expand Down Expand Up @@ -86,11 +91,20 @@ class StorySummaryAdapter(
inner class StorySummaryViewHolder(private val binding: TopicPlayStorySummaryBinding) :
RecyclerView.ViewHolder(binding.root) {
internal fun bind(storySummaryViewModel: StorySummaryViewModel, position: Int) {
var isChapterListVisible = false
if (currentExpandedChapterListIndex != null) {
isChapterListVisible = currentExpandedChapterListIndex!! == position
if (currentExpandedChapterListIndex != null && currentExpandedChapterListIndex==position) {
binding.isListExpanded = true

if (currentExpandedChapterListIndex == position) {
binding.chapterListDropDownIcon.animate().rotation(180F).setDuration(400).start()
expand(binding.chapterListContainer)
} else {
binding.chapterListDropDownIcon.animate().rotation(0F).setDuration(400).start()
collapse(binding.chapterListContainer)
}
}
else {
binding.isListExpanded = false
}
binding.isListExpanded = isChapterListVisible
binding.viewModel = storySummaryViewModel

val chapterSummaries = storySummaryViewModel.storySummary.chapterList
Expand All @@ -111,25 +125,75 @@ class StorySummaryAdapter(
binding.chapterRecyclerView.adapter = ChapterSummaryAdapter(chapterList, chapterSummarySelector)

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) {
notifyItemChanged(currentExpandedChapterListIndex!!)

binding.isListExpanded = currentExpandedChapterListIndex == position

if (currentExpandedChapterListIndex == position) {
binding.chapterListDropDownIcon.animate().rotation(180F).setDuration(400).start()
expand(binding.chapterListContainer)
} else {
if (previousIndex != null) {
notifyItemChanged(previousIndex)
binding.chapterListDropDownIcon.animate().rotation(0F).setDuration(400).start()
collapse(binding.chapterListContainer)
}
}
}

private fun expand(chapterListContainer: View) {
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

I'm a bit confused by this. Why can't we rely on RecyclerView's own item animator for this? These animations seem a bit complex, and should really be tested directly since if they regress it will be hard for us to detect. I'd rather we rely on existing animation systems rather than creating our own Animation classes, if possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am actually not able to understand how can we do this using RecyclerView's animator?
Also, the main point behind introducing these animations were two fold:

  1. Make expand/collapse animation smooth.
  2. The animation time of expand, and arrow rotate all should be in sync.

I don't have much experience of automated animation, can you just give information regarding RecyclerView's item animator in more detail?
Thanks.

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Sure. I was thinking specifically DefaultItemAnimator, but I'm guessing that might not do the right thing. In which case, maybe something like https://stackoverflow.com/a/37453839 or another animator library might make this a bit easier if it's possible.

Copy link
Contributor Author

@rt4914 rt4914 Dec 11, 2019

Choose a reason for hiding this comment

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

This animation is applicable on recyclerview item. But here we want to expand and contract a linear layout, which is again within a recyclerview item.

chapterListContainer.clearAnimation()
val matchParentMeasureSpec =
View.MeasureSpec.makeMeasureSpec((chapterListContainer.parent as View).width, View.MeasureSpec.EXACTLY)
val wrapContentMeasureSpec = View.MeasureSpec.makeMeasureSpec(0, View.MeasureSpec.UNSPECIFIED)
chapterListContainer.measure(matchParentMeasureSpec, wrapContentMeasureSpec)
val targetHeight = chapterListContainer.measuredHeight

// Older versions of android (pre API 21) cancel animations for views with a height of 0.
chapterListContainer.layoutParams.height = 1
chapterListContainer.visibility = View.VISIBLE
val expandAnimation = object : Animation() {
override fun applyTransformation(interpolatedTime: Float, t: Transformation) {
if (interpolatedTime == 1f) {
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

We shouldn't ever do direct equality with floats due to precision error. Suggest instead using Float.approximatelyEquals (in our own FloatExtensions file).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you referring to something like this: https://alvinalexander.com/source-code/java-approximately-equals-method-function

Also, what exactly do you mean by our own FloatExtensions file, do we need to create a file, which extends float and in which there is a function which does the part of approximatelyEquals ?

chapterListContainer.layoutParams.height = LinearLayout.LayoutParams.WRAP_CONTENT
} else {
chapterListContainer.layoutParams.height = (targetHeight * interpolatedTime).toInt()
}
if (currentExpandedChapterListIndex != null) {
notifyItemChanged(currentExpandedChapterListIndex!!)
chapterListContainer.requestLayout()
}

override fun willChangeBounds(): Boolean {
return true
}
}
expandAnimation.duration = ANIMATION_DURATION
chapterListContainer.startAnimation(expandAnimation)
}

private fun collapse(chapterListContainer: View) {
chapterListContainer.clearAnimation()
val initialHeight = chapterListContainer.measuredHeight

val collapseAnimation = object : Animation() {
override fun applyTransformation(interpolatedTime: Float, t: Transformation?) {
if (interpolatedTime == 1f) {
chapterListContainer.visibility = View.GONE
} else {
chapterListContainer.layoutParams.height = initialHeight - (initialHeight * interpolatedTime).toInt()
}
chapterListContainer.requestLayout()
}

override fun willChangeBounds(): Boolean {
return true
}
}
collapseAnimation.duration = ANIMATION_DURATION
chapterListContainer.startAnimation(collapseAnimation)
}
}
}
3 changes: 2 additions & 1 deletion app/src/main/res/layout/topic_play_story_summary.xml
Original file line number Diff line number Diff line change
Expand Up @@ -126,11 +126,12 @@
android:minHeight="48dp">

<ImageView
android:id="@+id/chapter_list_drop_down_icon"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_gravity="center_horizontal|bottom"
android:contentDescription="@{@string/show_hide_chapter_list(viewModel.storySummary.storyName)}"
android:src="@{isListExpanded? @drawable/ic_arrow_drop_up_black_24dp : @drawable/ic_arrow_drop_down_black_24dp}" />
android:src="@drawable/ic_arrow_drop_down_black_24dp" />
</FrameLayout>
</LinearLayout>
</LinearLayout>
Expand Down