User-specified frame-method for GTM-managed window sheets

Allow GTM-managed sheets to call something other than |-[NSView frame]| to determine the region over which sheets are modal (allowing the "user" to specify a selector for "frame"). Useful when the logical tab area is not the same as the area switched by the view. Fully backwards compatible. A little hacky, but the most elegant hack that I could think of.

[viettrungluu, 2009-08-20 12:44:52]

Could you please take a look at this? Thanks.

[Avi D, 2009-08-20 13:51:50]

BTW, dmac is the guardian of GTM, so roping him in.

http://codereview.appspot.com/109050/diff/1/3
File GTMWindowSheetController.m (right):

http://codereview.appspot.com/109050/diff/1/3#newcode169
Line 169: frameSelector:nil
Why not just pass in @selector(frame) here? Then you can significantly simplify
things below...

http://codereview.appspot.com/109050/diff/1/3#newcode540
Line 540: [invocation getReturnValue:&viewFrame];
Ow, ow. Use NSInvocation only when you have to. Here, something like -[NSObject
performSelector:] is a lot easier.

[viettrungluu, 2009-08-20 13:57:22]

http://codereview.appspot.com/109050/diff/1/3
File GTMWindowSheetController.m (right):

http://codereview.appspot.com/109050/diff/1/3#newcode169
Line 169: frameSelector:nil
Okay, will do.

http://codereview.appspot.com/109050/diff/1/3#newcode540
Line 540: [invocation getReturnValue:&viewFrame];
On 2009/08/20 13:51:50, Avi D wrote:
> Ow, ow. Use NSInvocation only when you have to. Here, something like
-[NSObject
> performSelector:] is a lot easier.

It hurts me as much as it hurts you. Unfortunately, I have to, since |frame|
returns an |NSRect| (which is decidedly not an |id|, nor compatible with one).

[Avi D, 2009-08-20 14:07:47]

On 2009/08/20 13:57:22, viettrungluu wrote:
> Unfortunately, I have to, since |frame|
> returns an |NSRect|

D'oh. Yeah, NSInvocation is right, then.

[viettrungluu, 2009-08-20 14:31:08]

(Argh. Must remember where I ran upload.py from next time.)

http://codereview.appspot.com/109050/diff/1/3
File GTMWindowSheetController.m (right):

http://codereview.appspot.com/109050/diff/1/3#newcode169
Line 169: frameSelector:nil
On 2009/08/20 13:57:23, viettrungluu wrote:
> Okay, will do.

Done.

[dmaclach, 2009-08-20 16:02:52]

http://codereview.appspot.com/109050/diff/6/8
File AppKit/GTMWindowSheetController.m (right):

http://codereview.appspot.com/109050/diff/6/8#newcode320
Line 320: sheetInfo->frameSelector_ = frameSelector;
please verify that frameSelector and didEndSelector are implemented on their
targets by adding a call to
GTMAssertSelectorNilOrImplementedWithReturnTypeAndArguments for each one. You
can find this in GTMDebugSelectorValidation.h

BTW Avi, the direct accesses to member vars here kills me :(

[viettrungluu, 2009-08-20 16:31:12]

http://codereview.appspot.com/109050/diff/6/8
File AppKit/GTMWindowSheetController.m (right):

http://codereview.appspot.com/109050/diff/6/8#newcode320
Line 320: sheetInfo->frameSelector_ = frameSelector;
On 2009/08/20 16:02:53, dmaclach wrote:
> please verify that frameSelector and didEndSelector are implemented on their
> targets by adding a call to
> GTMAssertSelectorNilOrImplementedWithReturnTypeAndArguments for each one. You
> can find this in GTMDebugSelectorValidation.h
> 
> BTW Avi, the direct accesses to member vars here kills me :(

Done. (Also got rid of now-redundant checks which use -respondsToSelector.)

[dmaclach, 2009-08-20 18:08:00]

On 2009/08/20 16:31:12, viettrungluu wrote:
> http://codereview.appspot.com/109050/diff/6/8
> File AppKit/GTMWindowSheetController.m (right):
> 
> http://codereview.appspot.com/109050/diff/6/8#newcode320
> Line 320: sheetInfo->frameSelector_ = frameSelector;
> On 2009/08/20 16:02:53, dmaclach wrote:
> > please verify that frameSelector and didEndSelector are implemented on their
> > targets by adding a call to
> > GTMAssertSelectorNilOrImplementedWithReturnTypeAndArguments for each one.
You
> > can find this in GTMDebugSelectorValidation.h
> > 
> > BTW Avi, the direct accesses to member vars here kills me :(
> 
> Done. (Also got rid of now-redundant checks which use -respondsToSelector.)

LGTM

[dmaclach, 2009-08-20 18:10:54]

Sorry.. but could you also add a note to the release notes about your change?

[viettrungluu, 2009-08-20 18:18:34]

On 2009/08/20 18:10:54, dmaclach wrote:
> Sorry.. but could you also add a note to the release notes about your change?

Done. Thanks all for the reviews.

[dmaclach, 2009-08-20 18:21:40]

LGTM

Thanks!

[viettrungluu, 2009-08-22 02:30:55]

Could we hold this? I have something to add to it.

[viettrungluu, 2009-08-22 06:22:46]

Could you please re-review? I added the ability to specify a sheet-positioning
method (needed to hang things under toolbars in a tab). Since the user may often
want to opt for the default positioning, I allow nil to be specified for the
selector. It'd be silly to allow nil for that (the positionSelector), but not
nil for frameSelector, so that's back as well. Unfortunately, the CL has grown
quite a bit. Thanks in advance.

[Avi D, 2009-08-28 17:21:01]

Wow. LG.

http://codereview.appspot.com/109050/diff/1016/1018
File AppKit/GTMWindowSheetController.m (right):

http://codereview.appspot.com/109050/diff/1016/1018#newcode21
Line 21: #import "DebugUtils/GTMDebugSelectorValidation.h"
Oh! That's something I didn't see before. Nice.

[smileygurl18, 2021-05-20 14:12:16]

On 2009/08/28 17:21:01, Avi D wrote:
> Wow. LG.
> 
> http://codereview.appspot.com/109050/diff/1016/1018
> File AppKit/GTMWindowSheetController.m (right):
> 
> http://codereview.appspot.com/109050/diff/1016/1018#newcode21
> Line 21: #import "DebugUtils/GTMDebugSelectorValidation.h"
> Oh! That's something I didn't see before. Nice.

hi

[jace838, 2021-06-25 15:58:18]

gl@glong.net,gl041188@gmail.com,2200632@qq.com,guanlong huang