[haiku-commits] Re: r40306 - haiku/trunk/src/kits/interface

  • From: Stephan Aßmus <superstippi@xxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Fri, 28 Jan 2011 10:12:58 +0100

Am 28.01.2011 06:31, schrieb leavengood@xxxxxxxxx:
Author: leavengood
Date: 2011-01-28 06:31:40 +0100 (Fri, 28 Jan 2011)
New Revision: 40306
Changeset: http://dev.haiku-os.org/changeset/40306
Ticket: http://dev.haiku-os.org/ticket/7165

Modified:
    haiku/trunk/src/kits/interface/PopUpMenu.cpp
Log:
Apply the patch from Pete Goodeve from #7165 and therefore fix that ticket.

Instead of forcing a click to open rect this just restarts menu tracking if the
mouse was clicked. It seems to work great and is cleaner than my solution which
is removed in this commit as well.

This still fixes the problem of accidentially selecting menu items but also
maintains the API.

Good job Pete!


Modified: haiku/trunk/src/kits/interface/PopUpMenu.cpp
===================================================================
--- haiku/trunk/src/kits/interface/PopUpMenu.cpp        2011-01-28 05:09:52 UTC 
(rev 40305)
+++ haiku/trunk/src/kits/interface/PopUpMenu.cpp        2011-01-28 05:31:40 UTC 
(rev 40306)
@@ -317,19 +317,7 @@
  BPopUpMenu::_Go(BPoint where, bool autoInvoke, bool startOpened,
                BRect *_specialRect, bool async)
  {
-       // Force start opened. This is just better behavior.
-       startOpened = true;

-       BRect clickToOpenRect;
-
-       // If no click to open rect was provided make one around the opening
-       // point.
-       if (startOpened&&  _specialRect == NULL) {
-               clickToOpenRect.Set(where.x, where.y, where.x, where.y);
-               clickToOpenRect.InsetBy(-2, -2);
-               _specialRect =&clickToOpenRect;
-       }
-
        if (fTrackThread>= B_OK) {
                // we already have an active menu, wait for it to go away before
                // spawning another
@@ -430,10 +418,19 @@
        // called by BMenu::Track()
        fUseWhere = true;

+       // Determine when mouse-down-up will be taken as a 'press', rather than 
a 'click'
+       bigtime_t clickMaxTime = 0;
+       get_click_speed(&clickMaxTime);
+       clickMaxTime += system_time();
+       
        // Show the menu's window
        Show();
        snooze(50000);
        BMenuItem *result = Track(startOpened, _specialRect);
+
+       // If it was a click, keep the menu open and tracking
+       if (system_time()<= clickMaxTime)    
+               result = Track(true, _specialRect);
        if (result != NULL&&  autoInvoke)
                result->Invoke();

That is certainly a fix which "works", however there is some code in BMenu which is supposed to flash the selected menu item and there may also be some virtual BMenuItem method which is called on the result item and which a client app may override. So this patch does not actually prevent a menu item being technically selected during the double click period, it just stops it from propagating (but not early enough) and then just repeats the process. As a result, two items may be selected now, with the machinery being completed only for the second item. Over time, (IMHO) the menu tracking code has collected such "quick hacks" and it doesn't get cleaner this way. All in all, I am not so happy with this solution.

Best regards,
-Stephan

Other related posts: