Repository navigation
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
rdlabo
left a comment
There was a problem hiding this comment.
Thank you for investigating the tap-time flash, testing it on an iPhone SE, and keeping the change focused. The contribution is very much appreciated, and calling out the changed destroy behavior in the PR description was especially helpful.
The motivation makes sense: today, suspension restores the Web controls before the slide starts, leaving an interval in which the static Web header can become visible. However, the current ordering also protects the transition lifecycle. Moving the wait into finally does not preserve that protection: Ionic determines transition completion through onFinish, rather than awaiting the promise returned by play(), and destroys the animation after completion.
I compared the original implementation and this PR in Chromium using real Ionic Web Animations and a deliberately delayed suspension. Two regressions reproduced around packages/theme-ios27/src/transition/native-ui.ts:61–67:
- A successful transition can resume as canceled. With a 20 ms animation and a 200 ms suspension, reproducing Ionic's finish-then-destroy lifecycle produced
step = 1in both versions, but the original calledresume(false)and this PR calledresume(true). When suspension eventually settles,disposedis already true. For Vertical Bars, the canceled path marks the entering page as departed, even though navigation completed successfully. - Destroying during preparation can leave
play()pending. The original has not started playback yet and returns after preparation completes. This PR has already started playback; destroying the animation removes its completion callbacks, leaving the awaited playback promise unresolved. In the browser comparison, it remained pending beyond the animation's configured duration, although the suspension lease itself was released.
The existing 11 tests pass, but the mock play() resolves immediately and does not exercise this destruction behavior with Web Animations enabled.
I would keep suspension-before-play ordering for now and address the visible handoff gap while preserving that guarantee. If overlapping the two operations is necessary, successful completion and cancellation need to remain distinct through late suspension and teardown, with browser regression coverage for both cases above. The flash is worth fixing; this particular ordering change needs those lifecycle issues resolved before merging.
|
Thank you again for investigating the flash and proposing this change. Your contribution prompted a useful look at the presentation model itself, which I have summarized in #296 with simulator recordings. There may be a more fundamental direction to explore before changing the suspend/play ordering: keeping the native bar present while its items transition between pages, instead of restoring Web controls and projecting them again around each navigation. In a UIKit-only reference app, standard This is not yet a working Ionic integration. A standalone My inclination is to investigate that direction first, since it may remove the handoff responsible for the flash rather than shorten its visible interval. The completion/cancellation and pending-play concerns in my earlier review still apply to this patch. This does not take away from the contribution—the flash you identified is real, and this investigation builds on it. There is no expectation that you take on the architectural work in this PR. |
|
Thanks a lot for going deeper into this, can i help you in any way? |
|
Thank you for offering to help, and for identifying the flash in the first place. The replacement approach discussed in #296 is now implemented in #315 and released in v2.0.1. Supported toolbar controls remain native during navigation, with button correspondence and progress synchronization, while retaining the preparation-before-play ordering and cancellation safeguards we discussed here. Could you try 2.0.1 on your iPhone SE and let us know whether it resolves the flash you reported and matches the behavior you intended with this PR? Please update the packages, rebuild the Web assets, run |
Summary
withNativeUIShellTransitioncurrently awaitsprepare()(the suspend) beforeplay(). That retires the native controls while the page is still at rest, so Web shows for a frame before the slide starts. On a real iPhone SE (iOS 26) this is visible as a flash of the header segment or back button on tap.This starts
prepare()andplay()together and awaitsprepare()beforefinish(). Nothing else changes: Native UI, the bridge and Swift are untouched. The change is in the theme, which owns transition coordination since #282.Follow-up to the discussion in #266 (closed). Anything beyond this, such as visibility, measurement or handoff duration, is deliberately not included.
Behavior change
play()is now called before the suspension settles.destroy()during preparation can now happen afterplay()has already started. The test "destroy during bridge preparation cannot start an abandoned transition" had to change from['destroy', 'resume:true']to['play', 'destroy', 'resume:true']. Is that acceptable, or should an abandoned transition be guarded another way?Validation
native-ui-shell-transition.spec.ts: 11 passed (play starts before suspension settles; destroy case updated).build:tsand Prettier pass on the changed files.npm run test:esmand the demo Sass build fail on currentmainas well (missingutils/structured-list), with or without this change. I could not run the full demo or browser suites for that reason.packages/revision.