I'll try to get as many of  these issues resolved today as I can.

On Tue, Jul 14, 2009 at 5:22 AM, Fred Kiefer <[email protected]> wrote:

> 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
>



-- 
Gregory Casamento
Open Logic Corporation, Principal Consultant
## GNUstep Chief Maintainer
yahoo/skype: greg_casamento, aol: gjcasa
(240)274-9630 (Cell), (301)362-9640 (Home)
_______________________________________________
Gnustep-dev mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/gnustep-dev

Reply via email to