[haiku-commits] Re: r40306 - haiku/trunk/src/kits/interface
- From: Ryan Leavengood <leavengood@xxxxxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Fri, 28 Jan 2011 18:39:27 -0500
On Fri, Jan 28, 2011 at 3:05 PM, Axel Dörfler <axeld@xxxxxxxxxxxxxxxx> wrote:
> Stephan Aßmus <superstippi@xxxxxx> wrote:
>> That is certainly a fix which "works", however there is some code in
>> BMenu which is supposed to flash the selected menu item [...]
>> All in all, I am not so happy with this solution.
>
> Same thoughts here.
OK.
Warning long text below:
I wonder if we could resolve all these menu issues by first describing
the ideal behavior, taking the existing API into account. I think we
can all agree that in general how menus in BMenuBar works is fine
(excluding some bugs of course, like the keyboard navigation issues.)
So that leaves pop-up menus, both as context menus and as used in
BMenuField.
The main problem is context menus. Here are the two Go() method
signatures on BPopUpMenu (from the Be Book):
BMenuItem* Go(BPoint screenPoint,
bool deliversMessage = false,
bool openAnyway = false,
bool asynchronous = false);
BMenuItem* Go(BPoint screenPoint,
bool deliversMessage,
bool openAnyway,
BRect clickToOpenRect,
bool asynchronous = false);
When an application calls Go() on BPopUpMenu it will provide a BPoint
where the left-top of the menu window should be. Nothing too crazy
there, with the exception that if the menu would show off the screen,
it is moved within screen boundaries, which means the left-top is no
longer at screenPoint, and may in fact be under the mouse cursor. This
is where things can get messy. No one (including Pete Goodeve) wants
menu items accidentially invoked in this case. So the menu should
always stay on screen in that case and not invoke the item under the
cursor when the mouse button is released. How to achieve that is the
issue I guess.
Two of the other parameters to Go(), deliversMessage and asynchronous,
don't affect the menu behavior as far as mouse tracking goes, so they
can be ignored here.
That leaves openAnyway and the clickToOpenRect. If openAnyway is true
we always do sticky mode and leave the menu on the screen, so no
problem there. If it is false then we can turn on sticky mode only if
the mouse is released inside the clickToOpenRect. The moving of the
menu window if opened near a screen edge should not affect the
clickToOpenRect. If openAnyway is false and there is no
clickToOpenRect provided then the menu should never be sticky (except
in the case above of the menu opening under the mouse.) This is the
default behavior inherited from BeOS (for better or worse) if only the
screenPoint is provided.
If my understanding is wrong on the above at all, let me know.
So given all that I think the only issue that remains is not invoking
items if the menu opens under the cursor. That is what I was trying to
do in r40132 (http://dev.haiku-os.org/changeset/40132) but clearly I
did not know enough about the code at that point.
With further thought I think the proper solution is to just turn on
sticky mode if the menu opens under the mouse and the mouse button is
released within the double-click period. So somewhat of a hybrid
between this commit and r40132. Does that sound reasonable?
Another option in addition to the above is a Haiku user preference to
always force sticky mode in pop-up menus. People like Pete can leave
this off but the rest of us can leave it on.
The other question is whether these changes would cause problems with
BMenuField or BMenus in a BMenuBar. I don't think so, but would
obviously need to test to be sure.
As for a general refactoring of the menu code: it is still needed I
think, but we should try to get the behavior right first, and then
have well designed tests (automated if possible) to make sure that
behavior is maintained when refactoring.
Sorry for the long email but this is not a simple issue and I'm kind
of tired of trying different things which someone (whether Pete, you
guys or someone else) finds fault with.
--
Regards,
Ryan
Other related posts:
