search

twitter-archive / twui

A UI framework for Mac based on Core Animation
Other
2.74k stars 268 forks source link

TUIPopover issues #114

Closed avaidyam closed 12 years ago

avaidyam commented 12 years ago

Well first of all, just me being OCD, but I believe this class should be called TUIPopoverViewController. Just because its API and function mirrors the iOS one. Secondly, the real issue:

positioningView.nsWindow.screen.frame

Is incorrect, because then the window can be positioned below the dock, and below the menu bar. Instead, it's more feasible to use

positioningView.nsWindow.screen.visibleFrame

Which prevents these issues.

wingrunr21 commented 12 years ago

If we are trying to mirror UIKit (and be OCD) then it should be TUIPopoverController (no such thing as a PopoverViewController).

Do you mean it can't be positioned below the menu bar? I have a TUIPopover working alongside a statusbar item right now. This class has a number of other little issues, however, that make using it on a dock/status bar item a bit annoying. I've got a fork that I'm fixing a few things in.

avaidyam commented 12 years ago

Oh, alright then, I'll look at your fork. :D Thanks.

joshaber commented 12 years ago

Thanks @galaxas0, I incorporated that change at https://github.com/github/twui/commit/5c1cad198d7f60210be4d6f6852fa1f3c32df1ff.

As for the class name, we're not terribly interested in mirroring UIKit. In this case we're more inspired by NSPopover.

wingrunr21 commented 12 years ago

Note that change will alter how the preferredEdge should be set. ie before for a menubar dropdown you should have specified NSMaxYEdge but with this change you should specify NSMinYEdge.

avaidyam commented 12 years ago

Yes, there are a few side effects that I rendered up patching around. Please do expect to see more new content in my fork - I'll be pulling the best of my changes into it :D