Merge lp:~unity-2d-team/unity-2d/filter-option-compact-shell into lp:unity-2d

[Lohith D Shivamurthy]

[shell][dash] Now we have a separate renderer filter-checkoption-compact. Hence the same change is incorparated in unity-2d-shell.

[Michał Sawicz (saviq)]

The tests rely on the fact that Application and Music lens as we know them have 2 and 3 columns, respectively.
That doesn't look like a reliable way to test, can you think of a better way? The easiest would probably be to go through the lenses / filters to find one that's filter-check-option and the other filter-check-option-compact.

Obviously that won't work when no lens uses those, but at some point we might have a fake testing lens that will have all of the different filter types for testing and the tests will still work, whereas we can't be sure people actually have the Applications or the Music lens.

Gerry, what's your take on it?

review: Needs Fixing

[Gerry Boland (gerboland)]

The lenses are something we don't have control over. However in the absense of test lenses, we've got to assume *some* lenses are there. I'm happy assuming Applications, Documents & Music are there, as they're the defaults.

And to do a reliable test, we've got to know a lens with filter-check-option, and one with filter-check-option-compact. Applications & Music (resp.) are suitable. Cycling through them doesn't test something we know, only that we found a lens which claims to have 2 or 3 columns.

So I think these tests are sufficient.

Long term goal is to have a test lens, so we don't need to worry about lens changes.

[Gerry Boland (gerboland)]

Comments on test script:

lines 28, 66, 111: Pet peeve: please use "lens" instead of "lense"
 - http://oxforddictionaries.com/spellcheck/?region=us&q=lense

line 29: 'pwd' not needed

lines 78, 123: Expand the comments like "Could not find AppLensButton" to ask if the relevant lens package is installed. Should help people who have removed it.

lines 82+, and others
+ XDo::Mouse.move(button['x_absolute'].to_i + 1, button['y_absolute'].to_i + 1, 0, true)
+ XDo::Mouse.click(nil, nil, :left)
Use button.tap, it's easier & safer.

line 86,130: no need to reset "button"

line 99:
+ verify( TIMEOUT, 'FilterCheckOption don\'t have two columns' ) {
+ loader.GridViewWithSpacing()['columns'] == '2'
+ }
Please use:
verify_equal( 2, TIMEOUT, 'FilterCheckOption don\'t have two columns' ) {
  loader.GridViewWithSpacing()['columns'].to_i
}
instead, as it returns better error output if it fails.

line 123: wrong comment

And some small English suggestions:
61: Check Filter results list view having renderer as filter-checkoption is displayed with two columns
   -> "Check Filter results list view rendered with filter-checkoption is displayed with two columns"
68: Check that filter-checkoption renderer is having two columns
   -> "Check that the filter-checkoption renderer uses two columns"
and similar.

review: Needs Fixing

[Gerry Boland (gerboland)]

Ok now I've verified that Unity-Core has been updated to support this change, I've tested it and it's working.

Issue found: the Documents lens, the "Last Modified" checkboxes are not compact - but they should be according to the mockups.

This requires a fix in the documents lens. I've logged this bug here:
https://bugs.launchpad.net/unity-lens-files/+bug/928208
I'll hold off merging this until that bug is fixed.

I suggest in the mean time you keep this branch up to date.

[Florian Boucault (fboucault)]

Please resubmit the MR against lp:unity-2d

[lp:~unity-2d-team/unity-2d/filter-option-compact-shell]

937. By Lohith D Shivamurthy on 2012-02-17

nerge lp:unity-2d

938. By Lohith D Shivamurthy on 2012-02-17

merge lp:~dyams/unity-2d/fix-modifed-filter-3-columns

[Gerry Boland (gerboland)]

Question: can your small hack be a little more specific to the Files lens? Right now, any lens filterModel with id "modified" will be changed with this hack.

Small comment change needed:
+ unity-core fixes the issue
It is the files lens which needs fixing, not unity-core.

review: Needs Fixing

[Gerry Boland (gerboland)]

