Gregory Casamento schrieb: > Author: gcasa > Date: Mon Jul 13 20:12:52 2009 > New Revision: 28392 > > URL: http://svn.gna.org/viewcvs/gnustep?rev=28392&view=rev > Log: > * Source/NSAlert.m: Implementation of GSAlertSheet. > * Source/NSApplication.m: Change order in which setWindowParent: > and runModalForWindow: are called in beginSheet:... method. > * Source/NSDrawer.m: Remove notifications when drawer is closed > in dealloc. > > Modified: > libs/gui/trunk/ChangeLog > libs/gui/trunk/Source/NSAlert.m > libs/gui/trunk/Source/NSApplication.m > libs/gui/trunk/Source/NSDrawer.m
I have quite a few issues with this change and I would like these removed before we ship a new release of gui. So let's get discussing them. - GSAlertSheet has an instance variable _parentWindow this duplicates the ivar _parent found in NSWindow. Why do we need this twice? The only difference I see is that it gets retained here. Is this needed and if so, should we do it in NSWindow? It might well be that soem of the code from here rather belongs into NSWindow. - frameFromParentWindowFrame uses the _parentWindow ivar without checking whether it is set. This will horribly fail on Sparc machines. Currently this is save as the method will only be called when this variable is set, but in the long run this implementation is asking for trouble. - GSAlertSheet has an ivar _container that never gets used. What is it for? - The init... method on GSAlertSheet creates an NSBox object overlapping its normal content views. This may lead to drawing problems. The NSBox also never gets released and the size computation should use the proper methods from GSTheme. The rect variable here never gets used. - frameFromParentWindowFrame, why does the frame of the content view of the parent window come into play here? - NSReleaseAlertPanel() should reset the parent window to nil. - With all that said, I don't think that we need a separate GSAlertSheet class, GSAlertPanel should do. Which woudl remove a lot of code duplication. - All the NSBeginAlertSheet() functions duplicate the sending of the didEndSelector to the delegate found in NSApplication beginSheet:..., we should only send this once. Apart from all that, it is great to see some progress on the sheet code, although the main part, event handling in NSApplication is still missing. Cheers, Fred _______________________________________________ Gnustep-dev mailing list [email protected] http://lists.gnu.org/mailman/listinfo/gnustep-dev
