diff --git a/CodenameOne/src/com/codename1/ui/spinner/Picker.java b/CodenameOne/src/com/codename1/ui/spinner/Picker.java index 542226b878..b5be0ac298 100644 --- a/CodenameOne/src/com/codename1/ui/spinner/Picker.java +++ b/CodenameOne/src/com/codename1/ui/spinner/Picker.java @@ -120,6 +120,10 @@ public class Picker extends Button { private Runnable stopEditingCallback; private boolean suppressPaint; private final ArrayList lightweightPopupButtons = new ArrayList(); + /// While the lightweight popup is on screen this points at the live spinner widget + /// so that setters propagate into the visible wheels and getters read the wheel + /// position. Null whenever the popup is not showing. + private InternalPickerWidget currentSpinner; /// Placement options for custom lightweight popup buttons. public static final class LightweightPopupButtonPlacement { @@ -145,6 +149,24 @@ private LightweightPopupButton(String text, Runnable action, int placement, int } } + /// Listener fired when a custom popup button is pressed. Static (rather than an anonymous + /// inner class) so it does not retain a reference to the enclosing `Picker`; the only + /// state it needs is the matching `LightweightPopupButton` whose `action` it invokes. + private static final class PopupButtonActionListener implements ActionListener { + private final LightweightPopupButton popupButton; + + private PopupButtonActionListener(LightweightPopupButton popupButton) { + this.popupButton = popupButton; + } + + @Override + public void actionPerformed(ActionEvent evt) { + if (popupButton.action != null) { + popupButton.action.run(); + } + } + } + /// Default constructor public Picker() { setUIIDFinal("Picker"); @@ -463,6 +485,12 @@ private DurationSpinner3D createDurationPicker3D() { private void endEditing(int command, InteractionDialog dlg, InternalPickerWidget spinner) { currentInput = null; + if (currentSpinner == spinner) { //NOPMD CompareObjectsWithEquals + // Clear before reading so external action listeners triggered by + // fireActionEvent see getDate() return the just-committed value + // rather than the (about-to-disappear) wheel state. + currentSpinner = null; + } restoreContentPane(); dlg.disposeToTheBottom(); if (command != COMMAND_CANCEL) { @@ -516,6 +544,7 @@ private void showInteractionDialog() { default: throw new IllegalArgumentException("Unsupported picker type " + type); } + currentSpinner = spinner; final InteractionDialog dlg = new InteractionDialog() { ActionListener keyListener; @@ -588,8 +617,8 @@ protected void deinitialize() { .setBgTransparency(0) .setMargin(0) .setPaddingMillimeters(3f, 0); - Container topCustomButtons = createLightweightPopupButtonRow(spinner, LightweightPopupButtonPlacement.ABOVE_SPINNER, isTablet); - Container bottomCustomButtons = createLightweightPopupButtonRow(spinner, LightweightPopupButtonPlacement.BELOW_SPINNER, isTablet); + Container topCustomButtons = createLightweightPopupButtonRow(LightweightPopupButtonPlacement.ABOVE_SPINNER, isTablet); + Container bottomCustomButtons = createLightweightPopupButtonRow(LightweightPopupButtonPlacement.BELOW_SPINNER, isTablet); if (topCustomButtons != null || bottomCustomButtons != null) { Container spinnerSection = new Container(new BorderLayout()); spinnerSection.add(BorderLayout.CENTER, wrapper); @@ -685,7 +714,7 @@ public void actionPerformed(ActionEvent evt) { west.add(nextButton); } - Container centerButtons = createLightweightPopupButtonRow(spinner, LightweightPopupButtonPlacement.BETWEEN_CANCEL_AND_DONE, isTablet); + Container centerButtons = createLightweightPopupButtonRow(LightweightPopupButtonPlacement.BETWEEN_CANCEL_AND_DONE, isTablet); Container buttonBar = BorderLayout.centerEastWest(centerButtons, doneButton, west); buttonBar.setUIID(isTablet ? "PickerButtonBarTablet" : "PickerButtonBar"); dlg.getContentPane().add(BorderLayout.NORTH, buttonBar); @@ -759,7 +788,7 @@ public void actionPerformed(ActionEvent evt) { updateValue(); } - private Container createLightweightPopupButtonRow(final InternalPickerWidget spinner, int placement, boolean isTablet) { + private Container createLightweightPopupButtonRow(int placement, boolean isTablet) { Container left = null; Container center = null; Container right = null; @@ -767,18 +796,8 @@ private Container createLightweightPopupButtonRow(final InternalPickerWidget spi if (entry.placement != placement) { continue; } - final LightweightPopupButton popupButton = entry; - Button button = new Button(popupButton.text, isTablet ? "PickerButtonTablet" : "PickerButton"); - button.addActionListener(new ActionListener() { - @Override - public void actionPerformed(ActionEvent evt) { - if (popupButton.action != null) { - popupButton.action.run(); - } - spinner.setValue(value); - updateValue(); - } - }); + Button button = new Button(entry.text, isTablet ? "PickerButtonTablet" : "PickerButton"); + button.addActionListener(new PopupButtonActionListener(entry)); switch (entry.alignment) { case Component.CENTER: if (center == null) { @@ -1188,25 +1207,49 @@ public void setType(int type) { } } + /// Returns the value currently visible to the user: the live spinner wheel value while + /// the lightweight popup is showing, otherwise the committed `value` field. + private Object currentValue() { + if (currentSpinner != null) { + return currentSpinner.getValue(); + } + return value; + } + /// Returns the date, this value is used both for type date/date and time. Notice that this - /// value isn't used for time + /// value isn't used for time. + /// + /// While the lightweight popup is on screen this returns the value visible on the scroll + /// wheels (which only becomes the committed value once the user presses Done), so a custom + /// popup button can read it to compute a relative date. The native picker does not expose + /// the in-progress wheel state, so while a native picker is on screen this still returns + /// the last committed value. /// /// #### Returns /// /// the date object public Date getDate() { - return (Date) value; + return (Date) currentValue(); } /// Sets the date, this value is used both for type date/date and time. Notice that this - /// value isn't used for time. Notice that this value will have no effect if the picker - /// is currently showing. + /// value isn't used for time. + /// + /// If the lightweight popup is currently on screen the visible scroll wheels are also + /// moved to the new value, so a custom popup button can do `setDate(getDate() + n)` and + /// have the wheels reflect it (see `#addLightweightPopupButton(String, Runnable)`). + /// The native picker is read-only while shown - calling `setDate` against a Picker whose + /// native popup is open updates the committed `value` but leaves the on-screen wheels + /// unchanged until the user dismisses and re-opens the picker. /// /// #### Parameters /// /// - `d`: the new date public void setDate(Date d) { value = d; + if (currentSpinner != null) { + currentSpinner.setValue(d); + } updateValue(); } @@ -1332,6 +1375,7 @@ public void run() { @Override public void close() throws Exception { currentInput = null; + currentSpinner = null; if (sizeChanged != null) { f.removeSizeChangedListener(sizeChanged); } @@ -1458,33 +1502,44 @@ public void setStrings(String... strs) { updateValue(); } - /// Returns the current string + /// Returns the current string. While the lightweight popup is on screen this returns the + /// value visible on the scroll wheel; the native picker does not expose its in-progress + /// wheel state, so while a native picker is on screen this still returns the last + /// committed value. /// /// #### Returns /// /// the selected string public String getSelectedString() { - return (String) value; + return (String) currentValue(); } - /// Sets the current value in a string array picker + /// Sets the current value in a string array picker. If the lightweight popup is on screen + /// the visible scroll wheel is also moved to the new value; the native picker is read-only + /// while shown so against a native popup this updates only the committed value. /// /// #### Parameters /// /// - `str`: the current value public void setSelectedString(String str) { value = str; + if (currentSpinner != null) { + currentSpinner.setValue(str); + } updateValue(); } - /// Returns the index of the selected string + /// Returns the index of the selected string. While the lightweight popup is on screen + /// this returns the index visible on the scroll wheel; the native picker still returns + /// the last committed index. /// /// #### Returns /// /// the selected string offset or -1 public int getSelectedStringIndex() { + Object current = currentValue(); int offset = 0; - if (value == null) { + if (current == null) { for (String s : (String[]) metaData) { if (s == null) { return offset; @@ -1494,7 +1549,7 @@ public int getSelectedStringIndex() { return -1; } for (String s : (String[]) metaData) { - if (value.equals(s)) { + if (current.equals(s)) { return offset; } offset++; @@ -1502,13 +1557,18 @@ public int getSelectedStringIndex() { return -1; } - /// Returns the index of the selected string + /// Sets the index of the selected string. If the lightweight popup is on screen the + /// visible wheel is also moved to the new value; the native picker is read-only while + /// shown so against a native popup this updates only the committed value. /// /// #### Parameters /// /// - `index`: sets the index of the selected string public void setSelectedStringIndex(int index) { value = ((String[]) metaData)[index]; + if (currentSpinner != null) { + currentSpinner.setValue(value); + } updateValue(); } @@ -1603,23 +1663,30 @@ public void setTime(int hour, int minute) { } /// This value is only used for time type and is ignored in the case of date and time where - /// both are embedded within the date. + /// both are embedded within the date. While the lightweight popup is on screen this + /// returns the value visible on the scroll wheels; the native picker still returns the + /// last committed value. /// /// #### Returns /// /// the time value as minutes since midnight e.g. 630 is 10:30am public int getTime() { - return ((Integer) value).intValue(); + return ((Integer) currentValue()).intValue(); } /// This value is only used for time type and is ignored in the case of date and time where - /// both are embedded within the date. + /// both are embedded within the date. If the lightweight popup is on screen the visible + /// scroll wheels are also moved to the new value; the native picker is read-only while + /// shown so against a native popup this updates only the committed value. /// /// #### Parameters /// /// - `time`: the time value as minutes since midnight e.g. 630 is 10:30am public void setTime(int time) { value = Integer.valueOf(time); + if (currentSpinner != null) { + currentSpinner.setValue(value); + } updateValue(); } @@ -1653,7 +1720,9 @@ public void setDuration(int hour, int minute) { setDuration(hour * 60L * 60 * 1000L + minute * 60L * 1000L); } - /// This value is used for the duration type. + /// This value is used for the duration type. While the lightweight popup is on screen + /// this returns the value visible on the scroll wheels; the native picker still returns + /// the last committed value. /// /// #### Returns /// @@ -1665,10 +1734,12 @@ public void setDuration(int hour, int minute) { /// /// - #getDurationMinutes() public long getDuration() { - return (Long) value; + return (Long) currentValue(); } - /// This value is only used for duration type. + /// This value is only used for duration type. If the lightweight popup is on screen the + /// visible scroll wheels are also moved to the new value; the native picker is read-only + /// while shown so against a native popup this updates only the committed value. /// /// #### Parameters /// @@ -1685,6 +1756,9 @@ public long getDuration() { /// - #getDurationMinutes() public void setDuration(long duration) { value = Long.valueOf(duration); + if (currentSpinner != null) { + currentSpinner.setValue(value); + } updateValue(); } diff --git a/scripts/hellocodenameone/common/src/main/java/com/codenameone/examples/hellocodenameone/tests/LightweightPickerButtonsScreenshotTest.java b/scripts/hellocodenameone/common/src/main/java/com/codenameone/examples/hellocodenameone/tests/LightweightPickerButtonsScreenshotTest.java index 4646fd3a19..18505b5d8c 100644 --- a/scripts/hellocodenameone/common/src/main/java/com/codenameone/examples/hellocodenameone/tests/LightweightPickerButtonsScreenshotTest.java +++ b/scripts/hellocodenameone/common/src/main/java/com/codenameone/examples/hellocodenameone/tests/LightweightPickerButtonsScreenshotTest.java @@ -27,6 +27,16 @@ * captures: it lays out all three alignments in the same row with * explicit L / C / R labels, so a regression that re-broke any of * them would visibly collapse one column toward another. + * + * Doubles as a regression test for issue #4897 (live propagation of + * {@code setDate} to the visible spinner wheels). Each variant opens + * the popup with {@code new Date()} (so the wheels start at whatever + * day the test runs), then calls {@code picker.setDate(fixedDate)} + * after the slide-up to spin them to a known calendar position before + * the screenshot. The committed baselines all show April 11 2026 - + * if the live-propagation regresses the wheels will keep showing the + * runtime "today" instead and the diff will fail every day except by + * coincidence. */ public class LightweightPickerButtonsScreenshotTest extends BaseTest { private Form form; @@ -50,7 +60,11 @@ protected void onShowCompleted() { picker = new Picker(); picker.setType(Display.PICKER_TYPE_DATE); picker.setUseLightweightPopup(true); - picker.setDate(fixedDate); + // Initial value is "today" so the wheels open at a per-run date; each + // variant cycle then spins them to fixedDate via the live-propagation + // path (#4897) before screenshot, which is what makes the baselines + // date-independent. + picker.setDate(new Date()); form.add(picker); form.show(); return true; @@ -123,25 +137,38 @@ private void runVariantsFrom(final int index) { } final Variant variant = variants[index]; picker.clearLightweightPopupButtons(); - picker.setDate(fixedDate); + // Open at "today" so the wheels start somewhere date-dependent. The + // setDate(fixedDate) call below (post-show) then exercises the + // #4897 live-propagation path to drive the wheels to a stable + // calendar position before we capture. + picker.setDate(new Date()); variant.configure.run(); picker.startEditingAsync(); - // Wait for the popup to slide up before screenshotting. Each - // variant cycle (wait + chunk-throttled emit + popup dismiss) - // costs ~5s on Android, and the per-test deadline in - // Cn1ssDeviceRunner is 30s, so this budget can't be padded. - // 600ms is generous: the InteractionDialog transition is - // <300ms and setDate is applied synchronously above. + // First wait: InteractionDialog slide-up. <300ms in practice; 600ms + // is the budget the original test used and stays inside the per-test + // 30s deadline once the second wait below is added. UITimer.timer(600, false, form, new Runnable() { @Override public void run() { - Cn1ssDeviceRunnerHelper.emitCurrentFormScreenshot(variant.imageName, new Runnable() { + // Live-propagate the deterministic fixedDate to the visible + // wheels. Pre-#4897 this had no effect while the popup was + // showing, so the wheels would stay on "today" and the + // screenshot diff would fail any day other than April 11. + picker.setDate(fixedDate); + // Second wait: give the wheels a couple of frames to settle + // at the new month/day/year before snapping the PNG. + UITimer.timer(400, false, form, new Runnable() { @Override public void run() { - picker.stopEditing(new Runnable() { + Cn1ssDeviceRunnerHelper.emitCurrentFormScreenshot(variant.imageName, new Runnable() { @Override public void run() { - runVariantsFrom(index + 1); + picker.stopEditing(new Runnable() { + @Override + public void run() { + runVariantsFrom(index + 1); + } + }); } }); }