Skip to content

Commit

Permalink
Fix other listeners iteration order to protect against listeners that…
Browse files Browse the repository at this point in the history
… remove themselves
  • Loading branch information
rubensousa committed Nov 14, 2024
1 parent 42c1607 commit f319ad5
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import com.rubensousa.dpadrecyclerview.test.helpers.onRecyclerView
import com.rubensousa.dpadrecyclerview.test.helpers.selectPosition
import com.rubensousa.dpadrecyclerview.test.helpers.waitForCondition
import com.rubensousa.dpadrecyclerview.test.helpers.waitForIdleScrollState
import com.rubensousa.dpadrecyclerview.test.helpers.waitForLayout
import com.rubensousa.dpadrecyclerview.test.tests.DpadRecyclerViewTest
import com.rubensousa.dpadrecyclerview.testing.KeyEvents
import com.rubensousa.dpadrecyclerview.testing.rules.DisableIdleTimeoutRule
Expand Down Expand Up @@ -256,4 +257,31 @@ class FocusListenerTest : DpadRecyclerViewTest() {
assertThat(events).isEqualTo(0)
}

@Test
fun testFocusListenerThatRemovesItself() = report {
val listeners = 5
var events = 0
Given("Attach listeners") {
launchFragment()
onRecyclerView("Get recyclerView") { recyclerView ->
repeat(listeners) {
recyclerView.addOnViewFocusedListener(object : OnViewFocusedListener {
override fun onViewFocused(parent: RecyclerView.ViewHolder, child: View) {
events++
recyclerView.removeOnViewFocusedListener(this)
}
})
}
}
}
When("Select another position") {
selectPosition(2)
waitForLayout()
}

Then("Events are received") {
assertThat(events).isEqualTo(listeners)
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import androidx.test.espresso.matcher.ViewMatchers
import com.google.common.truth.Truth.assertThat
import com.rubensousa.dpadrecyclerview.ChildAlignment
import com.rubensousa.dpadrecyclerview.DpadRecyclerView
import com.rubensousa.dpadrecyclerview.OnViewHolderSelectedListener
import com.rubensousa.dpadrecyclerview.ParentAlignment
import com.rubensousa.dpadrecyclerview.ParentAlignment.Edge
import com.rubensousa.dpadrecyclerview.test.TestAdapterConfiguration
Expand Down Expand Up @@ -337,4 +338,38 @@ class SelectionTest : DpadRecyclerViewTest() {
}
}

@Test
fun testSelectionListenerThatRemovesItself() = report {
val listeners = 5
var events = 0
Given("Attach listeners") {
launchFragment()
onRecyclerView("Get recyclerView") { recyclerView ->
repeat(listeners) {
recyclerView.addOnViewHolderSelectedListener(object :
OnViewHolderSelectedListener {
override fun onViewHolderSelected(
parent: RecyclerView,
child: RecyclerView.ViewHolder?,
position: Int,
subPosition: Int
) {
super.onViewHolderSelected(parent, child, position, subPosition)
recyclerView.removeOnViewHolderSelectedListener(this)
events++
}
})
}
}
}
When("Select another position") {
selectPosition(10)
waitForLayout()
}

Then("Events are received") {
assertThat(events).isEqualTo(listeners)
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import androidx.recyclerview.widget.RecyclerView
import androidx.recyclerview.widget.SimpleItemAnimator
import com.rubensousa.dpadrecyclerview.layoutmanager.PivotLayoutManager
import com.rubensousa.dpadrecyclerview.layoutmanager.focus.GlobalFocusChangeListener
import com.rubensousa.dpadrecyclerview.layoutmanager.forEachReversed
import com.rubensousa.dpadrecyclerview.spacing.DpadGridSpacingDecoration
import com.rubensousa.dpadrecyclerview.spacing.DpadLinearSpacingDecoration
import com.rubensousa.dpadrecyclerview.spacing.DpadSpacingDecoration
Expand Down Expand Up @@ -88,7 +89,7 @@ open class DpadRecyclerView @JvmOverloads constructor(
}
private val globalFocusChangeListener by lazy {
GlobalFocusChangeListener(this) {
focusLossListeners.forEach { listener ->
focusLossListeners.forEachReversed { listener ->
listener.onFocusLost(this)
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*
* Copyright 2024 Rúben Sousa
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.rubensousa.dpadrecyclerview.layoutmanager

internal inline fun <T> List<T>.forEachReversed(action: (T) -> Unit) {
for (i in size - 1 downTo 0) {
action(get(i))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ internal class PivotSelector(
}
}
val focusedViewHolder = currentRecyclerView.findContainingViewHolder(view) ?: return
focusListeners.forEach { listener ->
focusListeners.forEachReversed { listener ->
listener.onViewFocused(
parent = focusedViewHolder,
child = view,
Expand All @@ -152,7 +152,7 @@ internal class PivotSelector(
pendingChildFocus = view
return
}
focusListeners.forEach { listener ->
focusListeners.forEachReversed { listener ->
listener.onViewFocused(
parent = parentViewHolder,
child = view
Expand Down Expand Up @@ -346,7 +346,7 @@ internal class PivotSelector(
if (viewHolder is DpadViewHolder) {
viewHolder.onViewHolderDeselected()
}
selectionListeners.forEach { listener ->
selectionListeners.forEachReversed { listener ->
listener.onViewHolderDeselected(recyclerView, viewHolder)
}
}
Expand All @@ -358,13 +358,13 @@ internal class PivotSelector(
selectedViewHolder = viewHolder

if (viewHolder != null) {
selectionListeners.forEach { listener ->
selectionListeners.forEachReversed { listener ->
listener.onViewHolderSelected(
recyclerView, viewHolder, position, subPosition
)
}
} else {
selectionListeners.forEach { listener ->
selectionListeners.forEachReversed { listener ->
listener.onViewHolderSelected(
recyclerView, null, RecyclerView.NO_POSITION, 0
)
Expand Down Expand Up @@ -406,13 +406,13 @@ internal class PivotSelector(
}

if (viewHolder != null) {
selectionListeners.forEach { listener ->
selectionListeners.forEachReversed { listener ->
listener.onViewHolderSelectedAndAligned(
recyclerView, viewHolder, position, subPosition
)
}
} else {
selectionListeners.forEach { listener ->
selectionListeners.forEachReversed { listener ->
listener.onViewHolderSelectedAndAligned(
recyclerView, null, RecyclerView.NO_POSITION, 0
)
Expand Down Expand Up @@ -459,7 +459,7 @@ internal class PivotSelector(
viewHolder.onViewHolderDeselected()
}
recyclerView?.let {
selectionListeners.forEach { listener ->
selectionListeners.forEachReversed { listener ->
listener.onViewHolderDeselected(it, viewHolder)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import com.rubensousa.dpadrecyclerview.layoutmanager.DpadLayoutParams
import com.rubensousa.dpadrecyclerview.layoutmanager.LayoutConfiguration
import com.rubensousa.dpadrecyclerview.layoutmanager.PivotSelector
import com.rubensousa.dpadrecyclerview.layoutmanager.alignment.LayoutAlignment
import com.rubensousa.dpadrecyclerview.layoutmanager.forEachReversed
import com.rubensousa.dpadrecyclerview.layoutmanager.layout.grid.GridLayoutEngineer
import com.rubensousa.dpadrecyclerview.layoutmanager.layout.linear.LinearLayoutEngineer
import com.rubensousa.dpadrecyclerview.layoutmanager.scroll.LayoutScroller
Expand Down Expand Up @@ -195,8 +196,8 @@ internal class PivotLayout(
updateInitialSelection()
}
layoutInfo.onLayoutCompleted()
for (i in layoutCompleteListeners.size - 1 downTo 0) {
layoutCompleteListeners[i].onLayoutCompleted(state)
layoutCompleteListeners.forEachReversed { listener ->
listener.onLayoutCompleted(state)
}
}

Expand Down Expand Up @@ -381,9 +382,8 @@ internal class PivotLayout(
}
val recyclerView = layoutInfo.getRecyclerView() ?: return
val viewHolder = layoutInfo.getChildViewHolder(view) ?: return

for (i in childLaidOutListeners.size - 1 downTo 0) {
childLaidOutListeners[i].onChildLaidOut(recyclerView, viewHolder)
childLaidOutListeners.forEachReversed { listener ->
listener.onChildLaidOut(recyclerView, viewHolder)
}
}

Expand Down

0 comments on commit f319ad5

Please sign in to comment.