Defer lightweight Picker traversal editing until dismiss completes#5092
Conversation
jsfan3
force-pushed
the
codex/defer-lightweight-picker-traversal-edit
branch
from
f39b770 to
30dcb33
Compare
|
@shai-almog Please wait before accepting this PR; I have a comment to make that I'll post shortly... |
|
@shai-almog Small clarification about verification status. This fix is based on a reproduced Android issue and on code-path analysis of the lightweight The diagnosis and patch were prepared with assistance from generative AI. The observed behavior is real and reproducible in my app: traversing from a lightweight If you think this needs direct validation before review/merge, I can test it properly by building Codename One locally with this patch and running the app against that patched build. That may take some time, but I can do it. |
|
@jsfan3 we have CI that checks most of these things and is now pretty thorough (albeit not perfect). The problem is that it's really hard to run that CI on 3rd party PRs... Even when I accept the PR and let CI run some things fail due to the complexity of our CI scripts and security concerns. Due to that I'd much rather get bug reports than PRs. I reviewed this and it looks safe so I'll merge it and it will run in CI post merging. |
…asserts PR #5092 routed lightweight Picker next/prev focus transfer through disposeToTheBottom(Runnable), and Done/Cancel still rely on the disposeToTheBottom() animation finishing, so the Next/Prev focus assertions and Done/Cancel "dialog closed" assertions in testPickerNextPrevButtons can no longer be checked synchronously after released() + flushEdt(). DisplayTest.flushEdt() by itself does not drive AnimationManager animations: it only iterates edtLoopImpl while shouldEDTSleepNoFormAnimation() is false, which is only the case when there is pending input or serial work. Queueing a no-op callSerially before each flushEdt forces one edtLoopImpl pass per loop iteration, which in turn runs Form.repaintAnimations and advances the dispose animation. The runAnimations helper now uses that pattern (with a longer 600ms budget to cover the 400ms dispose animation plus completion-callback latency), and a new runUntilFocusedOrEditing helper polls until the target picks up focus or starts editing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
#5097) * feat(InteractionDialog): expose animation speed and custom show/dispose setup (#5072) Allow callers to override the interactionDialogSpeedInt theme constant in code and to replace the built-in grow/shrink animation with custom positioning, addressing the request in #5072. The arrow-border-missing-during-slow-animation note in #5072 is a side-effect of the default reposition-from-1x1 animation: at the start of the animation the dialog is too small for the arrow image to render. The new setShowAnimationSetup docs include a translate-from-edge recipe that keeps the dialog at full size throughout, so the arrow stays visible. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(PickerCoverageTest): drive dispose animation before focus/close asserts PR #5092 routed lightweight Picker next/prev focus transfer through disposeToTheBottom(Runnable), and Done/Cancel still rely on the disposeToTheBottom() animation finishing, so the Next/Prev focus assertions and Done/Cancel "dialog closed" assertions in testPickerNextPrevButtons can no longer be checked synchronously after released() + flushEdt(). DisplayTest.flushEdt() by itself does not drive AnimationManager animations: it only iterates edtLoopImpl while shouldEDTSleepNoFormAnimation() is false, which is only the case when there is pending input or serial work. Queueing a no-op callSerially before each flushEdt forces one edtLoopImpl pass per loop iteration, which in turn runs Form.repaintAnimations and advances the dispose animation. The runAnimations helper now uses that pattern (with a longer 600ms budget to cover the 400ms dispose animation plus completion-callback latency), and a new runUntilFocusedOrEditing helper polls until the target picks up focus or starts editing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 participants
Summary
Pickerprevious/next focus transfer until the popup dismiss animation has completed.disposeToTheBottom()completion callback instead of immediately after requesting the dismiss.Problem
The lightweight
Pickersupports previous/next traversal buttons. When a traversal button is pressed,endEditing()currently callsdlg.disposeToTheBottom()and then immediately requests focus and callsstartEditingAsync()on the next component.InteractionDialog#disposeToTheBottom()is animated. On Android,startEditingAsync()creates and positions a nativeEditTextoverlay for the targetTextField. If that happens while the picker popup is still being dismissed and the form content pane has only just been restored from the picker padding/invisible-area adjustment, the native editor can be positioned using stale geometry. In a reproduced Android case, traversing from a lightweight picker to a numericTextFieldopened the numeric keyboard but rendered the field value outside of the field.Fix
For traversal commands that resolve to a next/previous component, this patch passes a completion callback to
disposeToTheBottom()and starts editing only after the lightweight picker has finished dismissing. Cancel and Done without traversal continue to dismiss normally.This keeps the existing traversal behavior, but avoids starting the next native edit session while the outgoing picker popup is still in its dismiss/layout transition.
Verification
Pickerfollowed by numericTextFields andpicker.setTraversable(true).git diff --checkon the branch.