Fix #4979: dim backdrop persists after Dialog from on-top side menu#4983
Merged
Conversation
Tapping a command in the on-top side menu used to translate to a synchronous closeSideMenu() followed immediately by cmd.actionPerformed on the same EDT call (SideMenuBar.CommandWrapper.actionPerformed). When the command shows a modal Dialog -- which is exactly what the default initializr archetype's `hello()` does -- the Dialog's event pump took over the EDT before the side menu's async dispose animation had a chance to advance. The dispose onFinish runnable that calls detachToolbarLayeredPane never fired, so the Toolbar's dim layered- pane backdrop (bgColor=0, bgTransparency around 64) stayed attached to the form. After the user dismissed the Dialog the form was still shaded. This is a sibling of #4912 (fixed in d042971). #4912 handled the "close the side menu by tapping outside" path; #4979 is the "tap a command" path where the close races with the command's Dialog. Fix lands in two pieces: * Toolbar gains public closeSideMenu(Runnable), closeLeftSideMenu( Runnable), and closeRightSideMenu(Runnable) overloads that invoke the supplied callback once the dispose animation has completed and the layered pane has been detached. When no menu is open the callback runs synchronously. closeSideMenu(Runnable) waits for both left and right to finish if both are showing. * SideMenuBar.CommandWrapper.actionPerformed routes the cmd.actionPerformed(evt) through this onFinish in the on-top branch. The non-onTop branch (lines 1812+) was already correct -- it defers via ShowWaiter. Regression test added: ToolbarTest.sideMenuCommandFiresAfterLayered- PaneDetach opens the side menu, captures the dim pane reference, fires the command button via pointer events, and asserts that when the command's listener runs the captured pane is already detached. The dispose animation is disabled via reflection so the test does not depend on wall-clock animation timing (which the test EDT can't tick while blocked on the test body). The existing right-side dispatch test needed the same animation-disable for the same reason -- pulled into a disableSideMenuAnimation helper. All 2501 tests in core-unittests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
SpotBugs rule SIC_INNER_SHOULD_BE_STATIC_ANON on the prior commit's anonymous countdown Runnable inside closeSideMenu(Runnable) -- it captured only locals (the remaining[] array and the onFinish Runnable) and never used the enclosing Toolbar instance, so it was holding the implicit outer-this reference for nothing. Extracts the logic into a private static final CloseSideMenuCountdown class with a plain int field, which is both cleaner and SpotBugs-clean. No behavioural change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Author
Android screenshot updatesCompared 110 screenshots: 109 matched, 1 updated.
Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
|
Compared 110 screenshots: 110 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 108 screenshots: 108 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
PMD rule AssignmentInOperand on the prior commit's `--remaining == 0` predecrement-then-compare in CloseSideMenuCountdown.run. Hoist the decrement to its own statement. No behavioural change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
…ac is happy The Ports/Android Ant build compiles CodenameOne/src under encoding=ASCII (build.xml line 111). My PR-#4983 comment included a literal U+2014 em-dash which the ASCII javac rejected with "unmappable character (0xE2/0x80/0x94)". Swap the em-dash for a double hyphen. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SideMenuBar.CommandWrapper.actionPerformed) used to firecmd.actionPerformed(evt)synchronously right after kicking off the side menu's async dispose animation. If the command opens a modalDialog(exactly what the initializr archetype'shello()does), the Dialog's event pump takes the EDT before the dispose animation has a chance to advance, so thedetachToolbarLayeredPaneonFinish never fires and the dim layered-pane backdrop stays visible after the Dialog is dismissed.closeSideMenu(Runnable),closeLeftSideMenu(Runnable),closeRightSideMenu(Runnable)overloads onToolbarthat invoke the supplied callback once the dispose animation has finished and the layered pane is detached;SideMenuBarnow routes the command-fire through that callback.Reproducer (from the issue)
Default initializr project — Hi form with a side-menu "Hello Command" that calls
Dialog.show("Hello Codename One", "Welcome to Codename One", "OK", null). Open side menu → tap Hello Command → Dialog appears → OK → form stays shaded.Test plan
ToolbarTest.sideMenuCommandFiresAfterLayeredPaneDetachcaptures the dim-pane reference, fires the command via pointer events, and asserts the captured pane is detached before the listener runs.core-unittestssuite passes locally on macOS / JDK 8 (2501 tests, 0 failures).🤖 Generated with Claude Code