SK-Yang / torchat

Automatically exported from code.google.com/p/torchat
0 stars 0 forks source link

First stab at a more normal Mac UI with toolbars. #89

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
This is a first attempt at producing a more Mac-like UI. The About and 
Preferences menu items don't do anything. Clicking on the dock icon brings the 
main window back if it has been closed. Quitting seems to do the right thing. 
Right clicking on the dock icon doesn't bring up the menu and the Hide menu 
item does very strange things.

I couldn't figure out how to get the top active window, wx.GetActiveWindow() 
always seemed to return None, so I had to create a new MenuBar for each frame, 
just to connect the Close menu item to the appropriate window.

patch -p1<patch-torchat-mac.diff

from inside the repository patches cleanly against r481.

Original issue reported on code.google.com by schecko...@gmail.com on 17 Jan 2011 at 7:11

Attachments:

GoogleCodeExporter commented 9 years ago
could you derive a class from wx.MenuBar and put the event listener methods 
into this class, so we would not need a separate App class for Mac?

instead of 

def getMenuBar(win):
    mb = wx.MenuBar()
    ...
    app.Bind(...)

do it like this:

class MenuBar(wx.MenuBar):
    def __init__(self, win):
        wx.MenuBar.__init__(self)
        ...
        self.Bind(...)

and then in getMenu() simple instantiate and return an object of this class?

Original comment by prof7...@gmail.com on 17 Jan 2011 at 10:14

GoogleCodeExporter commented 9 years ago

Original comment by prof7...@gmail.com on 17 Jan 2011 at 10:15

GoogleCodeExporter commented 9 years ago
That could be done for the menu bars, but not for getting the click on the dock 
icon MacReopenApp(). I figured it'd be better to just subclass a single class 
than two. Is there a problem with subclassing wx.App? It's what the various 
guides I saw showed.

Original comment by schecko...@gmail.com on 17 Jan 2011 at 10:21

GoogleCodeExporter commented 9 years ago
you would not need to determine the top window, you could make the event 
methods just use self.win to have the window they belong to.

Original comment by prof7...@gmail.com on 17 Jan 2011 at 10:33

GoogleCodeExporter commented 9 years ago
at least you could move a lot of the menu event methods where they belong: into 
the menu itself.

Original comment by prof7...@gmail.com on 17 Jan 2011 at 10:37

GoogleCodeExporter commented 9 years ago
if it leads to more complicated code then ignore my comments above, I just 
thought it would be more straightforward subclassing wx.MenuBar and have it do 
at least those things on its own that it can handle without the app object. 

There is nothing wrong with subclassing wx.App, if it is easier for some things 
then do it.

Original comment by prof7...@gmail.com on 17 Jan 2011 at 10:44

GoogleCodeExporter commented 9 years ago
my popup menu for example also has all events bound to its own methods, so I 
have the entire code for the menu in one place.

Original comment by prof7...@gmail.com on 17 Jan 2011 at 10:47

GoogleCodeExporter commented 9 years ago
I'll look into it tomorrow.

Subclassing a menubar just seems really bizarre to me. That's not at all how 
Mac programming works. The menu bar holds menus, the menus have menu items, the 
menu items have targets and actions. When you select a menu item, it invokes 
the action on the target. There would never be a reason to subclass the NSMenu 
class, for example.

Original comment by schecko...@gmail.com on 17 Jan 2011 at 10:57

GoogleCodeExporter commented 9 years ago
its not more bizarre than subclassing a Frame or wx.App or any other class. Its 
actually the way is is supposed to be done. For me it is more bizarre to bind 
some arbitrary events to the App class when in reality they come from a MenuBar 
and act on its Frame but have nothing to do with the App, even more so when 
processing the events then needs information (which wx.Frame the menu belonged 
to) which would be easily available in the MenuBar instance that generated them 
itself but not in the App object which has no knowledge about it. 

If you need to attach the same menu bar to different Frames (Frames of 
different classes!) you should definitely make it an object that encapsulates 
all things it needs, that can initialize itself and knows how to behave 
completely on its own.

You currently do this initializing in your getMenuBar() function (you 
initialize a stock wx.MenuBar and then you customize it) but if you think about 
it for a while this is actually the exact same thing that an overridden 
constructor would do in a custom subclass. Actually your getMenuBar() IS 
nothing other than a constructor, it is only written in a procedural syntax.

actually you actually do some sort of subclassing already but you do it in an 
unconventional way: you have a constructor (but you have written it as a 
function), you have methods (but you put them into some other and unrelated 
object and then you have to make these "methods" figure out by calling some API 
each time to which Frame/MenuBar the current call belongs) and you then create 
many instances of it to attach it to at least 3 different Frames. This looks 
like a procedural emulation of an OOP concept. The OOP approach would simply be 
to subclass wx.MenuBar.

