Merge lp:~dyams/unity-2d/altf1-toggle-launcher-focus into lp:unity-2d

[Lohith D Shivamurthy]

[launcher] Force the launcher to reposition to the beginning when Alt-F1 is hit

[launcher] Force the launcher to reposition to the beginning when Alt-F1 is hit

[Gerry Boland (gerboland)]

Bug:
1. Have Launcher scrollable, i.e. has more icons than it can display at one time
2. With mouse, reveal launcher and scroll down to the bottom
3. Press Alt+F1

Expected Result:
Launcher repositions listview to top, highlights BFB button

Actual Result:
Launcher does not reposition, but BFB still highlighted.

[Gerry Boland (gerboland)]

Please separate the Super-key fix from this MR and submit separately.

review: Needs Fixing

[Gerry Boland (gerboland)]

I see no need to add /* lp:885304 */ as a note in the source code. The function name makes it obvious what it's doing

Some style corrections: instead of

  if (!main.activeFocus)
      main.focus = true

please use either

  if (!main.activeFocus) main.focus = true

or

  if (!main.activeFocus) {
      main.focus = true
  }

Also please consult the CODING document for the use of braces with if/else blocks in C++, this is wrong:

  }
  else {

Would you have any comment on why Alt+F1 isn't working while you are in the Spread?

review: Needs Fixing

[Gerry Boland (gerboland)]

In the absence of automated testing, please add test-cases to https://wiki.ubuntu.com/Unity2DRegressionTest

[Gerry Boland (gerboland)]

typo in URL above, make it: https://wiki.ubuntu.com/Unity2DRegressionTests

[Lohith D Shivamurthy (dyams)]

> I see no need to add /* lp:885304 */ as a note in the source code. The
> function name makes it obvious what it's doing
>
> Some style corrections: instead of
>
> if (!main.activeFocus)
> main.focus = true
>
> please use either
>
> if (!main.activeFocus) main.focus = true
>
> or
>
> if (!main.activeFocus) {
> main.focus = true
> }
>
>
> Also please consult the CODING document for the use of braces with if/else
> blocks in C++, this is wrong:
>
> }
> else {

Sorry.

I have updated the branch with above changes now.

>
>
> Would you have any comment on why Alt+F1 isn't working while you are in the
> Spread?

Yes, I have. Currently, when Spread is will eat all mouse and keyboard events while it is open.
IMO, We need a separate discussion for this.

[Lohith D Shivamurthy (dyams)]

> >
> > Would you have any comment on why Alt+F1 isn't working while you are in the
> > Spread?
>
> Yes, I have. Currently, when Spread is will eat all mouse and keyboard events
> while it is open.
> IMO, We need a separate discussion for this.

 Yes, I have.
 Currently, Spread will eat all mouse and keyboard events while it is open.
 IMO, We need a separate discussion for this.

[Lohith D Shivamurthy (dyams)]

> typo in URL above, make it: https://wiki.ubuntu.com/Unity2DRegressionTests

Yes, updated.

[Gerry Boland (gerboland)]

Ok, all looks good. Thank you!

review: Approve

[Bob The Builder (bobthebuilder-deactivatedaccount)]

Attempt to merge into lp:unity-2d failed due to conflicts:

text conflict in launcher/Launcher.qml
text conflict in launcher/app/launcherview.cpp

[lp:~dyams/unity-2d/altf1-toggle-launcher-focus]

787. By Lohith D Shivamurthy on 2011-11-25

Resolved merge conflicts

[Lohith D Shivamurthy (dyams)]

> Attempt to merge into lp:unity-2d failed due to conflicts:
>
> text conflict in launcher/Launcher.qml
> text conflict in launcher/app/launcherview.cpp

Merge conflicts resolved

[lp:~dyams/unity-2d/altf1-toggle-launcher-focus]

788. By Lohith D Shivamurthy on 2011-11-25

cleaned the code to remove few lines related to another bug

[Gerry Boland (gerboland)]

Looks good, merging.

review: Approve

=== modified file 'launcher/Launcher.qml'
--- launcher/Launcher.qml	2011-11-23 12:14:44 +0000
+++ launcher/Launcher.qml	2011-11-25 12:35:26 +0000
@@ -37,9 +37,13 @@
         }
     }
 
-    function positionAtBeginning() {
+    function focusBFB() {
+        if (!main.activeFocus) {
+            main.focus = true
+        }
+
+        main.currentIndex = 0
         main.positionViewAtBeginning()
-        hideMenu()
     }
 
     GnomeBackground {
 
=== modified file 'launcher/app/launcherview.cpp'
--- launcher/app/launcherview.cpp	2011-11-23 12:14:44 +0000
+++ launcher/app/launcherview.cpp	2011-11-25 12:35:26 +0000
@@ -72,9 +72,9 @@
     connect(&launcher2dConfiguration(), SIGNAL(superKeyEnableChanged(bool)), SLOT(updateSuperKeyMonitoring()));
     updateSuperKeyMonitoring();
 
-    /* Alt+F1 reveal the launcher and gives the keyboard focus to the Dash Button. */
+    /* Alt+F1 toggle the keyboard focus between laucher and other(previous) application. */
     Hotkey* altF1 = HotkeyMonitor::instance().getHotkeyFor(Qt::Key_F1, Qt::AltModifier);
-    connect(altF1, SIGNAL(pressed()), SLOT(forceActivateWindow()));
+    connect(altF1, SIGNAL(pressed()), SLOT(onAltF1Pressed()));
 
     /* Alt+F2 shows the dash with the commands lens activated. */
     Hotkey* altF2 = HotkeyMonitor::instance().getHotkeyFor(Qt::Key_F2, Qt::AltModifier);
@@ -263,7 +263,6 @@
         }
 
         dashInterface.asyncCall(DASH_DBUS_METHOD_ACTIVATE_HOME);
-        positionAtBeginning();
     }
 }
 
@@ -283,9 +282,15 @@
 }
 
 void
-LauncherView::positionAtBeginning()
+LauncherView::onAltF1Pressed()
 {
     QGraphicsObject* launcher = rootObject();
-    QMetaObject::invokeMethod(launcher, "positionAtBeginning", Qt::AutoConnection);
+
+    if (hasFocus()) {
+        QMetaObject::invokeMethod(launcher, "hideMenu", Qt::AutoConnection);
+        forceDeactivateWindow();
+    } else {
+        forceActivateWindow();
+        QMetaObject::invokeMethod(launcher, "focusBFB", Qt::AutoConnection);
+    }
 }
-
 
=== modified file 'launcher/app/launcherview.h'
--- launcher/app/launcherview.h	2011-11-23 12:14:44 +0000
+++ launcher/app/launcherview.h	2011-11-25 12:35:26 +0000
@@ -60,15 +60,13 @@
     void toggleDash();
     void showCommandsLens();
     void onSuperSPressed();
+    void onAltF1Pressed();
 
 protected:
     void focusInEvent(QFocusEvent* event);
     void focusOutEvent(QFocusEvent* event);
 
 private:
-    void positionAtBeginning();
-
-private:
     bool m_superKeyPressed;
     bool m_superKeyHeld;
     bool m_superPressIgnored;