Skip to content

fix(navigation): guard forward navigation against repeat taps - #57

Merged
aoreshkov merged 2 commits into
mainfrom
fix/single-top-navigation
Oct 9, 2026
Merged

aoreshkov merged 2 commits into
mainfrom
fix/single-top-navigation

Conversation

@aoreshkov

Copy link
Copy Markdown
Owner

Follow-up to #56, which made back navigation pop only its own entry. This PR covers the forward direction with two complementary guards.

Problems

  1. Duplicate entries in the two-pane layout. Tapping the selected posting row again (or the add/edit FAB twice) pushed a second entry with the same key. It shares the first entry's content key, so it also shares its ViewModel and saved state. Nothing visibly changes, and the user then has to press back twice to leave.
  2. Taps during a single-pane transition. On Android and iOS the outgoing screen stays composed and clickable while the scene animates. Tapping a second row during the slide pushed another screen on top.

Changes

  • Navigator.goTo is single-top: a no-op while the destination is already the current section's top entry. It is a pure back-stack check, so it works in every layout and at any timing.
  • dropUnlessResumed wraps the three forward-navigation clicks (posting row, add FAB, edit FAB), following the Android UI-events guidance. Navigation 3 caps entries in a transitioning scene at STARTED, so the second tap is dropped.

dropUnlessResumed alone would not fix problem 1. With two panes, ListDetailSceneStrategy keeps one scene key for the whole list-detail stack, so NavDisplay never transitions and every entry stays RESUMED. In turn, goTo single-top alone cannot catch problem 2, where the destinations are different. Back arrows keep goBack(from) from #56; wrapping them as well would be redundant.

On desktop, NavDisplay's default transition is None, so the single-pane window does not exist there. Only the single-top check matters on desktop.

Tests

  • NavigatorTest: repeated goTo(same) adds once; the same key below the top still appends; goTo(sectionRoot) is a no-op.
  • Screen tests host each screen at STARTED and assert the click is dropped (list row, add FAB, edit FAB). Mutation-checked: removing one guard fails exactly one test.
    • The test owner uses LifecycleRegistry.createUnsafe. On desktop, the regular registry's main-thread check runs runBlocking(Dispatchers.Main), which deadlocks in test classes that swap Main for a StandardTestDispatcher.
  • Run locally, all passing: :core:navigation:check, :feature:posting:impl:check, and posting jvmTest and testAndroidHostTest.

No public API change (the goTo signature is unchanged), so no apiDump.

🤖 Generated with Claude Code

aoreshkov and others added 2 commits October 10, 2026 00:09
Navigator.goTo always appended. In the two-pane list-detail layout the list
stays on screen and the scene never transitions, so tapping the selected row
again (or the add/edit FAB twice) pushed a second entry with the same key.
It shares the first one's content key, ViewModel and saved state, so nothing
visibly changes and the user has to press back twice to leave it. A lifecycle
guard such as dropUnlessResumed cannot catch this: every entry in that scene
stays RESUMED.

Make goTo single-top: a no-op while the destination is already the current
section's top entry.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
During a single-pane scene transition (Android and iOS animate it) the
outgoing screen stays composed and clickable. Tapping a second posting
row, or the add/edit FAB again, while the first navigation animates
pushed another screen on top. Navigation 3 caps every entry in a
transitioning scene at STARTED, so wrap the three forward-navigation
clicks in dropUnlessResumed, as the Android UI-events guidance does.

goTo's single-top check already covers a repeat of the same destination
in every layout; this covers different destinations during the
transition. Back arrows stay on goBack(from).

The negative tests host the screen at STARTED through
LifecycleRegistry.createUnsafe: on desktop, the registry's main-thread
check runs runBlocking(Dispatchers.Main) and deadlocks under a swapped
StandardTestDispatcher.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown

Code Coverage

Overall Project 98.48% 🍏
Files changed 100% 🍏

File Coverage
Navigator.kt 100% 🍏
PostingDetailsScreen.kt 98.49% 🍏
PostingListScreen.kt 98.42% 🍏

@aoreshkov
aoreshkov merged commit 1821777 into main Oct 9, 2026
14 checks passed
@aoreshkov
aoreshkov deleted the fix/single-top-navigation branch October 9, 2026 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant