Skip to content

Commit fa09d3f

Browse files
authored
[MOBILE-3601] Fix pager page actions (#1226)
* Fix actions for initial pager page displays * Update PagerModelTest * spots
1 parent 0007892 commit fa09d3f

File tree

2 files changed

+44
-11
lines changed

2 files changed

+44
-11
lines changed

urbanairship-layout/src/main/java/com/urbanairship/android/layout/model/PagerModel.kt

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import com.urbanairship.android.layout.property.ViewType
1818
import com.urbanairship.android.layout.util.pagerScrolls
1919
import com.urbanairship.android.layout.view.PagerView
2020
import com.urbanairship.json.JsonValue
21+
import kotlinx.coroutines.flow.filter
22+
import kotlinx.coroutines.flow.map
2123
import kotlinx.coroutines.launch
2224

2325
internal class PagerModel(
@@ -82,6 +84,25 @@ internal class PagerModel(
8284
pagerState.update { state ->
8385
state.copyWithPageIds(pageIds = items.map { it.identifier })
8486
}
87+
88+
// Listen for page changes (or the initial page display)
89+
// and run any actions for the current page.
90+
modelScope.launch {
91+
92+
pagerState.changes
93+
.map { it.pageIndex to it.lastPageIndex }
94+
.filter { (pageIndex, lastPageIndex) ->
95+
// If current and last are both 0, we're initializing the pager.
96+
// Otherwise, we only want to act on changes to the pageIndex.
97+
pageIndex == 0 && lastPageIndex == 0 || pageIndex != lastPageIndex
98+
}
99+
.collect { (pageIndex, _) ->
100+
// Run any actions for the current page.
101+
items[pageIndex].actions?.let { actions ->
102+
runActions(actions)
103+
}
104+
}
105+
}
85106
}
86107

87108
override fun onCreateView(context: Context, viewEnvironment: ViewEnvironment) =
@@ -90,12 +111,15 @@ internal class PagerModel(
90111
}
91112

92113
override fun onViewAttached(view: PagerView) {
114+
// Collect page index changes from state and tell the view to scroll to the current page.
93115
viewScope.launch {
94116
pagerState.changes.collect {
95117
listener?.scrollTo(it.pageIndex)
96118
}
97119
}
98120

121+
// Collect pager scrolls, update pager state, and report
122+
// the page swipe if it was triggered by the user.
99123
viewScope.launch {
100124
view.pagerScrolls().collect { (position, isInternalScroll) ->
101125
pagerState.update { state ->
@@ -105,11 +129,6 @@ internal class PagerModel(
105129
if (!isInternalScroll) {
106130
reportPageSwipe(pagerState.changes.value)
107131
}
108-
109-
// Run any actions for the current page.
110-
items[position].actions?.let { actions ->
111-
runActions(actions)
112-
}
113132
}
114133
}
115134
}

urbanairship-layout/src/test/java/com/urbanairship/android/layout/model/PagerModelTest.kt

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,10 @@ import io.mockk.verify
1919
import kotlinx.coroutines.ExperimentalCoroutinesApi
2020
import kotlinx.coroutines.flow.MutableSharedFlow
2121
import kotlinx.coroutines.flow.first
22+
import kotlinx.coroutines.test.StandardTestDispatcher
2223
import kotlinx.coroutines.test.TestResult
2324
import kotlinx.coroutines.test.TestScope
25+
import kotlinx.coroutines.test.runCurrent
2426
import kotlinx.coroutines.test.runTest
2527
import org.junit.Assert.assertEquals
2628
import org.junit.Assert.assertFalse
@@ -33,6 +35,8 @@ import org.robolectric.RobolectricTestRunner
3335
@OptIn(ExperimentalCoroutinesApi::class)
3436
@RunWith(RobolectricTestRunner::class)
3537
public class PagerModelTest {
38+
private val testDispatcher = StandardTestDispatcher()
39+
private val testScope = TestScope(testDispatcher)
3640

3741
private val scrollsFlow = MutableSharedFlow<PagerScrollEvent>()
3842

@@ -41,7 +45,7 @@ public class PagerModelTest {
4145
private val mockEnv: ModelEnvironment = mockk(relaxed = true) {
4246
every { reporter } returns mockReporter
4347
every { actionsRunner } returns mockActionsRunner
44-
every { modelScope } returns TestScope()
48+
every { modelScope } returns testScope
4549
}
4650
private val mockView: PagerView = mockk(relaxed = true)
4751
private val mockViewListener: PagerModel.Listener = mockk(relaxed = true)
@@ -65,6 +69,8 @@ public class PagerModelTest {
6569

6670
mockkStatic(PagerView::pagerScrolls)
6771
every { mockView.pagerScrolls() } returns scrollsFlow
72+
73+
testScope.runCurrent()
6874
}
6975

7076
@Test
@@ -81,7 +87,10 @@ public class PagerModelTest {
8187
// Verify that the correct number of page items is available via the model
8288
assertEquals(3, pagerModel.items.size)
8389

90+
// Verify that the model notified the view to scroll to the first page
8491
verify { mockViewListener.scrollTo(0) }
92+
// Verify that actions were run for the initial page display
93+
verify(exactly = 1) { mockActionsRunner.run(any(), any()) }
8594
}
8695

8796
@Test
@@ -105,10 +114,9 @@ public class PagerModelTest {
105114
assertTrue(updatedState.hasNext)
106115
assertTrue(updatedState.hasPrevious)
107116

108-
verify {
109-
mockReporter.report(any(), any())
110-
mockActionsRunner.run(any(), any())
111-
}
117+
// Verify that we reported an event and ran actions
118+
verify { mockReporter.report(any(), any()) }
119+
verify(exactly = 1) { mockActionsRunner.run(any(), any()) }
112120

113121
ensureAllEventsConsumed()
114122
}
@@ -135,7 +143,7 @@ public class PagerModelTest {
135143
assertTrue(updatedState.hasNext)
136144
assertTrue(updatedState.hasPrevious)
137145

138-
// Verify that we didn't report an event, but ran any actions for the given page.
146+
// Verify that we didn't report an event, but did run actions.
139147
verify(exactly = 0) { mockReporter.report(any(), any()) }
140148
verify(exactly = 1) { mockActionsRunner.run(any(), any()) }
141149

@@ -148,14 +156,20 @@ public class PagerModelTest {
148156
pagerModel.onViewAttached(mockView)
149157

150158
verify { mockViewListener.scrollTo(0) }
159+
// Verify actions were run for the initial page display
160+
verify(exactly = 1) { mockActionsRunner.run(any(), any()) }
151161

152162
pagerState.update { it.copyWithPageIndex(1) }
163+
// Run the pending state update task, so the model can process it.
164+
testScope.runCurrent()
153165

154166
val state = pagerState.changes.first()
155167
// Sanity check
156168
assertEquals(1, state.pageIndex)
157169

158170
verify { mockViewListener.scrollTo(1) }
171+
// Verify actions were also run on display of the next page
172+
verify(exactly = 2) { mockActionsRunner.run(any(), any()) }
159173
}
160174

161175
@Test

0 commit comments

Comments
 (0)