Skip to content

Commit

Permalink
Fix model group issues (#676)
Browse files Browse the repository at this point in the history
  • Loading branch information
@elihart
elihart authored Feb 5, 2019
1 parent da50a4c commit 6f8e85a
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 44 deletions.
13 changes: 8 additions & 5 deletions epoxy-adapter/src/main/java/com/airbnb/epoxy/EpoxyModelGroup.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,12 +141,15 @@ public void bind(@NonNull ModelGroupHolder holder, @NonNull EpoxyModel<?> previo
public void onModel(EpoxyModel model, EpoxyViewHolder viewHolder, int modelIndex) {
setViewVisibility(model, viewHolder);

EpoxyModel<?> previousModel = previousGroup.models.get(modelIndex);
if (previousModel.id() == model.id()) {
viewHolder.bind(model, previousModel, Collections.emptyList(), modelIndex);
} else {
viewHolder.bind(model, null, Collections.emptyList(), modelIndex);
if (modelIndex < previousGroup.models.size()) {
EpoxyModel<?> previousModel = previousGroup.models.get(modelIndex);
if (previousModel.id() == model.id()) {
viewHolder.bind(model, previousModel, Collections.emptyList(), modelIndex);
return;
}
}

viewHolder.bind(model, null, Collections.emptyList(), modelIndex);
}
});
}
Expand Down
51 changes: 24 additions & 27 deletions epoxy-adapter/src/main/java/com/airbnb/epoxy/ModelGroupHolder.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,40 +58,31 @@ class ModelGroupHolder : EpoxyHolder() {
}

