This actually reminds me of when these events were created. I think this point got brought up, which is why they were left in there. They may appear semantically as duplicates, but they are called at different points in the lifecycle of the event, so they do add value.
-T On Tue, Sep 1, 2009 at 8:00 AM, Todd Volkert <[email protected]> wrote: > 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(); >>>>> >>>>> >>>>> >>>>> >>>>> >>>> >> >
