Skip to content

fix: skip the farewell frame when back() answers a system back - #370

Open
SRWieZ wants to merge 2 commits into
NativePHP:mainfrom
SRWieZ:fix/no-farewell-frame-on-system-back
Open

fix: skip the farewell frame when back() answers a system back#370
SRWieZ wants to merge 2 commits into
NativePHP:mainfrom
SRWieZ:fix/no-farewell-frame-on-system-back

Conversation

@SRWieZ

@SRWieZ SRWieZ commented Aug 24, 2026

Copy link
Copy Markdown
Member

The bug

With shared-stack chrome, backing out of a screen via the system back (back chevron, edge-swipe, hardware button) can flash the screen back in and immediately out again. It shows when the departing screen's farewell tree differs from its last publish and teardown spans a few frames. A byte-identical republish is dropped by the bridge diff, so that case never re-pushes.

Why

Three pieces, each fine on its own:

  1. back() publishes a farewell frame of the departing screen before setting the BACK intent, so a PHP-initiated pop has fresh content to show while the native side animates it.
  2. On a system back, native navigation initiated the pop and its path may already have shrunk before PHP receives sendSystemBackEvent.
  3. The coordinator reconciles a publish whose URI is no longer in the path as a brand-new push.

So for affected system backs: native navigation starts backing away → PHP answers with a changed farewell frame of the departing screen → the coordinator pushes it back on → the router publishes the level below → it disappears again.

The fix

The type-8 dispatch now goes through handleSystemBack(), which flags the native-system-back context. back() skips the farewell frame while that flag is set; PHP-initiated backs are untouched and keep their farewell frame. pressBack() in the test harness mirrors the production dispatch.

navigate() from onBackPressed() deliberately keeps its farewell publish because that re-push is what aligns the native path with PHP's resulting stack. On Android plain screens, the previous frame remains visible during teardown instead of publishing a fresh one; this affects only state mutated in onBackPressed() immediately before back(), for the teardown interval plus the next screen's onResume().

Tests

SystemBackFarewellTest now uses the EDGE suite's Pest style and pins five cases:

  • PHP-initiated back publishes one farewell frame.
  • A production system back publishes none.
  • The system-back flag resets if onBackPressed() throws.
  • Harness pressBack() publishes no farewell frame.
  • A PHP-initiated harness back still publishes one.

Full suite: 901 passed. Pint and PHPStan clean.

back() publishes one last frame of the departing screen so a
PHP-initiated pop has fresh content to show during the animation. When
the back comes FROM the system (hardware button, back chevron,
edge-swipe pop), the native pop has already animated and the
coordinator's path has already shrunk — the farewell frame then arrives
as an unknown URI and is reconciled as a brand-new push: the screen
flashes back in, then out again once the router publishes the level
below. Visible whenever a screen's teardown spans a few frames.

The type-8 dispatch now goes through handleSystemBack(), which flags
the context so back() skips the farewell; PHP-initiated backs keep it.
The test harness's pressBack() mirrors the production dispatch.
@SRWieZ SRWieZ changed the title Skip the farewell frame when back() answers a system back fix: skip the farewell frame when back() answers a system back Aug 24, 2026
@gwleuverink

gwleuverink commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Nice find 🔥 Reproduced on simulators, fixed by this branch on both platforms.

On iOS the effects were the worst, it shows on every system back with no slow teardown at all, the departed screen sits under the root's nav bar for about 300 ms before snapping. Android is clean at the natural 20 ms gap, hitches from about 30 ms of teardown work and bounces fully from 100 ms.

Worth adding to the description: it only shows when the farewell tree differs from the last publish. A byte-identical republish is dropped by the bridge diff and never re-pushes.

One test & two docblock updates I suggest before merge:

  • The harness change is untested; reverting pressBack()/goBack() to onBackPressed() leaves the suite green. This pair in SystemBackFarewellTest pins it (needs Native\Mobile\Testing\Native and Tests\Fixtures\Edge\DetailScreen imported; the first one fails on the reverted harness, both pass on this branch):
public function test_the_harness_back_press_skips_the_farewell_frame_too(): void
{
    $bridge = Native::fakeBridge();

    $screen = Native::test(DetailScreen::class);
    $mounted = count($bridge->publishes);

    $screen->pressBack()->assertWentBack();

    $this->assertCount($mounted, $bridge->publishes);
}

public function test_the_harness_still_publishes_a_farewell_frame_for_a_php_initiated_back(): void
{
    $bridge = Native::fakeBridge();

    $screen = Native::test(DetailScreen::class);
    $mounted = count($bridge->publishes);

    $screen->tap('Go back')->assertWentBack();

    $this->assertCount($mounted + 1, $bridge->publishes);
}
  • The afterInteraction() docblock in TestableComponent still claims the final state was published by publishFinalState().
  • "the native pop has ALREADY animated" overstates it: the tabs chevron, the stack root chevron and Android's plain-screen hardware back hit the same flag with nothing animated.

Two things I checked so they don't need a decision: navigate() from onBackPressed() should keep its farewell, the re-push is what keeps the native path aligned with PHP's stack there (both end on [root, detail, other]). And the plain-screen Android case now shows the previous frame during teardown instead of a fresh one; only state mutated in onBackPressed() before back() is affected, for about 20 ms plus the next screen's onResume(). Fine by me, worth a sentence in the description.

@gwleuverink

Copy link
Copy Markdown
Contributor

Small sidenote, not for this PR: the suite has two styles. The build and compiler tests are PHPUnit classes from the original codebase, everything around EDGE and the testing harness written since July is Pest.

SystemBackFarewellTest is the first EDGE test as a class. Works fine either way since pest runs both, just something to keep in mind for the next one so the EDGE side stays uniform.

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.

2 participants