private fun createViewStubData(viewGroup: ViewGroup): List<ViewStubData> {
val stubs = ArrayList<ViewStubData>(4)
return ArrayList<ViewStubData>(4).apply {

while (true) {
val stubData = getNextViewStub(viewGroup)
if (stubData != null) {
stubs.add(stubData)
stubData.removeView()
} else {
break
}
}
collectViewStubs(viewGroup, this)

if (stubs.isEmpty()) {
throw IllegalStateException(
"No view stubs found. If viewgroup is not empty it must contain ViewStubs."
)
if (isEmpty()) {
throw IllegalStateException(
"No view stubs found. If viewgroup is not empty it must contain ViewStubs."
)
}
}

return stubs
}

private fun getNextViewStub(viewGroup: ViewGroup): ViewStubData? {
val childCount = viewGroup.childCount
for (i in 0 until childCount) {
private fun collectViewStubs(
viewGroup: ViewGroup,
stubs: ArrayList<ViewStubData>
) {
for (i in 0 until viewGroup.childCount) {
val child = viewGroup.getChildAt(i)

if (child is ViewGroup) {
getNextViewStub(child)?.let { return it }
collectViewStubs(child, stubs)
} else if (child is ViewStub) {
return ViewStubData(viewGroup, child, i)
stubs.add(ViewStubData(viewGroup, child, i))
}
}

return null
}

fun bindGroupIfNeeded(group: EpoxyModelGroup) {
Expand All @@ -116,8 +107,7 @@ class ModelGroupHolder : EpoxyHolder() {

if (usingStubs() && stubs.size < modelCount) {
throw IllegalStateException(
"Insufficient view stubs for EpoxyModelGroup. " + modelCount +
" models were provided but only " + stubs.size + " view stubs exist."
"Insufficient view stubs for EpoxyModelGroup. $modelCount models were provided but only ${stubs.size} view stubs exist."
)
}
viewHolders.ensureCapacity(modelCount)
Expand Down Expand Up @@ -180,7 +170,7 @@ class ModelGroupHolder : EpoxyHolder() {

private fun removeAndRecycleView(modelPosition: Int) {
if (usingStubs()) {
stubs[modelPosition].removeView()
stubs[modelPosition].resetStub()
} else {
childContainer.removeViewAt(modelPosition)
}
Expand All @@ -204,6 +194,8 @@ private class ViewStubData(
) {

fun setView(view: View, useStubLayoutParams: Boolean) {
removeCurrentView()

// Carry over the stub id manually since we aren't inflating via the stub
val inflatedId = viewStub.inflatedId
if (inflatedId != View.NO_ID) {
Expand All @@ -217,7 +209,12 @@ private class ViewStubData(
}
}

fun removeView() {
fun resetStub() {
removeCurrentView()
viewGroup.addView(viewStub, position)
}

private fun removeCurrentView() {
val view = viewGroup.getChildAt(position)
?: throw IllegalStateException("No view exists at position $position")
viewGroup.removeView(view)
Expand Down
78 changes: 66 additions & 12 deletions epoxy-adapter/src/test/java/com/airbnb/epoxy/EpoxyModelGroupTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ class EpoxyModelGroupTest(val useViewStubs: Boolean) {
assertEquals(0, modelGroupHolder.viewHolders.size)
}

private fun bind(modelGroup: EpoxyModelGroup) {
private fun bind(modelGroup: EpoxyModelGroup, previousGroup: EpoxyModelGroup? = null) {
if (topLevelHolder == null) {
topLevelHolder = EpoxyViewHolder(modelGroup.buildView(recyclerView), false)
}
topLevelHolder!!.bind(modelGroup, null, emptyList(), 0)
topLevelHolder!!.bind(modelGroup, previousGroup, emptyList(), 0)
}

private fun assertModelsBound(modelGroup: EpoxyModelGroup) {
Expand All @@ -62,47 +62,53 @@ class EpoxyModelGroupTest(val useViewStubs: Boolean) {

@Test
fun bind_Unbind_Rebind_LinearLayoutWithLessModels() {
bind(createFrameLayoutGroup(3))
val firstGroup = createFrameLayoutGroup(3)
bind(firstGroup)
unbind()

createSpaceGroup(2).let {
bind(it)
bind(it, firstGroup)
assertModelsBound(it)
}
}

@Test
fun bind_Unbind_Rebind_LinearLayoutWithMoreModels() {
bind(createFrameLayoutGroup(3))
val firstGroup = createFrameLayoutGroup(3)
bind(firstGroup)
unbind()
createSpaceGroup(4).let {
bind(it)
bind(it, firstGroup)
assertModelsBound(it)
}
}

@Test
fun rebind_LinearLayoutWithSameViewTypes() {
bind(createFrameLayoutGroup(3))
val firstGroup = createFrameLayoutGroup(3)
bind(firstGroup)
createFrameLayoutGroup(4).let {
bind(it)
bind(it, firstGroup)
assertModelsBound(it)
}
}

@Test
fun rebind_LinearLayoutWithMoreModels() {
bind(createFrameLayoutGroup(3))
val firstGroup = createFrameLayoutGroup(3)
bind(firstGroup)
createSpaceGroup(4).let {
bind(it)
bind(it, firstGroup)
assertModelsBound(it)
}
}

@Test
fun rebind_LinearLayoutWithLessModels() {
bind(createFrameLayoutGroup(3))
val firstGroup = createFrameLayoutGroup(3)
bind(firstGroup)
createSpaceGroup(2).let {
bind(it)
bind(it, firstGroup)
assertModelsBound(it)
}
}
Expand All @@ -120,6 +126,54 @@ class EpoxyModelGroupTest(val useViewStubs: Boolean) {
assertEquals(firstHolders, secondHolders)
}

@Test
fun viewStubsOutOfOrder() {
val models = (0 until 4).map { NestedModelFrameLayout().id(it) }

val modelGroup = object : EpoxyModelGroup(0, models) {
public override fun buildView(parent: ViewGroup): View {
return LinearLayout(parent.context).apply {

addView(ViewStub(parent.context).apply {
inflatedId = 0
})

addView(LinearLayout(parent.context).apply {
addView(ViewStub(parent.context).apply {
inflatedId = 1
})

addView(Space(parent.context))

addView(ViewStub(parent.context).apply {
inflatedId = 2
})
})

addView(ViewStub(parent.context).apply {
inflatedId = 3
})
}
}
}

bind(modelGroup)

modelGroupHolder.viewHolders.forEachIndexed { index, viewholder ->

val view = viewholder.itemView
assertEquals(index, view.id)

val indexInsideParentView = (view.parent as ViewGroup).indexOfChild(view)
when (view.id) {
0 -> assertEquals(0, indexInsideParentView)
1 -> assertEquals(0, indexInsideParentView)
2 -> assertEquals(2, indexInsideParentView)
3 -> assertEquals(2, indexInsideParentView)
}
}
}

private fun createFrameLayoutGroup(modelCount: Int): EpoxyModelGroup {
val models = (0 until modelCount).map { NestedModelFrameLayout().id(it) }
return if (useViewStubs) ViewStubsGroupModel(models) else LinerLayoutGroupModel(models)
Expand Down

0 comments on commit 6f8e85a

Please sign in to comment.