Merge lp:~aacid/unity-2d/unity-2d-shell_fix_dash_launcher_interactions into lp:~unity-2d-team/unity-2d/unity-2d-shell

[Albert Astals Cid]

Fix dash and launcher interaction with Super, Alt+f1 and Super, Super, Alt+F1

[Lohith D Shivamurthy (dyams)]

Yes, the issues are addressed properly. Don't we need tests?
One information needed: In Unity-3D, Hitting Alt-F1 While Dash is active, Launcher will gain the focus and dash is not closed. But here the dash is closed. Which one is correct?

review: Needs Information

[Lohith D Shivamurthy (dyams)]

> Yes, the issues are addressed properly. Don't we need tests?
> One information needed: In Unity-3D, Hitting Alt-F1 While Dash is active,
> Launcher will gain the focus and dash is not closed. But here the dash is
> closed. Which one is correct?

Hey I found the tests here
https://code.launchpad.net/~aacid/unity-2d/unity-2d_test_dash_launcher_interactions/+merge/90155

[Albert Astals Cid (aacid)]

Oh, I forgot to add the comment with the tests, sorry about that it was late in my day yesterday.

About "Hitting Alt-F1 While Dash is active" behaviour i don't know, i did mimic what unity-2d did in unity-2d-shell, i guess we need to ask someone from design

[Albert Astals Cid (aacid)]

@Lohith: Seems unity-2d behaviour is the correct one https://bugs.launchpad.net/ayatana-design/+bug/919209

[Lohith D Shivamurthy (dyams)]

ah yes. Then this is correct.

review: Approve

[Michał Sawicz (saviq)]

I'm getting a failed test when running the whole suite:

michal@michal-vm:/media/sf_dev/canonical/desktop/unity-2d/tests/places$ ./places-tests.rb
Running tests on applications contained within /home/michal/dev/canonical/desktop/unity-2d
Loaded suite ./places-tests
Started
...F
Finished in 24.930652 seconds.

  1) Failure:
test_Super__Super_and_Alt_F1_interaction(Dash_Tests)
    [/usr/lib/ruby/vendor_ruby/tdriver/verify/verify.rb:715:in `verify_equal'
     ./places-tests.rb:191:in `test_Super__Super_and_Alt_F1_interaction'
     /media/sf_dev/canonical/desktop/unity-2d/tests/misc/lib/testhelper.rb:114:in `__send__'
     /media/sf_dev/canonical/desktop/unity-2d/tests/misc/lib/testhelper.rb:114:in `run'
     /media/sf_dev/canonical/desktop/unity-2d/tests/misc/lib/testhelper.rb:39:in `run']:
Verification "Launcher should be hiding after toggling the dash" at ./places-tests.rb:191:in `test_Super__Super_and_Alt_F1_interaction' failed:
=> verify_equal( -WIDTH, TIMEOUT, 'Launcher should be hiding after toggling the dash' ) {
         @app.Launcher()['x_absolute'].to_i
       }
The value was not equal to -65. It returned: 0

4 tests, 0 assertions, 1 failures, 0 errors

If I just run the single test, it passes:

michal@michal-vm:/media/sf_dev/canonical/desktop/unity-2d/tests/places$ ./places-tests.rb -n test_Super__Super_and_Alt_F1_interaction
Running tests on applications contained within /home/michal/dev/canonical/desktop/unity-2d
Loaded suite ./places-tests
Started
.
Finished in 5.85724 seconds.

1 tests, 0 assertions, 0 failures, 0 errors

Manual testing shows everything works fine, so unless you find something wrong with the test / implementation, we can go ahead.

Below is the diff containing the adapted tests.

=== zmodyfikowano plik tests/places/places-tests.rb
--- tests/places/places-tests.rb 2012-01-25 12:03:31 +0000
+++ tests/places/places-tests.rb 2012-01-26 16:24:37 +0000
@@ -30,6 +30,7 @@

 ############################# Test Suite #############################
 context "Dash Tests" do
+ WIDTH = 65
   # Run once at the beginning of this test suite
   startup do
     $SUT.execute_shell_command 'killall unity-2d-shell'
@@ -75,7 +76,7 @@
   # References
   # * None
   test "Alt+F2 shows the Dash" do
- verify_equal("false", 0, 'There should not be a Dash declarative view on startup') {
+ verify_equal("false", 2, 'There should not be a Dash declarative view on startup') {
       @app.Dash()['active']
     }
     XDo::Keyboard.alt_F2 #Must use uppercase F to indicate function keys
@@ -97,7 +98,7 @@
   # References
   # * None
   test "Pressing the bfb shows the Dash" do
- verify_equal("false", 0, 'There should not be a Dash declarative view on startup') {
+ verify_equal("false", 2, 'There should not be a Dash declarative view on startup') {
       @app.Dash()['active']
     }
     bfb = @app.LauncherList( :name => 'main' ).LauncherList( :isBfb => true );
@@ -111,4 +112,92 @@
         @app.Dash()['active']
     }
   end
+
+ # Test case objectives:
+ # * Check Super and Alt+F1 interaction
+ # Pre-conditions
+ # * Desktop with no running applications
+ # Test steps
+ # * Verify dash is not showing
+ # * Pr...

review: Needs Information

=== modified file 'shell/app/shelldeclarativeview.cpp'
--- shell/app/shelldeclarativeview.cpp	2012-01-24 15:05:33 +0000
+++ shell/app/shelldeclarativeview.cpp	2012-01-25 17:09:42 +0000
@@ -263,6 +263,7 @@
 {
     if (dashActive()) {
         setDashActive(false);
+        forceDeactivateWindow();
     } else {
         Q_EMIT activateHome();
     }
@@ -281,7 +282,12 @@
         Q_EMIT launcherFocusRequested();
         forceActivateWindow();
     } else {
-        forceDeactivateWindow();
+        if (dashActive()) {
+            setDashActive(false);
+            Q_EMIT launcherFocusRequested();
+        } else {
+            forceDeactivateWindow();
+        }
     }
 }