Well I can answer my question: No easy way to make this more specific to Files lens, without changing filters API. This hack will do.

Also, from comments on the bug attached, it appears fixes to unity-core will be needed.
As a result I withdraw my objections and approve. Thanks!

review: Approve

=== modified file 'shell/dash/FilterCheckoption.qml'
--- shell/dash/FilterCheckoption.qml	2012-01-10 16:52:24 +0000
+++ shell/dash/FilterCheckoption.qml	2012-02-17 12:01:32 +0000
@@ -23,11 +23,13 @@
 
     height: filters.height
 
+    property variant grid_columns : 2
+
     GridViewWithSpacing {
         id: filters
 
-        columns: ( filterView.filterModel.id == "genre"
-                  || filterView.filterModel.id == "modified" ) ? 3 : 2
+        columns: grid_columns
+
         rows: Math.ceil(count/columns)
 
         horizontalSpacing: 10

=== added file 'shell/dash/FilterCheckoptionCompact.qml'
--- shell/dash/FilterCheckoptionCompact.qml	1970-01-01 00:00:00 +0000
+++ shell/dash/FilterCheckoptionCompact.qml	2012-02-17 12:01:32 +0000
@@ -0,0 +1,22 @@
+/*
+ * This file is part of unity-2d
+ *
+ * Copyright 2010-2011 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+import QtQuick 1.0
+
+FilterCheckoption {
+    grid_columns : 3
+}

=== modified file 'shell/dash/FilterLoader.qml'
--- shell/dash/FilterLoader.qml	2011-11-30 10:28:01 +0000
+++ shell/dash/FilterLoader.qml	2012-02-17 12:01:32 +0000
@@ -22,6 +22,7 @@
 
 FocusScope {
     id: filterLoader
+    objectName: filterModel.rendererName
 
     property variant lens
     property variant filterModel
@@ -96,6 +97,7 @@
 
     Loader {
         id: filterView
+        objectName: filterModel.rendererName
 
         KeyNavigation.up: header
         anchors.top: header.bottom

=== modified file 'shell/dash/FilterPane.qml'
--- shell/dash/FilterPane.qml	2012-01-10 17:31:32 +0000
+++ shell/dash/FilterPane.qml	2012-02-17 12:01:32 +0000
@@ -32,6 +32,7 @@
 
     AbstractButton {
         id: header
+        objectName: "filterResults"
 
         Accessible.name: title.text
 

=== modified file 'shell/dash/FilterRadiooption.qml'
--- shell/dash/FilterRadiooption.qml	2011-11-09 12:47:37 +0000
+++ shell/dash/FilterRadiooption.qml	2012-02-17 12:01:32 +0000
@@ -19,4 +19,17 @@
 import QtQuick 1.0
 
 FilterCheckoption {
+    /*TODO: This is a workaround for the unity-core bug
+      Files & Folders-> LastModified should have 3 columns not 2
+      https://bugs.launchpad.net/unity-lens-files/+bug/928208
+      We should remove the following block as soon as
+      unity-core fixes the issue
+    */
+    grid_columns : {
+        if (filterModel.id === "modified") {
+            return 3
+        } else {
+            return 2
+        }
+    }
 }

