Merge lp:~unity-team/unity/unity.fix-741657 into lp:unity

[Mirco Müller]

Skip over separator items in Quicklists, if walking them with keyboard-navigation. Fixes LP: #741657

[Tim Penhey (thumper)]

On Thu, 31 Mar 2011 00:27:20 you wrote:
> Mirco Müller has proposed merging lp:~unity-team/unity/unity.fix-741657
> into lp:unity.
>
> Requested reviews:
> Unity Team (unity-team)
> Related bugs:
> Bug #741657 in unity: "quicklist key-nav needs to ignore separator items"
> https://bugs.launchpad.net/unity/+bug/741657
>
> For more details, see:
> https://code.launchpad.net/~unity-team/unity/unity.fix-741657/+merge/55517
>
> Skip over separator items in Quicklists, if walking them with
> keyboard-navigation. Fixes LP: #741657

How about adding a method like like:

bool menu_item_is_seperator(int index) const
{
  DbusmenuMenuitem* item = GetNthItems(index)->_menuItem;
  const gchar* label = dbusmenu_menuitem_property_get(
      item, DBUSMENU_MENUITEM_PROP_LABEL);
  return label != 0;
}

Then you can use it in the up and down code:

     case NUX_VK_UP:
       if (_current_item_index > 0)
       {
         GetNthItems (_current_item_index)->_prelight = false;
         --_current_item_index;
         while (menu_item_is_seperator(_current_item_index)
         {
           --_current_item_index;
         }
         GetNthItems (_current_item_index)->_prelight = true;
         QueueDraw ();
       }

This makes the code in the case a little easier to understand.

[Mirco Müller (macslow)]

Made code more readable.

[Jason Smith (jassmith)]

Does this handle the case where the last or first item is a separator?

review: Needs Information

[Tim Penhey (thumper)]

On Fri, 01 Apr 2011 08:41:06 you wrote:
> Review: Needs Information
> Does this handle the case where the last or first item is a separator?

Not very well in the original form.

[Mirco Müller (macslow)]

I does... now :)

[Mirco Müller (macslow)]

Doh... need to remerge with trunk.

[Mirco Müller (macslow)]

Ok, everything is in order again.

[Jason Smith (jassmith)]

Looks better now!

review: Approve

=== modified file 'src/QuicklistView.cpp'
--- src/QuicklistView.cpp	2011-03-30 14:58:44 +0000
+++ src/QuicklistView.cpp	2011-04-01 13:48:06 +0000
@@ -123,6 +123,29 @@
 {
 }
 
+bool
+QuicklistView::IsMenuItemSeperator (int index)
+{
+  DbusmenuMenuitem* item   = NULL;
+  const gchar*      label  = NULL;
+  bool              result = false;
+
+  if (index < 0)
+    return false;
+
+  item = GetNthItems (index)->_menuItem;
+  if (!item)
+    return false;
+
+  label = dbusmenu_menuitem_property_get (item, DBUSMENU_MENUITEM_PROP_LABEL);
+  if (!label)
+    result = true;
+  else
+    result = false;
+
+  return result;
+}
+
 void
 QuicklistView::RecvKeyPressed (unsigned int  key_sym,
                                unsigned long key_code,
@@ -133,10 +156,18 @@
     // up (highlight previous menu-item)
     case NUX_VK_UP:
     case NUX_KP_UP:
+      // protect against edge-case of first item being a separator 
+      if (_current_item_index == 1 && IsMenuItemSeperator (0))
+        break;
+
       if (_current_item_index > 0)
       {
         GetNthItems (_current_item_index)->_prelight = false;
         _current_item_index--;
+
+        while (IsMenuItemSeperator (_current_item_index))
+          _current_item_index--;
+
         GetNthItems (_current_item_index)->_prelight = true;
         QueueDraw ();
       }
@@ -145,10 +176,18 @@
     // down (highlight next menu-item)
     case NUX_VK_DOWN:
     case NUX_KP_DOWN:
+      // protect against edge-case of last item being a separator 
+      if (_current_item_index == (GetNumItems () - 1) && IsMenuItemSeperator (GetNumItems ()))
+        break;
+
       if (_current_item_index < GetNumItems () - 1)
       {
         GetNthItems (_current_item_index)->_prelight = false;
         _current_item_index++;
+
+        while (IsMenuItemSeperator (_current_item_index))
+          _current_item_index++;
+
         GetNthItems (_current_item_index)->_prelight = true;
         QueueDraw ();
       }

=== modified file 'src/QuicklistView.h'
--- src/QuicklistView.h	2011-02-27 08:18:45 +0000
+++ src/QuicklistView.h	2011-04-01 13:48:06 +0000
@@ -139,7 +139,9 @@
   
   //! Check the mouse up event sent by an item. Detect the item where the mous is and emit the appropriate signal.
   void CheckAndEmitItemSignal (int x, int y);
-  
+
+  bool IsMenuItemSeperator (int index);
+
   //nux::CairoGraphics*   _cairo_graphics;
   int                   _anchorX;
   int                   _anchorY;