Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(163)

Issue 109050: User-specified frame-method for GTM-managed window sheets

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 10 months ago by viettrungluu
Modified:
1 month ago
CC:
thakis
Base URL:
http://google-toolbox-for-mac.googlecode.com/svn/trunk/AppKit/
Visibility:
Public.

Description

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.

Patch Set 1 #

Total comments: 5

Patch Set 2 : Just send @selector(frame), as per Avi's comments. #

Total comments: 2

Patch Set 3 : Added assertions which check selectors. #

Patch Set 4 : Added entry to release notes. #

Patch Set 5 : Added user-specifiable positioning. #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+256 lines, -20 lines) Patch
M AppKit/GTMWindowSheetController.h View 2 3 4 3 chunks +74 lines, -0 lines 0 comments Download
M AppKit/GTMWindowSheetController.m View 2 3 4 13 chunks +176 lines, -20 lines 1 comment Download
M ReleaseNotes.txt View 4 1 chunk +6 lines, -0 lines 0 comments Download

Messages

Total messages: 15
viettrungluu
Could you please take a look at this? Thanks.
11 years, 10 months ago (2009-08-20 12:44:52 UTC) #1
Avi D
BTW, dmac is the guardian of GTM, so roping him in. http://codereview.appspot.com/109050/diff/1/3 File GTMWindowSheetController.m (right): ...
11 years, 10 months ago (2009-08-20 13:51:50 UTC) #2
viettrungluu
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 ...
11 years, 10 months ago (2009-08-20 13:57:22 UTC) #3
Avi D
On 2009/08/20 13:57:22, viettrungluu wrote: > Unfortunately, I have to, since |frame| > returns an ...
11 years, 10 months ago (2009-08-20 14:07:47 UTC) #4
viettrungluu
(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 ...
11 years, 10 months ago (2009-08-20 14:31:08 UTC) #5
dmaclach
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 ...
11 years, 10 months ago (2009-08-20 16:02:52 UTC) #6
viettrungluu
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: ...
11 years, 10 months ago (2009-08-20 16:31:12 UTC) #7
dmaclach
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 > ...
11 years, 10 months ago (2009-08-20 18:08:00 UTC) #8
dmaclach
Sorry.. but could you also add a note to the release notes about your change?
11 years, 10 months ago (2009-08-20 18:10:54 UTC) #9
viettrungluu
On 2009/08/20 18:10:54, dmaclach wrote: > Sorry.. but could you also add a note to ...
11 years, 10 months ago (2009-08-20 18:18:34 UTC) #10
dmaclach
LGTM Thanks!
11 years, 10 months ago (2009-08-20 18:21:40 UTC) #11
viettrungluu
Could we hold this? I have something to add to it.
11 years, 10 months ago (2009-08-22 02:30:55 UTC) #12
viettrungluu
Could you please re-review? I added the ability to specify a sheet-positioning method (needed to ...
11 years, 10 months ago (2009-08-22 06:22:46 UTC) #13
Avi D
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 ...
11 years, 10 months ago (2009-08-28 17:21:01 UTC) #14
smileygurl18
1 month ago (2021-05-20 14:12:16 UTC) #15
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
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b