Subclassing the menu bar would serve the purpose that each menu bar instance 
would be able to make usage of its knowledge to which wx.Frame it belongs and 
not bother some wx API functions each time. You would get rid of all the 
GetXxxxWindow() calls if you would handle the events directly in the MenuBar.

I don't see any of these methods operating on the App object, they all only try 
to find their Frame and operate on the Frame, so I see no reason why any of 
them should belong to App.

I am guilty of the same misdoings as well, look at the ChatWindow for example, 
its currently a really ugly mess, the text controls, maybe even the 
SplitterWindow itself should be subclassed and all their functionality be 
encapsulated and self-contained. I will do this eventually, because I want to 
make the UI more flexible, allow many of these panels optionally be put into 
some sort of tabbed window and also need an UI for the chat rooms that need the 
same chat panels.

Original comment by prof7...@gmail.com on 18 Jan 2011 at 12:00

GoogleCodeExporter commented 9 years ago
I've written like 5 different replies to that last comment. Suffice it to say, 
we have a different view about this (I wouldn't say I'm customizing the MenuBar 
in any respect; I'm not changing its functionality, I'm adding Menus to it.)

But that all aside, for some reason it doesn't actually work. If I subclass 
wx.MenuBar and do nothing more than move the getMenuBar() code into __init__(), 
then every thing works just fine. However,

        item = fileMenu.Append(wx.ID_EXIT)
        self.Bind(wx.EVT_MENU, self.onExit, item)

and moving the onExit() from App to the subclass of wx.MenuBar causes it to 
never be called. The same is true of the others.

In fact, this is a known limitation of wx.MenuBar: 
http://wxpython-users.1045709.n5.nabble.com/Binding-events-of-wx-MenuBar-td23686
47.html so either each frame object handles this (which is silly because none 
of these menu items have anything to do with the frame except for Close) or the 
application handles it as the last handler.

The right thing to do is to have only a single menu bar. That was what I 
originally wanted, but wx.GetActiveWindow() is not implemented on the Mac for 
some reason. So I wrote my own version of it. This new patch only creates a 
single MenuBar and it relies on the App being the last to be called in the 
event handler chain.

Original comment by schecko...@gmail.com on 18 Jan 2011 at 1:57

Attachments:

GoogleCodeExporter commented 9 years ago
> "The right thing to do is to have only a single menu bar" 

but there are different frames, each frame must be given its own menu bar (of 
course this could all be instances of the same class, maybe even references to 
the identicak instance [not sure abiut this] but to be compatible with other 
platforms where menus belong to windows it is done this way in wx, even if a 
native Cocoa application would work differently)

Maybe its because you are used to the way a native Mac application is 
architected, for me for example it is a completely bizarre concept that a menu 
bar would NOT belong to (into) the window. On other platforms different windows 
of the same application can have different menus and the menu bar is even drawn 
inside the window for this reason. This is the reason why in wx you set the 
menu when initializing the Frame (in each Frame!). So the previous approach 
setting the menu for each frame from each frame's constructor was correct 
already. 

Just out of interest: What happens when you would set different menu bars to 
the different windows of the same application? I would expect it (wild guess) 
to display a different menu on top of the screen depending on which window is 
currently focused. Isn't this concept used by Mac applications too?

I will apply your patch later today (or tomorrow) when I have some more time to 
meditate over it. Its just that something in it somehow does not feel "right" 
to me or like how I would have implemented it or how I (as a non-Mac user) 
would look at the entire problem. I will have a closer look later and also 
compare both versions of the patch with each other.

Original comment by prof7...@gmail.com on 18 Jan 2011 at 2:41

GoogleCodeExporter commented 9 years ago
don't get me wrong, I greatly appreciate your work, its just that the entire 
Mac UI concept is so completely foreign to me and all my concepts of what 
belongs to where originate from how things are done in the windows/kde/gnome 
universe. This is why i perceive a menu bar as a component contained in a 
window on the screen, not only visually but also logically.

Original comment by prof7...@gmail.com on 18 Jan 2011 at 2:56

GoogleCodeExporter commented 9 years ago
Actually now, after looking at it a second time it makes more sense to me, wx 
has built in mac specific methods and it seems there is indeed a way to have a 
menu bar that is not bound to a window.

Also there seem to be even more mac specific apple events that are directly 
passed to the app object http://docs.wxwidgets.org/trunk/classwx_app.html, so 
after thinking about it for a while it now makes more and more sense to me to 
have these things (and even more apple stuff) in the app class. Your second 
patch looks actually quite good. I will apply it.

Original comment by prof7...@gmail.com on 18 Jan 2011 at 3:25

GoogleCodeExporter commented 9 years ago
applied the second version of the patch in r488

Original comment by prof7...@gmail.com on 18 Jan 2011 at 11:15