[vlc-devel] [PATCH] Mac OS X: access_screen: support Lion, add Display ID options

[Felix Paul Kühne]

Hello,

On 27.02.2012, at 15:28, Jean-Baptiste Kempf wrote:

> On Mon, Feb 27, 2012 at 10:22:28PM +0900, FUJISAWA Tooru wrote :
>> Please download fixed patch from here: (fixed indentation and framework loading)
>> http://homepage.mac.com/arai_a/fixed2/0001-Mac-OS-X-access_screen-support-Lion-add-Display-ID-o.patch
> 
> The code seems fine now.
I mostly agree on that (see below).


> Felix: xib review?
1) why do you add 2 empty nsmenuitem view's to the xib without any connections to a menu or another super view?
2) I don't think that the stepper next to the Display ID is a proper idea. Since the display ID tends to be a pseudo-random integer with more than 10 digits, chances are really low that a user selects a correct value by using the stepper.
2.1) I'd implement the Display ID selection the other way around, similar to the simple preferences window. There, we display a list within a popup button with "Screen n (%i * %i)" (width, height), so the user actually see which display s/he wants to select. Internally, you map the selected screen to the gained Display IDs and forward that value to the access module.

Best regards,

Felix


--
VideoLAN
Felix Paul Kühne
Developer of VLC's Mac OS X port
www.videolan.org