I actually found a reason why the old SheetStateListener.sheetClosed() provided value (because the change broke my app, and now I have to use a hack to get around it). It was called after the sheet.close() method did its work, whereas WindowStateListener.windowClosed() is called before the work is done. In my case, I was checking to see if the owner's content was blocked, but in windowClosed(), it's still blocked because my listener is called before the sheet re-enabled the owner's content! I can't use a SheetCloseListener either, because I'm not the one who opened the sheet...
-T On Tue, Sep 1, 2009 at 7:30 AM, Greg Brown <[email protected]> wrote: > Interesting. Maybe we could also get rid of the veto methods. I don't like > leaving it as it was, because the events were redundant, and we generally > stay away from redundancy in our APIs. > > > On Aug 31, 2009, at 8:27 PM, Todd Volkert wrote: > > I can understand the motivation for the change, but how come there's still >> SheetStateListener.sheetCloseVetoed(Sheet,Vote), since it seems to not >> provide anything that WindowStateListener.windowCloseVetoed(Window,Vote) >> doesn't already provide. The fact that previewSheetClose() has different >> parameters than previewWindowClose() makes me think that perhaps we should >> leave it as it was - with the full gamut of previewSheetClose(), >> sheetCloseVetoed(), and sheetClosed()... as it stands now, it seems a >> little >> inconsistent. >> >> -T >> >> On Mon, Aug 31, 2009 at 3:33 PM, Greg Brown <[email protected]> wrote: >> >> Hi all, >>> >>> This commit represents an API change, which we are supposed to be trying >>> to >>> avoid for point releases. However, the change is relatively minor and I >>> think it is worth making. If anyone has any questions or concerns, let me >>> know. >>> >>> G >>> >>> On Aug 31, 2009, at 3:23 PM, [email protected] wrote: >>> >>> Author: gbrown >>> >>>> Date: Mon Aug 31 19:23:26 2009 >>>> New Revision: 809701 >>>> >>>> URL: http://svn.apache.org/viewvc?rev=809701&view=rev >>>> Log: >>>> Remove redundant close events from DialogStateListener, >>>> SheetStateListener, and MenuPopupStateListener. These events did not add >>>> any >>>> more information than could already be obtained via windowClosed(). >>>> >>>> Modified: >>>> >>>> >>>> incubator/pivot/trunk/tools/src/org/apache/pivot/tools/net/HTTPClient.java >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/Dialog.java >>>> >>>> >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/DialogStateListener.java >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/MenuPopup.java >>>> >>>> >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/MenuPopupStateListener.java >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/Sheet.java >>>> >>>> >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/SheetStateListener.java >>>> >>>> >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/skin/TablePaneSkin.java >>>> >>>> >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/skin/terra/TerraDialogSkin.java >>>> >>>> >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/skin/terra/TerraMenuPopupSkin.java >>>> >>>> >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/skin/terra/TerraSheetSkin.java >>>> >>>> Modified: >>>> >>>> incubator/pivot/trunk/tools/src/org/apache/pivot/tools/net/HTTPClient.java >>>> URL: >>>> >>>> http://svn.apache.org/viewvc/incubator/pivot/trunk/tools/src/org/apache/pivot/tools/net/HTTPClient.java?rev=809701&r1=809700&r2=809701&view=diff >>>> >>>> >>>> ============================================================================== >>>> --- >>>> >>>> incubator/pivot/trunk/tools/src/org/apache/pivot/tools/net/HTTPClient.java >>>> (original) >>>> +++ >>>> >>>> incubator/pivot/trunk/tools/src/org/apache/pivot/tools/net/HTTPClient.java >>>> Mon Aug 31 19:23:26 2009 >>>> @@ -40,12 +40,14 @@ >>>> import org.apache.pivot.wtk.Mouse; >>>> import org.apache.pivot.wtk.PushButton; >>>> import org.apache.pivot.wtk.Sheet; >>>> +import org.apache.pivot.wtk.SheetCloseListener; >>>> import org.apache.pivot.wtk.SheetStateListener; >>>> import org.apache.pivot.wtk.TableView; >>>> import org.apache.pivot.wtk.TaskAdapter; >>>> import org.apache.pivot.wtk.TextArea; >>>> import org.apache.pivot.wtk.TextInput; >>>> import org.apache.pivot.wtk.Window; >>>> +import org.apache.pivot.wtk.WindowStateListener; >>>> import org.apache.pivot.wtk.content.ListItem; >>>> import org.apache.pivot.wtkx.WTKXSerializer; >>>> >>>> @@ -190,15 +192,14 @@ >>>> passwordTextInput.setText(credentials.getPassword()); >>>> } >>>> >>>> - sheet.getSheetStateListeners().add(new >>>> SheetStateListener() { >>>> - public Vote previewSheetClose(Sheet sheet, boolean >>>> result) { >>>> - return Vote.APPROVE; >>>> - } >>>> + sheet.getWindowStateListeners().add(new >>>> WindowStateListener.Adapter() { >>>> + public void windowClosed(Window window, Display >>>> display) { >>>> >>>> - public void sheetCloseVetoed(Sheet sheet, Vote >>>> reaso) >>>> { >>>> - // No-op >>>> } >>>> + }); >>>> >>>> + sheet.open(window, new SheetCloseListener() { >>>> + @Override >>>> public void sheetClosed(Sheet sheet) { >>>> if (sheet.getResult()) { >>>> TextInput usernameTextInput = (TextInput) >>>> @@ -217,8 +218,6 @@ >>>> } >>>> } >>>> }); >>>> - >>>> - sheet.open(window); >>>> } >>>> }); >>>> >>>> @@ -300,7 +299,10 @@ >>>> public void sheetCloseVetoed(Sheet sheet, Vote reaso) { >>>> // No-op >>>> } >>>> + }); >>>> >>>> + sheet.open(window, new SheetCloseListener() { >>>> + @Override >>>> public void sheetClosed(Sheet sheet) { >>>> if (sheet.getResult()) { >>>> System.setProperty("javax.net.ssl.trustStore", >>>> keystorePath); >>>> @@ -308,8 +310,6 @@ >>>> } >>>> } >>>> }); >>>> - >>>> - sheet.open(window); >>>> } >>>> }); >>>> >>>> >>>> Modified: incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/Dialog.java >>>> URL: >>>> >>>> http://svn.apache.org/viewvc/incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/Dialog.java?rev=809701&r1=809700&r2=809701&view=diff >>>> >>>> >>>> ============================================================================== >>>> --- incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/Dialog.java >>>> (original) >>>> +++ incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/Dialog.java Mon >>>> Aug >>>> 31 19:23:26 2009 >>>> @@ -41,12 +41,6 @@ >>>> listener.dialogCloseVetoed(dialog, reason); >>>> } >>>> } >>>> - >>>> - public void dialogClosed(Dialog dialog) { >>>> - for (DialogStateListener listener : this) { >>>> - listener.dialogClosed(dialog); >>>> - } >>>> - } >>>> } >>>> >>>> private boolean modal = false; >>>> @@ -217,8 +211,6 @@ >>>> dialogCloseListener.dialogClosed(this); >>>> dialogCloseListener = null; >>>> } >>>> - >>>> - dialogStateListeners.dialogClosed(this); >>>> } >>>> } else { >>>> dialogStateListeners.dialogCloseVetoed(this, vote); >>>> >>>> Modified: >>>> >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/DialogStateListener.java >>>> URL: >>>> >>>> http://svn.apache.org/viewvc/incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/DialogStateListener.java?rev=809701&r1=809700&r2=809701&view=diff >>>> >>>> >>>> ============================================================================== >>>> --- >>>> >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/DialogStateListener.java >>>> (original) >>>> +++ >>>> >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/DialogStateListener.java >>>> Mon Aug 31 19:23:26 2009 >>>> @@ -21,20 +21,19 @@ >>>> /** >>>> * Dialog state listener interface. >>>> */ >>>> -public interface DialogStateListener extends DialogCloseListener { >>>> +public interface DialogStateListener { >>>> /** >>>> * Dialog state listener adapter. >>>> */ >>>> public static class Adapter implements DialogStateListener { >>>> + @Override >>>> public Vote previewDialogClose(Dialog dialog, boolean result) { >>>> return Vote.APPROVE; >>>> } >>>> >>>> + @Override >>>> public void dialogCloseVetoed(Dialog dialog, Vote reason) { >>>> } >>>> - >>>> - public void dialogClosed(Dialog dialog) { >>>> - } >>>> } >>>> >>>> /** >>>> >>>> Modified: >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/MenuPopup.java >>>> URL: >>>> >>>> http://svn.apache.org/viewvc/incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/MenuPopup.java?rev=809701&r1=809700&r2=809701&view=diff >>>> >>>> >>>> ============================================================================== >>>> --- incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/MenuPopup.java >>>> (original) >>>> +++ incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/MenuPopup.java >>>> Mon >>>> Aug 31 19:23:26 2009 >>>> @@ -49,12 +49,6 @@ >>>> listener.menuPopupCloseVetoed(menuPopup, reason); >>>> } >>>> } >>>> - >>>> - public void menuPopupClosed(MenuPopup menuPopup) { >>>> - for (MenuPopupStateListener listener : this) { >>>> - listener.menuPopupClosed(menuPopup); >>>> - } >>>> - } >>>> } >>>> >>>> private Menu menu; >>>> @@ -136,10 +130,6 @@ >>>> >>>> if (vote.isApproved()) { >>>> super.close(); >>>> - >>>> - if (isClosed()) { >>>> - menuPopupStateListeners.menuPopupClosed(this); >>>> - } >>>> } else { >>>> menuPopupStateListeners.menuPopupCloseVetoed(this, vote); >>>> } >>>> >>>> Modified: >>>> >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/MenuPopupStateListener.java >>>> URL: >>>> >>>> http://svn.apache.org/viewvc/incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/MenuPopupStateListener.java?rev=809701&r1=809700&r2=809701&view=diff >>>> >>>> >>>> ============================================================================== >>>> --- >>>> >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/MenuPopupStateListener.java >>>> (original) >>>> +++ >>>> >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/MenuPopupStateListener.java >>>> Mon Aug 31 19:23:26 2009 >>>> @@ -32,9 +32,6 @@ >>>> >>>> public void menuPopupCloseVetoed(MenuPopup menuPopup, Vote reason) >>>> { >>>> } >>>> - >>>> - public void menuPopupClosed(MenuPopup menuPopup) { >>>> - } >>>> } >>>> >>>> /** >>>> @@ -52,11 +49,4 @@ >>>> * @param reason >>>> */ >>>> public void menuPopupCloseVetoed(MenuPopup menuPopup, Vote reason); >>>> - >>>> - /** >>>> - * Called when a menu popup has closed. >>>> - * >>>> - * @param menuPopup >>>> - */ >>>> - public void menuPopupClosed(MenuPopup menuPopup); >>>> } >>>> >>>> Modified: incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/Sheet.java >>>> URL: >>>> >>>> http://svn.apache.org/viewvc/incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/Sheet.java?rev=809701&r1=809700&r2=809701&view=diff >>>> >>>> >>>> ============================================================================== >>>> --- incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/Sheet.java >>>> (original) >>>> +++ incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/Sheet.java Mon >>>> Aug >>>> 31 19:23:26 2009 >>>> @@ -41,12 +41,6 @@ >>>> listener.sheetCloseVetoed(sheet, reason); >>>> } >>>> } >>>> - >>>> - public void sheetClosed(Sheet sheet) { >>>> - for (SheetStateListener listener : this) { >>>> - listener.sheetClosed(sheet); >>>> - } >>>> - } >>>> } >>>> >>>> private boolean result = false; >>>> @@ -168,8 +162,6 @@ >>>> sheetCloseListener.sheetClosed(this); >>>> sheetCloseListener = null; >>>> } >>>> - >>>> - sheetStateListeners.sheetClosed(this); >>>> } >>>> } else { >>>> sheetStateListeners.sheetCloseVetoed(this, vote); >>>> >>>> Modified: >>>> >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/SheetStateListener.java >>>> URL: >>>> >>>> http://svn.apache.org/viewvc/incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/SheetStateListener.java?rev=809701&r1=809700&r2=809701&view=diff >>>> >>>> >>>> ============================================================================== >>>> --- >>>> >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/SheetStateListener.java >>>> (original) >>>> +++ >>>> >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/SheetStateListener.java >>>> Mon Aug 31 19:23:26 2009 >>>> @@ -21,20 +21,19 @@ >>>> /** >>>> * Sheet state listener interface. >>>> */ >>>> -public interface SheetStateListener extends SheetCloseListener { >>>> +public interface SheetStateListener { >>>> /** >>>> * Sheet state listener adapter. >>>> */ >>>> public static class Adapter implements SheetStateListener { >>>> + @Override >>>> public Vote previewSheetClose(Sheet sheet, boolean result) { >>>> return Vote.APPROVE; >>>> } >>>> >>>> + @Override >>>> public void sheetCloseVetoed(Sheet sheet, Vote reason) { >>>> } >>>> - >>>> - public void sheetClosed(Sheet sheet) { >>>> - } >>>> } >>>> >>>> /** >>>> >>>> Modified: >>>> >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/skin/TablePaneSkin.java >>>> URL: >>>> >>>> http://svn.apache.org/viewvc/incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/skin/TablePaneSkin.java?rev=809701&r1=809700&r2=809701&view=diff >>>> >>>> >>>> ============================================================================== >>>> --- >>>> >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/skin/TablePaneSkin.java >>>> (original) >>>> +++ >>>> >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/skin/TablePaneSkin.java >>>> Mon Aug 31 19:23:26 2009 >>>> @@ -1215,9 +1215,7 @@ >>>> } >>>> >>>> TablePane tablePane = (TablePane)getComponent(); >>>> - >>>> TablePane.RowSequence rows = tablePane.getRows(); >>>> - TablePane.ColumnSequence columns = tablePane.getColumns(); >>>> >>>> int rowCount = tablePane.getRows().getLength(); >>>> int columnCount = tablePane.getColumns().getLength(); >>>> >>>> Modified: >>>> >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/skin/terra/TerraDialogSkin.java >>>> URL: >>>> >>>> http://svn.apache.org/viewvc/incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/skin/terra/TerraDialogSkin.java?rev=809701&r1=809700&r2=809701&view=diff >>>> >>>> >>>> ============================================================================== >>>> --- >>>> >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/skin/terra/TerraDialogSkin.java >>>> (original) >>>> +++ >>>> >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/skin/terra/TerraDialogSkin.java >>>> Mon Aug 31 19:23:26 2009 >>>> @@ -103,8 +103,4 @@ >>>> public void dialogCloseVetoed(Dialog dialog, Vote reason) { >>>> // No-op >>>> } >>>> - >>>> - public void dialogClosed(Dialog dialog) { >>>> - // No-op >>>> - } >>>> } >>>> >>>> Modified: >>>> >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/skin/terra/TerraMenuPopupSkin.java >>>> URL: >>>> >>>> http://svn.apache.org/viewvc/incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/skin/terra/TerraMenuPopupSkin.java?rev=809701&r1=809700&r2=809701&view=diff >>>> >>>> >>>> ============================================================================== >>>> --- >>>> >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/skin/terra/TerraMenuPopupSkin.java >>>> (original) >>>> +++ >>>> >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/skin/terra/TerraMenuPopupSkin.java >>>> Mon Aug 31 19:23:26 2009 >>>> @@ -92,7 +92,7 @@ >>>> } >>>> }; >>>> >>>> - private static final int CLOSE_TRANSITION_DURATION = 250; >>>> + private static final int CLOSE_TRANSITION_DURATION = 200; >>>> private static final int CLOSE_TRANSITION_RATE = 30; >>>> >>>> public TerraMenuPopupSkin() { >>>> @@ -198,6 +198,9 @@ >>>> super.windowClosed(window, display); >>>> >>>> display.getContainerMouseListeners().remove(displayMouseListener); >>>> + >>>> + window.setEnabled(true); >>>> + closeTransition = null; >>>> } >>>> >>>> @Override >>>> @@ -218,6 +221,8 @@ >>>> public Vote previewMenuPopupClose(final MenuPopup menuPopup, boolean >>>> immediate) { >>>> if (!immediate >>>> && closeTransition == null) { >>>> + menuPopup.setEnabled(false); >>>> + >>>> closeTransition = new FadeWindowTransition(menuPopup, >>>> CLOSE_TRANSITION_DURATION, CLOSE_TRANSITION_RATE, >>>> dropShadowDecorator); >>>> @@ -238,12 +243,9 @@ >>>> if (reason == Vote.DENY >>>> && closeTransition != null) { >>>> closeTransition.stop(); >>>> + >>>> + menuPopup.setEnabled(true); >>>> closeTransition = null; >>>> } >>>> } >>>> - >>>> - @Override >>>> - public void menuPopupClosed(MenuPopup menuPopup) { >>>> - closeTransition = null; >>>> - } >>>> } >>>> >>>> Modified: >>>> >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/skin/terra/TerraSheetSkin.java >>>> URL: >>>> >>>> http://svn.apache.org/viewvc/incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/skin/terra/TerraSheetSkin.java?rev=809701&r1=809700&r2=809701&view=diff >>>> >>>> >>>> ============================================================================== >>>> --- >>>> >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/skin/terra/TerraSheetSkin.java >>>> (original) >>>> +++ >>>> >>>> incubator/pivot/trunk/wtk/src/org/apache/pivot/wtk/skin/terra/TerraSheetSkin.java >>>> Mon Aug 31 19:23:26 2009 >>>> @@ -24,6 +24,7 @@ >>>> import org.apache.pivot.wtk.Component; >>>> import org.apache.pivot.wtk.ComponentListener; >>>> import org.apache.pivot.wtk.Dimensions; >>>> +import org.apache.pivot.wtk.Display; >>>> import org.apache.pivot.wtk.GraphicsUtilities; >>>> import org.apache.pivot.wtk.Insets; >>>> import org.apache.pivot.wtk.Keyboard; >>>> @@ -360,6 +361,17 @@ >>>> }); >>>> } >>>> >>>> + @Override >>>> + public void windowClosed(Window window, Display display) { >>>> + super.windowClosed(window, display); >>>> + >>>> + Window owner = window.getOwner(); >>>> + owner.getComponentListeners().remove(ownerComponentListener); >>>> + >>>> + Component ownerContent = owner.getContent(); >>>> + >>>> >>>> ownerContent.getComponentListeners().remove(ownerContentComponentListener); >>>> + } >>>> + >>>> public Vote previewSheetClose(final Sheet sheet, final boolean result) >>>> { >>>> // Start a close transition, return false, and close the window >>>> // when the transition is complete >>>> @@ -403,14 +415,6 @@ >>>> } >>>> } >>>> >>>> - public void sheetClosed(Sheet sheet) { >>>> - Window owner = sheet.getOwner(); >>>> - owner.getComponentListeners().remove(ownerComponentListener); >>>> - >>>> - Component ownerContent = owner.getContent(); >>>> - >>>> >>>> ownerContent.getComponentListeners().remove(ownerContentComponentListener); >>>> - } >>>> - >>>> public void alignToOwnerContent() { >>>> Sheet sheet = (Sheet)getComponent(); >>>> >>>> >>>> >>>> >>>> >>> >