=== modified file 'shell/dash/LensBar.qml'
--- shell/dash/LensBar.qml	2012-02-10 20:25:00 +0000
+++ shell/dash/LensBar.qml	2012-02-17 12:01:32 +0000
@@ -100,6 +100,7 @@
             model: visibleLenses
             delegate: LensButton {
                 Accessible.name: u2d.tr(item.name)
+                objectName: item.name
 
                 /* Heuristic: if iconHint does not contain a '/' then it is an icon name */
                 icon: {

=== added file 'tests/dash/test-renderer-filter-check-option-compact.rb'
--- tests/dash/test-renderer-filter-check-option-compact.rb	1970-01-01 00:00:00 +0000
+++ tests/dash/test-renderer-filter-check-option-compact.rb	2012-02-17 12:01:32 +0000
@@ -0,0 +1,135 @@
+#!/usr/bin/env ruby1.8
+=begin
+/*
+ * This file is part of unity-2d
+ *
+ * Copyright 2011 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+=end
+
+require '../run-tests.rb' unless $INIT_COMPLETED
+require 'xdo/xwindow'
+require 'xdo/keyboard'
+require 'xdo/mouse'
+
+############################# Test Suite #############################
+context "Dash - Filter Results renderer tests" do
+  # Run once at the beginning of this test suite
+  startup do
+    $SUT.execute_shell_command 'killall unity-2d-shell'
+    $SUT.execute_shell_command 'killall unity-2d-shell'
+  end
+
+  # Run once at the end of this test suite
+  shutdown do
+  end
+
+  # Run before each test case begins
+  setup do
+    # Execute the application
+    @app = $SUT.run( :name => UNITY_2D_SHELL,
+                     :arguments => "-testability",
+                     :sleeptime => 2 )
+  end
+
+  # Run after each test case completes
+  teardown do
+    $SUT.execute_shell_command "pkill -nf unity-2d-shell"
+  end
+
+  #####################################################################################
+  # Test cases
+
+  # Test case objectives:
+  #   * Verify renderer filter-check-option is displayed with two columns
+  # Pre-conditions
+  #   * None
+  # Test steps
+  #   * Open Dash
+  #   * Goto Applications Lens
+  #   * Open Filter Results
+  #   * verify that filter-checkoption renderer is having two columns
+  # Post-conditions
+  #   * None
+  # References
+  #   * None
+  test "Verify renderer filter-check-option" do
+    XDo::Keyboard.super
+    verify (TIMEOUT) { @app.Dash() }
+
+    button = ""
+    verify( TIMEOUT, 'Could not find Applications Lens. Did you install it?' ) {
+        button = @app.Dash().LensBar().LensButton( :name => 'Applications' )
+    }
+
+    button.tap
+
+    verify( TIMEOUT, 'Could not find filterResults button' ) {
+        button = @app.Dash().FilterPane().AbstractButton( :name => 'filterResults' )
+    }
+
+    button.tap
+
+    loader = ""
+    verify( TIMEOUT, 'Could not find FilterCheckOption Loader' ) {
+        loader = @app.Dash().FilterPane().FilterLoader(:name => 'filter-checkoption').QDeclarativeLoader( :name => 'filter-checkoption' )
+    }
+
+    verify_equal(2, TIMEOUT, 'FilterCheckOption don\'t have two columns' ) {
+        loader.GridViewWithSpacing()['columns'].to_i
+    }
+
+  end
+
+  # Test case objectives:
+  #   * Verify renderer filter-check-option-compact is displayed with three columns
+  # Pre-conditions
+  #   * None
+  # Test steps
+  #   * Open Dash
+  #   * Goto Music Lens
+  #   * Open Filter-Results
+  #   * verify that filter-checkoption-compact renderer is having three columns
+  # Post-conditions
+  #   * None
+  # References
+  #   * None
+  test "Verify renderer filter-check-option-compact" do
+    XDo::Keyboard.super
+    verify (TIMEOUT) { @app.Dash() }
+
+    button = ""
+    verify( TIMEOUT, 'Could not find Music Lens. Did you install it?' ) {
+        button = @app.Dash().LensBar().LensButton( :name => 'Music' )
+    }
+
+    button.tap
+
+    verify( TIMEOUT, 'Could not find filterResults button' ) {
+        button = @app.Dash().FilterPane().AbstractButton( :name => 'filterResults' )
+    }
+
+    button.tap
+
+    loader = ""
+    verify( TIMEOUT, 'Could not find FilterCheckOptionCompact Loader' ) {
+        loader = @app.Dash().FilterPane().FilterLoader(:name => 'filter-checkoption-compact').QDeclarativeLoader( :name => 'filter-checkoption-compact' )
+    }
+
+    verify_equal(3, TIMEOUT, 'FilterCheckOptionCompact don\'t have three columns' ) {
+        loader.GridViewWithSpacing()['columns'].to_i
+    }
+    end
+end