From 7418f4ed0dba2a6762716472aa8c596c3e4ab498 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Fri, 29 May 2026 19:22:21 +0300 Subject: [PATCH 1/2] 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) --- .../components/InteractionDialog.java | 125 ++++++++++++++++-- .../components/InteractionDialogTest.java | 71 ++++++++++ 2 files changed, 188 insertions(+), 8 deletions(-) diff --git a/CodenameOne/src/com/codename1/components/InteractionDialog.java b/CodenameOne/src/com/codename1/components/InteractionDialog.java index e1a85034bb..cf07d4ec25 100644 --- a/CodenameOne/src/com/codename1/components/InteractionDialog.java +++ b/CodenameOne/src/com/codename1/components/InteractionDialog.java @@ -84,6 +84,9 @@ public void run() { private boolean repositionAnimation = true; private boolean disposed; private boolean disposeWhenPointerOutOfBounds; + private int animationSpeed = -1; + private Runnable showAnimationSetup; + private Runnable disposeAnimationSetup; /// Whether the interaction dialog uses the form layered pane of the regular layered pane private boolean formMode; @@ -259,6 +262,13 @@ public Label getTitleComponent() { return title; } + private int resolveAnimationSpeed() { + if (animationSpeed >= 0) { + return animationSpeed; + } + return getUIManager().getThemeConstant("interactionDialogSpeedInt", 400); + } + private void cleanupLayer(Form f) { if (formMode) { Container c = f.getFormLayeredPane(InteractionDialog.class, true); @@ -327,7 +337,7 @@ public void resize(final int top, final int bottom, final int left, final int ri getParent().setWidth(getWidth()); getParent().setHeight(getHeight()); - getLayeredPane(f).animateLayout(getUIManager().getThemeConstant("interactionDialogSpeedInt", 400)); + getLayeredPane(f).animateLayout(resolveAnimationSpeed()); } } @@ -371,9 +381,11 @@ public void show(int top, int bottom, int left, int right) { getLayeredPane(f).addComponent(BorderLayout.center(this)); if (animateShow) { - int x = left + (f.getWidth() - right - left) / 2; - int y = top + (f.getHeight() - bottom - top) / 2; - if (repositionAnimation) { + if (showAnimationSetup != null) { + showAnimationSetup.run(); + } else if (repositionAnimation) { + int x = left + (f.getWidth() - right - left) / 2; + int y = top + (f.getHeight() - bottom - top) / 2; getParent().setX(x); getParent().setY(y); getParent().setWidth(1); @@ -386,7 +398,7 @@ public void show(int top, int bottom, int left, int right) { getParent().setWidth(getWidth()); getParent().setHeight(getHeight()); } - getLayeredPane(f).animateLayout(getUIManager().getThemeConstant("interactionDialogSpeedInt", 400)); + getLayeredPane(f).animateLayout(resolveAnimationSpeed()); } else { //getLayeredPane(f).revalidate(); f.revalidateWithAnimationSafety(); @@ -423,13 +435,15 @@ public void dispose() { Form f = p.getComponentForm(); if (f != null) { if (animateShow) { - if (repositionAnimation) { + if (disposeAnimationSetup != null) { + disposeAnimationSetup.run(); + } else if (repositionAnimation) { setX(getX() + getWidth() / 2); setY(getY() + getHeight() / 2); setWidth(1); setHeight(1); } - p.animateUnlayoutAndWait(getUIManager().getThemeConstant("interactionDialogSpeedInt", 400), 100); + p.animateUnlayoutAndWait(resolveAnimationSpeed(), 100); } Container pp = getLayeredPane(f); remove(); @@ -542,7 +556,7 @@ private void disposeTo(int direction, final Runnable onFinish) { } if (animateShow) { - p.animateUnlayout(getUIManager().getThemeConstant("interactionDialogSpeedInt", 400), 255, new Runnable() { + p.animateUnlayout(resolveAnimationSpeed(), 255, new Runnable() { @Override public void run() { if (p.getParent() != null) { @@ -615,6 +629,101 @@ public void setAnimateShow(boolean animateShow) { this.animateShow = animateShow; } + /// Duration in milliseconds used by the show, dispose and resize animations. + /// When set to a non-negative value this overrides the + /// `interactionDialogSpeedInt` theme constant. The default is -1 which means + /// "defer to the theme constant" (which itself defaults to 400ms). + /// + /// #### Returns + /// + /// the animation speed in ms, or -1 if the theme constant is used + public int getAnimationSpeed() { + return animationSpeed; + } + + /// Sets the duration in milliseconds used by the show, dispose and resize + /// animations, overriding the `interactionDialogSpeedInt` theme constant. Pass + /// any value < 0 (typically -1) to revert to the theme constant. + /// + /// #### Parameters + /// + /// - `animationSpeed`: animation duration in ms, or a value < 0 to defer to the theme constant + public void setAnimationSpeed(int animationSpeed) { + this.animationSpeed = animationSpeed; + } + + /// Callback invoked just before the show animation runs to position the dialog + /// parent at the animation start state. When set, this replaces the default + /// `#setRepositionAnimation(boolean)` behavior (grow from a 1x1 point at the + /// center, or stay at full size). Inside the callback, manipulate + /// `getParent()` bounds (`setX`/`setY`/`setWidth`/`setHeight`) to define + /// where the dialog should animate from. The animation will then interpolate + /// the layered pane layout to the dialog's final bounds. Pass `null` (the + /// default) to use the built-in show animation. + /// + /// This callback only fires when `#isAnimateShow()` is true. + /// + /// This hook is the recommended workaround when using popup dialogs that + /// render a pointing-arrow border (`#showPopupDialog(com.codename1.ui.Component)`). + /// With the built-in "grow from 1x1" animation the dialog is too small for + /// the arrow image to render until the animation completes; providing a + /// translate-from-edge setup keeps the dialog at full size for the entire + /// animation so the arrow is visible throughout. For example, to slide in + /// from off-screen below: + /// + /// ```java + /// dlg.setShowAnimationSetup(() -> { + /// Container parent = dlg.getParent(); + /// parent.setY(Display.getInstance().getDisplayHeight()); + /// }); + /// ``` + /// + /// #### Returns + /// + /// the show animation setup callback or null + public Runnable getShowAnimationSetup() { + return showAnimationSetup; + } + + /// Sets a callback that positions the dialog parent at the animation start + /// state, overriding the default show animation. See `#getShowAnimationSetup()` + /// for details. + /// + /// #### Parameters + /// + /// - `showAnimationSetup`: callback or null to use the built-in show animation + public void setShowAnimationSetup(Runnable showAnimationSetup) { + this.showAnimationSetup = showAnimationSetup; + } + + /// Callback invoked just before the dispose animation runs to position the + /// dialog at the animation end state. When set, this replaces the default + /// `#setRepositionAnimation(boolean)` behavior (shrink to a 1x1 point at the + /// dialog center). Inside the callback, manipulate the dialog bounds + /// (`setX`/`setY`/`setWidth`/`setHeight`) to define where the dialog should + /// animate to. Pass `null` (the default) to use the built-in dispose + /// animation. + /// + /// This callback only fires when `#isAnimateShow()` is true. + /// + /// #### Returns + /// + /// the dispose animation setup callback or null + public Runnable getDisposeAnimationSetup() { + return disposeAnimationSetup; + } + + /// Sets a callback that positions the dialog at the animation end state, + /// overriding the default dispose animation. See `#getDisposeAnimationSetup()` + /// for details. + /// + /// #### Parameters + /// + /// - `disposeAnimationSetup`: callback or null to use the built-in dispose animation + public void setDisposeAnimationSetup(Runnable disposeAnimationSetup) { + this.disposeAnimationSetup = disposeAnimationSetup; + } + private void installPointerOutOfBoundsListeners() { final Form f = getComponentForm(); diff --git a/maven/core-unittests/src/test/java/com/codename1/components/InteractionDialogTest.java b/maven/core-unittests/src/test/java/com/codename1/components/InteractionDialogTest.java index 01b68a2784..5f54971954 100644 --- a/maven/core-unittests/src/test/java/com/codename1/components/InteractionDialogTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/components/InteractionDialogTest.java @@ -259,6 +259,77 @@ void showPopupDialogLandscapeFullWidthRectGetsVisibleSize() { } } + @Test + void animationSpeedDefaultsToThemeConstant() { + InteractionDialog dialog = new InteractionDialog(); + assertEquals(-1, dialog.getAnimationSpeed(), + "default should be -1 meaning 'use theme constant interactionDialogSpeedInt'"); + } + + @Test + void animationSpeedSetterStoresValue() { + InteractionDialog dialog = new InteractionDialog(); + dialog.setAnimationSpeed(1500); + assertEquals(1500, dialog.getAnimationSpeed()); + dialog.setAnimationSpeed(-1); + assertEquals(-1, dialog.getAnimationSpeed(), + "setting -1 reverts to the theme constant"); + } + + @Test + void showAnimationSetupRunsInsteadOfDefaultRepositionAnimation() { + // #5072: users need to customize show animations. The + // setShowAnimationSetup callback replaces the built-in + // "grow from 1x1 at center" behavior. Verify it runs and + // that the parent bounds we set inside it are preserved + // when the animation kicks off. + Form form = new Form(new BorderLayout()); + implementation.setCurrentForm(form); + InteractionDialog dialog = new InteractionDialog(); + final int[] callCount = {0}; + dialog.setShowAnimationSetup(new Runnable() { + @Override + public void run() { + callCount[0]++; + // Slide-from-bottom setup: full size, translated off-screen + Container parent = dialog.getParent(); + parent.setY(1000); + } + }); + dialog.show(0, 0, 0, 0); + assertEquals(1, callCount[0], "showAnimationSetup must run once on show()"); + assertSame(dialog.getShowAnimationSetup(), dialog.getShowAnimationSetup(), + "getter returns the stored callback"); + dialog.setShowAnimationSetup(null); + assertNull(dialog.getShowAnimationSetup(), "setShowAnimationSetup(null) clears the override"); + dialog.dispose(); + } + + @Test + void disposeAnimationSetupRunsInsteadOfDefaultRepositionAnimation() { + // #5072: dispose animation should be customizable too. + Form form = new Form(new BorderLayout()); + implementation.setCurrentForm(form); + InteractionDialog dialog = new InteractionDialog(); + dialog.setAnimateShow(false); + dialog.show(0, 0, 0, 0); + + final int[] callCount = {0}; + dialog.setDisposeAnimationSetup(new Runnable() { + @Override + public void run() { + callCount[0]++; + } + }); + // Re-enable animation so dispose runs the animation path + // (and thus the dispose setup callback). + dialog.setAnimateShow(true); + dialog.dispose(); + assertEquals(1, callCount[0], "disposeAnimationSetup must run once on dispose()"); + dialog.setDisposeAnimationSetup(null); + assertNull(dialog.getDisposeAnimationSetup(), "setDisposeAnimationSetup(null) clears the override"); + } + private T getPrivateField(Object target, String name, Class type) throws Exception { Field field = target.getClass().getDeclaredField(name); field.setAccessible(true); From da7b09e865e123af02b69f96b91e4a524e13ff7f Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Fri, 29 May 2026 19:52:12 +0300 Subject: [PATCH 2/2] 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) --- .../ui/spinner/PickerCoverageTest.java | 51 ++++++++++++++++--- 1 file changed, 43 insertions(+), 8 deletions(-) diff --git a/maven/core-unittests/src/test/java/com/codename1/ui/spinner/PickerCoverageTest.java b/maven/core-unittests/src/test/java/com/codename1/ui/spinner/PickerCoverageTest.java index 62f306bfe9..4603df7790 100644 --- a/maven/core-unittests/src/test/java/com/codename1/ui/spinner/PickerCoverageTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/ui/spinner/PickerCoverageTest.java @@ -41,16 +41,20 @@ private void waitForForm(Form form) { } private void runAnimations(Form f) { + // Driving an AnimationManager animation in test mode needs each EDT + // tick to actually execute an edtLoopImpl pass (that is what runs + // repaintAnimations -> AnimationManager.updateAnimations). flushEdt + // alone is a no-op when there is no pending input/serial work, so + // we queue a no-op serial call before each flush to force one pass. long start = System.currentTimeMillis(); - while (System.currentTimeMillis() - start < 400) { - if (f != null) { - f.animate(); - f.layoutContainer(); - } + while (System.currentTimeMillis() - start < 600) { + CN.callSerially(new Runnable() { + @Override + public void run() { + // no-op: only here to force one edtLoopImpl pass + } + }); DisplayTest.flushEdt(); - if (f != null) { - f.revalidate(); - } try { Thread.sleep(10); } catch (InterruptedException e) {} @@ -75,6 +79,31 @@ private void runAnimationsUntil(Form f, AtomicBoolean condition) { } } + private void runUntilFocusedOrEditing(Form f, Component target) { + // #5092 routes lightweight picker next/prev focus transfer through + // disposeToTheBottom(Runnable), so the focus assertion has to wait + // for the dispose animation to finish and its completion callback + // to run. Driving the animation in test mode requires pumping a + // serial call before each flushEdt() so the EDT actually runs an + // edtLoopImpl iteration (which is what calls repaintAnimations -> + // AnimationManager.updateAnimations); flushEdt by itself is a no-op + // when there is no pending work in the input/serial queues. + long start = System.currentTimeMillis(); + while (System.currentTimeMillis() - start < 3000) { + if (target.hasFocus() || target.isEditing()) return; + CN.callSerially(new Runnable() { + @Override + public void run() { + // no-op: only here to force one edtLoopImpl pass + } + }); + DisplayTest.flushEdt(); + try { + Thread.sleep(10); + } catch (InterruptedException e) {} + } + } + private String printTree(Component c, String indent) { StringBuilder sb = new StringBuilder(); sb.append(indent).append(c.getClass().getName()).append(" [name=").append(c.getName()).append("]\n"); @@ -146,6 +175,11 @@ public void testPickerNextPrevButtons() { nextButton.released(); DisplayTest.flushEdt(); + // #5092 deferred next/prev focus transfer until the dispose animation + // completes via the disposeToTheBottom(Runnable) callback, so the + // focus assertion has to wait for that animation rather than checking + // immediately after released(). + runUntilFocusedOrEditing(f, nextTf); Assertions.assertTrue(nextTf.hasFocus() || nextTf.isEditing(), "Next component should have focus/editing"); // Re-open picker @@ -166,6 +200,7 @@ public void testPickerNextPrevButtons() { prevButton.released(); DisplayTest.flushEdt(); + runUntilFocusedOrEditing(f, prevTf); Assertions.assertTrue(prevTf.hasFocus() || prevTf.isEditing(), "Prev component should have focus/editing"); // Re-open picker to test Done/Cancel