Run form-entry Next navigation off the main thread#3667
Run form-entry Next navigation off the main thread#3667
Conversation
Split the form navigation logic into a pure state-stepping function (stepToRenderableEvent) that returns a NavResult, plus a main-thread dispatcher (renderNavResult) that handles view creation and dialog display. Behavior unchanged; this is the seam needed to later move stepping off the main thread.
Introduces a reusable helper that runs form-stepping work on a background dispatcher and delivers the resulting NavResult back on the main thread, with a re-entrancy guard and overlay visibility hook. Not wired into the UI yet. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Wires showNextView through AsyncFormNavigator so stepToRenderableEvent runs on Dispatchers.Default. A loading overlay with an indeterminate ProgressBar is shown while the step is in flight and hidden before renderNavResult runs on the main thread. shouldIgnoreNavigationAction now also returns true while a navigation is in flight to prevent re-entry from rapid Next taps or a Previous during a forward step. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Wraps the overlay view in a FormLoadingOverlay helper whose show() posts a 150 ms-delayed visibility change and whose hide() cancels any pending show so fast navigations don't flash the overlay. Adds a centered "Loading next question…" TextView below the ProgressBar, fed from the new form.entry.loading.next.question locale entry. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Moves navigationInFlight out of AsyncFormNavigator and onto FormEntryActivityUIController so the UI controller owns nav gating (checked from shouldIgnoreNavigationAction). The navigator now takes the start/result callbacks per navigate() call instead of an overlay callback in the constructor, retains the launched Job, and exposes cancel() so lifecycle hooks can abort an in-flight step. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Each navigate() captures an incrementing id and re-checks it after the background step returns; results whose id has been superseded (by a newer navigate() or a cancel()) are silently dropped. cancel() bumps the id too so a pending continuation can't race past Job.cancel(). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds FormEntryActivityUIController.cancelNavigation() which cancels the navigator, clears navigationInFlight, and hides the loading overlay. FormEntryActivity.onPause() invokes it so a background step doesn't keep running (or deliver a late result against stale state) while the activity is paused. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis pull request introduces asynchronous form navigation in the CommCare form entry flow. A new Sequence Diagram(s)sequenceDiagram
actor User
participant FormEntry as FormEntryActivity
participant UICtrl as FormEntryActivityUIController
participant Overlay as FormLoadingOverlay
participant Navigator as AsyncFormNavigator
participant Dispatcher as Dispatchers.Default
participant Controller as FormController
User->>FormEntry: User swipes/taps next
FormEntry->>UICtrl: showNextView()
alt shouldIgnoreNavigationAction() check
UICtrl-->>FormEntry: Early return (navigationInFlight=true)
else Navigation allowed
UICtrl->>Navigator: navigate(resuming, onStart, onResult)
Navigator->>UICtrl: onStart.onStart()
UICtrl->>Overlay: show()
Overlay-->>UICtrl: Loading overlay displayed
Navigator->>Dispatcher: Launch coroutine
Dispatcher->>Controller: step() (background)
Controller-->>Dispatcher: NavResult
Dispatcher->>Navigator: Result received
Navigator->>UICtrl: onResult.onResult(navResult)
UICtrl->>UICtrl: renderNavResult(navResult)
UICtrl->>Overlay: hide()
Overlay-->>UICtrl: Loading overlay hidden
UICtrl->>FormEntry: Render next question/view
FormEntry-->>User: Updated form display
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/org/commcare/activities/FormEntryActivity.java (1)
911-919:⚠️ Potential issue | 🔴 Critical
cancelNavigation()cannot truly interrupt the background step beforesaveAnswersForCurrentScreenruns.
AsyncFormNavigator.cancel()usesJob.cancel(), butstepWork.step()is a blocking Java call (mFormController.stepToNextEvent(...)) with no suspension points, so coroutine cancellation will not interrupt it. The background thread may continue executingFormEntryControllermutations while the main thread immediately proceeds intosaveAnswersForCurrentScreen(false)at line 917 and subsequent pause handling — both touching the same non-thread-safemFormController. This can corrupt form state or lose answers on pause-during-navigation.Consider awaiting the in-flight job (e.g.,
runBlocking { job?.join() }incancel(), or anAtomicBooleancancelled flag plus a synchronous wait) before lettingonPause()continue, or gatesaveAnswersForCurrentScreenon the navigation no longer being in flight.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/org/commcare/activities/FormEntryActivity.java` around lines 911 - 919, The cancelNavigation() call in onPause() does not wait for AsyncFormNavigator.cancel() to actually stop the background stepWork.step() (which calls mFormController.stepToNextEvent(...)), so saveAnswersForCurrentScreen(false) can run concurrently against mutable mFormController state; change cancelNavigation()/AsyncFormNavigator.cancel() to synchronously wait for the in-flight job to complete (for example have cancel() join the coroutine job or set an AtomicBoolean and block until stepWork finishes) and/or make onPause() gate calling saveAnswersForCurrentScreen() until the navigator reports navigationComplete; update AsyncFormNavigator.cancel(), stepWork.step(), and the code path used by onPause()/cancelNavigation() to use this join/flag handshake so no FormEntryController mutations happen concurrently with saveAnswersForCurrentScreen().
🧹 Nitpick comments (1)
app/src/org/commcare/activities/AsyncFormNavigator.kt (1)
14-21: Consider passinglifecycleScopeor a dispatcher explicitly for testability.Hard-coding
lifecycleOwner.lifecycleScope+Dispatchers.Defaultinside the class makes this navigator awkward to unit test (no way to inject aTestDispatcher). A small refactor to acceptCoroutineScopeandCoroutineDispatcher(with current values as defaults) would ease the follow-up test work called out in the PR description.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/org/commcare/activities/AsyncFormNavigator.kt` around lines 14 - 21, The AsyncFormNavigator currently hard-codes lifecycleOwner.lifecycleScope and Dispatchers.Default which hinders testability; refactor the constructor to accept a CoroutineScope and a CoroutineDispatcher (e.g., add params coroutineScope: CoroutineScope = lifecycleOwner.lifecycleScope and dispatcher: CoroutineDispatcher = Dispatchers.Default) and update any internal usages to use these injected values instead of lifecycleOwner.lifecycleScope/Dispatchers.Default; keep the existing StepWork functional interface unchanged so callers can still pass step(), and ensure tests can pass a TestScope/TestDispatcher by injecting them.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/org/commcare/activities/AsyncFormNavigator.kt`:
- Around line 34-50: The navigate/cancel flow leaves blocking Java work
uncontrolled and races stale results; fix by (1) running the blocking
stepWork.step() on a dedicated single-thread ExecutorService (store its Future
in a field like stepFuture) instead of relying on cooperative cancellation, call
stepFuture.cancel(true) in cancel() to attempt to interrupt the background
thread, and ensure you stop any FormEntryController mutations from that thread;
(2) assign the coroutine Job to job immediately by doing val assignedJob =
lifecycleOwner.lifecycleScope.launch { ... } then job = assignedJob (or
construct a Job and pass it into launch) so cancel() always sees the latest job,
and make cancel perform job?.cancelAndJoin() (use runBlocking if cancel() must
be synchronous) to wait for the coroutine to finish; and (3) add a simple
AtomicBoolean navigating guard in navigate() to serialize invocations (set true
at start, false on completion) to prevent overlap between onStart/onResult
windows. Ensure references to stepWork.step(),
lifecycleOwner.lifecycleScope.launch, job, currentNavId, cancel(), and
stepFuture/Future are updated accordingly.
In `@app/src/org/commcare/activities/FormEntryActivityUIController.java`:
- Around line 449-489: The stepToRenderableEvent method currently only catches
XPathException, so unchecked exceptions from
FormEntryController.stepToNextEvent/indexIsInFieldList/getQuestionPrompts can
crash the coroutine and leave navigationInFlight/overlay stuck; widen the catch
to Throwable (or RuntimeException) in stepToRenderableEvent and return new
NavResult.Error(throwable) so all failures surface as NavResult.Error, or
alternatively wrap the caller's navigate callback to always clear
navigationInFlight and hide the overlay in a finally block—update references in
stepToRenderableEvent, NavResult.Error, and the navigate/navigationInFlight
overlay handling to ensure any thrown Throwable is converted to NavResult.Error
or always clears the in-flight state.
---
Outside diff comments:
In `@app/src/org/commcare/activities/FormEntryActivity.java`:
- Around line 911-919: The cancelNavigation() call in onPause() does not wait
for AsyncFormNavigator.cancel() to actually stop the background stepWork.step()
(which calls mFormController.stepToNextEvent(...)), so
saveAnswersForCurrentScreen(false) can run concurrently against mutable
mFormController state; change cancelNavigation()/AsyncFormNavigator.cancel() to
synchronously wait for the in-flight job to complete (for example have cancel()
join the coroutine job or set an AtomicBoolean and block until stepWork
finishes) and/or make onPause() gate calling saveAnswersForCurrentScreen() until
the navigator reports navigationComplete; update AsyncFormNavigator.cancel(),
stepWork.step(), and the code path used by onPause()/cancelNavigation() to use
this join/flag handshake so no FormEntryController mutations happen concurrently
with saveAnswersForCurrentScreen().
---
Nitpick comments:
In `@app/src/org/commcare/activities/AsyncFormNavigator.kt`:
- Around line 14-21: The AsyncFormNavigator currently hard-codes
lifecycleOwner.lifecycleScope and Dispatchers.Default which hinders testability;
refactor the constructor to accept a CoroutineScope and a CoroutineDispatcher
(e.g., add params coroutineScope: CoroutineScope = lifecycleOwner.lifecycleScope
and dispatcher: CoroutineDispatcher = Dispatchers.Default) and update any
internal usages to use these injected values instead of
lifecycleOwner.lifecycleScope/Dispatchers.Default; keep the existing StepWork
functional interface unchanged so callers can still pass step(), and ensure
tests can pass a TestScope/TestDispatcher by injecting them.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9ddc3e7e-9cdb-412e-a4a2-cd589efba9fa
📒 Files selected for processing (7)
app/assets/locales/android_translatable_strings.txtapp/res/layout/screen_form_entry.xmlapp/src/org/commcare/activities/AsyncFormNavigator.ktapp/src/org/commcare/activities/FormEntryActivity.javaapp/src/org/commcare/activities/FormEntryActivityUIController.javaapp/src/org/commcare/activities/FormLoadingOverlay.ktapp/src/org/commcare/activities/NavResult.kt
| fun navigate(resuming: Boolean, onStart: StartCallback, onResult: ResultCallback) { | ||
| val navId = ++currentNavId | ||
| onStart.onStart() | ||
| job = lifecycleOwner.lifecycleScope.launch { | ||
| val result = withContext(Dispatchers.Default) { stepWork.step() } | ||
| if (navId != currentNavId) { | ||
| return@launch | ||
| } | ||
| onResult.onResult(result) | ||
| } | ||
| } | ||
|
|
||
| fun cancel() { | ||
| currentNavId++ | ||
| job?.cancel() | ||
| job = null | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and read the AsyncFormNavigator.kt file
fd -t f AsyncFormNavigator.ktRepository: dimagi/commcare-android
Length of output: 120
🏁 Script executed:
# Look at the file structure and content around lines 34-50
cat -n app/src/org/commcare/activities/AsyncFormNavigator.kt | head -60Repository: dimagi/commcare-android
Length of output: 1723
🏁 Script executed:
# Check the entire class to understand the context better
wc -l app/src/org/commcare/activities/AsyncFormNavigator.ktRepository: dimagi/commcare-android
Length of output: 123
🏁 Script executed:
# Find usages of AsyncFormNavigator
rg -t kotlin "AsyncFormNavigator" --max-count 20Repository: dimagi/commcare-android
Length of output: 146
🏁 Script executed:
# Search for StepWork implementations to understand what step() does
rg -t kotlin "StepWork" -A 3 --max-count 20Repository: dimagi/commcare-android
Length of output: 550
🏁 Script executed:
# Look for FormEntryActivity and how cancelNavigation is called
fd -t f FormEntryActivity.ktRepository: dimagi/commcare-android
Length of output: 49
🏁 Script executed:
# Search for cancelNavigation usage
rg -t kotlin "cancelNavigation" -B 2 -A 2Repository: dimagi/commcare-android
Length of output: 49
🏁 Script executed:
# Search for FormEntry related files
fd -t f "FormEntry" --iglob "*FormEntry*"Repository: dimagi/commcare-android
Length of output: 269
🏁 Script executed:
# Check git log for AsyncFormNavigator to see when it was added
git log --oneline -- app/src/org/commcare/activities/AsyncFormNavigator.kt | head -5Repository: dimagi/commcare-android
Length of output: 127
🏁 Script executed:
# Check git diff to see if this is recent
git log -1 --format="%ai" -- app/src/org/commcare/activities/AsyncFormNavigator.ktRepository: dimagi/commcare-android
Length of output: 92
🏁 Script executed:
# Get the full diff for AsyncFormNavigator to see the context
git diff HEAD~1 -- app/src/org/commcare/activities/AsyncFormNavigator.ktRepository: dimagi/commcare-android
Length of output: 97
🏁 Script executed:
# Check the commit details
git show --stat 21f76cc29Repository: dimagi/commcare-android
Length of output: 50380
🏁 Script executed:
# Search for all recent files that might use AsyncFormNavigator
git diff HEAD~1 --name-only | head -20Repository: dimagi/commcare-android
Length of output: 253
🏁 Script executed:
# Search for FormEntryActivityUIController
fd -t f "FormEntryActivityUIController.kt"Repository: dimagi/commcare-android
Length of output: 49
🏁 Script executed:
# Search for where cancel() might be called
rg -t kotlin "\.cancel\(\)" -B 2 -A 2 | head -50Repository: dimagi/commcare-android
Length of output: 2905
🏁 Script executed:
# Search for navigationInFlight mentioned in commit message
rg -t kotlin "navigationInFlight" -B 2 -A 2Repository: dimagi/commcare-android
Length of output: 49
🏁 Script executed:
# Check if lifecycleScope.launch is synchronous or asynchronous
rg -t kotlin "lifecycleScope" -B 2 -A 5 | head -80Repository: dimagi/commcare-android
Length of output: 2778
🏁 Script executed:
# Look for how cancel() is intended to be called based on the comment
rg -t kotlin "onPause\|cancelNavigation" -B 3 -A 3Repository: dimagi/commcare-android
Length of output: 49
🏁 Script executed:
# Check what withContext does with blocking work
rg -t kotlin "withContext\(Dispatchers" -B 2 -A 5 | head -60Repository: dimagi/commcare-android
Length of output: 5293
🏁 Script executed:
# Look for kotlin coroutine documentation references or patterns
rg -t kotlin "cancelAndJoin\|runBlocking" -B 2 -A 2Repository: dimagi/commcare-android
Length of output: 49
🏁 Script executed:
# Check if there are any InternalCoroutinesApi violations in this file
rg -t kotlin "InternalCoroutinesApi|tryResume" app/src/org/commcare/activities/AsyncFormNavigator.ktRepository: dimagi/commcare-android
Length of output: 49
🏁 Script executed:
# Verify the actual behavior of lifecycleScope.launch - is it synchronous or async?
# Check the common patterns in the codebase for launching and cancelling
rg -t kotlin "lifecycleScope.launch" -A 8 | head -100Repository: dimagi/commcare-android
Length of output: 1545
🏁 Script executed:
# Look for any existing serialization guards or job handling patterns
rg -t kotlin "AtomicBoolean|atomic|synchronized" app/src/org/commcare/activities/ -B 2 -A 2Repository: dimagi/commcare-android
Length of output: 49
🏁 Script executed:
# Check what FormEntryController is and if it's thread-safe
fd -t f "FormEntryController.kt" -o "FormEntryController.java"Repository: dimagi/commcare-android
Length of output: 234
🏁 Script executed:
# Look for any synchronization patterns around FormEntryController mutations
rg -t kotlin -t java "FormEntryController" -B 2 -A 5 | head -80Repository: dimagi/commcare-android
Length of output: 6475
🏁 Script executed:
# Verify kotlinx.coroutines version to understand cancellation semantics
rg -t gradle "kotlinx-coroutines" app/build.gradleRepository: dimagi/commcare-android
Length of output: 143
🏁 Script executed:
# Check Kotlin coroutine documentation on lifecycleScope.launch behavior
rg -t kotlin "suspend\|launch" app/src/org/commcare/activities/AsyncFormNavigator.kt -B 2 -A 5Repository: dimagi/commcare-android
Length of output: 49
🏁 Script executed:
# Verify if there's a real window where two navigate() calls could both start executing
# by checking if there are any barriers between navigate() calls
rg -t kotlin "navigate\(" -B 3 -A 8 --max-count 10Repository: dimagi/commcare-android
Length of output: 1190
cancel() is not effective for blocking stepWork.step() and leaves a window for stale results.
Two related issues:
-
stepWork.step()runs viawithContext(Dispatchers.Default)but is pure blocking Java with no cooperative cancellation checks.job?.cancel()will only preventonResultfrom firing (already guarded bynavId), it will not stop theFormEntryControllermutations on the background thread. Callers that assume a synchronous post-cancel state (e.g.,FormEntryActivity.onPause()→cancelNavigation()→saveAnswersForCurrentScreen) will race with the still-running step on the shared, non-thread-safe controller. -
lifecycleScope.launchdispatches asynchronously even for the main body; betweenonStart.onStart()and entering the coroutine, anothernavigate()can be invoked, potentially overlapping work. Consider whether invocations must be serialized.
At minimum, consider job?.cancelAndJoin() from a runBlocking/suspend context at cancel time, or add an AtomicBoolean "navigating" guard and block new navigate calls until the previous step returns. Also consider capturing job before launch returns in a way that guarantees cancel() observes the latest job.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/org/commcare/activities/AsyncFormNavigator.kt` around lines 34 - 50,
The navigate/cancel flow leaves blocking Java work uncontrolled and races stale
results; fix by (1) running the blocking stepWork.step() on a dedicated
single-thread ExecutorService (store its Future in a field like stepFuture)
instead of relying on cooperative cancellation, call stepFuture.cancel(true) in
cancel() to attempt to interrupt the background thread, and ensure you stop any
FormEntryController mutations from that thread; (2) assign the coroutine Job to
job immediately by doing val assignedJob = lifecycleOwner.lifecycleScope.launch
{ ... } then job = assignedJob (or construct a Job and pass it into launch) so
cancel() always sees the latest job, and make cancel perform
job?.cancelAndJoin() (use runBlocking if cancel() must be synchronous) to wait
for the coroutine to finish; and (3) add a simple AtomicBoolean navigating guard
in navigate() to serialize invocations (set true at start, false on completion)
to prevent overlap between onStart/onResult windows. Ensure references to
stepWork.step(), lifecycleOwner.lifecycleScope.launch, job, currentNavId,
cancel(), and stepFuture/Future are updated accordingly.
| private NavResult stepToRenderableEvent() { | ||
| try { | ||
| while (true) { | ||
| int event = FormEntryActivity.mFormController.stepToNextEvent(FormEntryController.STEP_OVER_GROUP); | ||
| switch (event) { | ||
| case FormEntryController.EVENT_QUESTION: | ||
| return NavResult.Question.INSTANCE; | ||
| case FormEntryController.EVENT_END_OF_FORM: | ||
| return NavResult.EndOfForm.INSTANCE; | ||
| case FormEntryController.EVENT_PROMPT_NEW_REPEAT: | ||
| return NavResult.PromptNewRepeat.INSTANCE; | ||
| case FormEntryController.EVENT_GROUP: | ||
| //We only hit this event if we're at the _opening_ of a field | ||
| //list, so it seems totally fine to do it this way, technically | ||
| //though this should test whether the index is the field list | ||
| //host. | ||
| if (FormEntryActivity.mFormController.indexIsInFieldList() | ||
| && FormEntryActivity.mFormController.getQuestionPrompts().length != 0) { | ||
| return NavResult.FieldListGroup.INSTANCE; | ||
| } | ||
| // otherwise it's not a field-list group, so just skip it | ||
| break; | ||
| case FormEntryController.EVENT_REPEAT: | ||
| Log.i(TAG, "repeat: " + FormEntryActivity.mFormController.getFormIndex().getReference()); | ||
| // skip repeats | ||
| break; | ||
| case FormEntryController.EVENT_REPEAT_JUNCTURE: | ||
| Log.i(TAG, "repeat juncture: " | ||
| + FormEntryActivity.mFormController.getFormIndex().getReference()); | ||
| // skip repeat junctures until we implement them | ||
| break; | ||
| default: | ||
| Log.w(TAG, | ||
| "JavaRosa added a new EVENT type and didn't tell us... shame on them."); | ||
| break; | ||
| } | ||
| } | ||
| } catch (XPathException e) { | ||
| return new NavResult.Error(e); | ||
| } | ||
| } |
There was a problem hiding this comment.
Only XPathException is caught — other runtime exceptions from stepping will crash the coroutine.
FormEntryController.stepToNextEvent, indexIsInFieldList, and getQuestionPrompts can throw other unchecked exceptions (e.g., XPathTypeMismatchException extends XPathException so OK, but generic RuntimeException from evaluator or IllegalStateException do not). On the old synchronous path those propagated up to callers that wrapped the flow with UserfacingErrorHandling. Here, an uncaught exception inside the Dispatchers.Default coroutine will be swallowed/logged by the coroutine exception handler and the overlay will never be hidden, leaving the UI stuck with navigationInFlight=true and the overlay visible after 150 ms.
Consider widening the catch to Throwable (or at least RuntimeException) and surfacing via NavResult.Error, or wrapping the navigate callback to guarantee navigationInFlight/overlay are cleared on any failure.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/org/commcare/activities/FormEntryActivityUIController.java` around
lines 449 - 489, The stepToRenderableEvent method currently only catches
XPathException, so unchecked exceptions from
FormEntryController.stepToNextEvent/indexIsInFieldList/getQuestionPrompts can
crash the coroutine and leave navigationInFlight/overlay stuck; widen the catch
to Throwable (or RuntimeException) in stepToRenderableEvent and return new
NavResult.Error(throwable) so all failures surface as NavResult.Error, or
alternatively wrap the caller's navigate callback to always clear
navigationInFlight and hide the overlay in a finally block—update references in
stepToRenderableEvent, NavResult.Error, and the navigate/navigationInFlight
overlay handling to ensure any thrown Throwable is converted to NavResult.Error
or always clears the in-flight state.
1 participant
Product Description
When tapping Next in a form, the CommCare client previously froze the
UI while it walked the form model between renderable events — on forms
with heavy XPath evaluation (complex relevance, calculations, lookups)
this produced noticeable stalls and dropped frames. With this change,
that walk runs on a background dispatcher and a "Loading next question…"
overlay appears after 150 ms on slow steps (invisible on fast ones).
Rapid double-taps now advance a single question instead of double-stepping.
Technical Summary
No ticket; produced from the off-main-thread form navigation plan.
NavResult(sealed class) models the outcome of a step:Question,FieldListGroup,PromptNewRepeat,EndOfForm,Error(XPathException).showNextViewsplit intostepToRenderableEvent()(pure controllerwalk, returns
NavResult) andrenderNavResult()(view work on Main).AsyncFormNavigator(new Kotlin) runs the stepper viawithContext(Dispatchers.Default)onlifecycleScope.launch, retainsthe launched
Job, and uses a monotoniccurrentNavIdto dropresults whose navigation was superseded or cancelled.
FormLoadingOverlay(new Kotlin) posts a 150 ms-delayedVISIBLEso fast navigations don't flash;
hide()cancels any pending post.navigationInFlightlives onFormEntryActivityUIController;shouldIgnoreNavigationAction()checks it so Next/Previous/swipeare dropped during an in-flight step.
FormEntryActivity.onPause()callsuiController.cancelNavigation()which cancels the navigator, clears the flag, and hides the overlay —
prevents a late result firing against a paused UI.
form.entry.loading.next.questionadded for the overlay.Out of scope / deferred:
createView()/showView()still run on Main (view inflation must).showPreviousView/refreshCurrentView, and apendingNavigationAfterRestoreflagfor config-change mid-nav resumption.
Safety Assurance
Safety story
AsyncFormNavigatorguarantees late/stale resultsare dropped, including results that resume past a
Job.cancel()due to cooperative cancellation.
cancelNavigation()ononPause()prevents a background step fromdelivering a result after the activity is paused.
shouldIgnoreNavigationAction()blocks additional Next/Previousduring an in-flight step, so the main thread is idle between the
showNextViewprologue and the result callback.FormController/FormDefare not internally synchronized; correctness relies onthis invariant — other main-thread code paths that touch the form
controller (save-answers, etc.) run either before
navigate()orafter the result callback.
through
UserfacingErrorHandlingviaNavResult.Error.refresh-current-view, and repeat-dialog entry are unchanged.
Automated test coverage
None added in this PR. Follow-up to add Robolectric coverage for:
delayed-overlay visibility, superseded-result drop, cancel-on-pause,
and dropped rapid double-tap. Plus an Espresso test exercising a
relevance-heavy form.
QA Plan
steps, renders correct next question afterward.
EVENT_PROMPT_NEW_REPEATopens the add-repeat dialog.(Mid-step resumption is a known limitation — deferred follow-up.)
Labels and Review
RELEASES.md is updated accordingly
If yes, RELEASES.md is updated accordingly
level of risk of the change