Merge lp:~unity-2d-team/unity-2d/filter-option-compact-shell into lp:~unity-2d-team/unity-2d/unity-2d-shell
| Status: | Superseded |
|---|---|
| Proposed branch: | lp:~unity-2d-team/unity-2d/filter-option-compact-shell |
| Merge into: | lp:~unity-2d-team/unity-2d/unity-2d-shell |
| Diff against target: |
1375 lines (+564/-220) 29 files modified
data/com.canonical.Unity2d.gschema.xml (+1/-1) debian/control (+1/-1) libunity-2d-private/src/lens.cpp (+15/-3) libunity-2d-private/src/lenses.cpp (+8/-5) libunity-2d-private/src/lenses.h (+2/-0) shell/Shell.qml (+1/-1) shell/app/shelldeclarativeview.cpp (+7/-0) shell/app/shelldeclarativeview.h (+2/-0) shell/common/Background.qml (+111/-0) shell/common/IconTile.qml (+97/-0) shell/common/SearchEntry.qml (+25/-33) shell/dash/Dash.qml (+41/-64) shell/dash/FilterCheckoption.qml (+4/-2) shell/dash/FilterCheckoptionCompact.qml (+22/-0) shell/dash/FilterLoader.qml (+2/-0) shell/dash/FilterPane.qml (+1/-0) shell/dash/Home.qml (+1/-1) shell/dash/LensBar.qml (+21/-19) shell/dash/LensView.qml (+7/-6) shell/dash/RendererGrid.qml (+6/-14) shell/launcher/LauncherItem.qml (+17/-67) shell/launcher/LauncherList.qml (+1/-1) tests/dash/dash-tests.rb (+3/-1) tests/dash/test-renderer-filter-check-option-compact.rb (+135/-0) tests/launcher/autohide_show_tests.rb (+7/-0) tests/launcher/autohide_show_tests_rtl.rb (+7/-0) tests/shell/input_shaping.rb (+7/-0) tests/shell/input_shaping_rtl.rb (+7/-0) tests/spread/spread-tests.rb (+5/-1) |
| To merge this branch: | bzr merge lp:~unity-2d-team/unity-2d/filter-option-compact-shell |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Gerry Boland | 2012-01-24 | Needs Fixing on 2012-01-24 | |
| Michał Sawicz | 2012-01-24 | Needs Fixing on 2012-01-24 | |
|
Review via email:
|
|||
This proposal has been superseded by a proposal from 2012-02-16.
Description of the Change
[shell][dash] Now we have a separate renderer filter-
| Gerry Boland (gerboland) wrote : | # |
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-
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) wrote : | # |
Comments on test script:
lines 28, 66, 111: Pet peeve: please use "lens" instead of "lense"
- http://
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.
+ XDo::Mouse.
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.
+ }
Please use:
verify_equal( 2, TIMEOUT, 'FilterCheckOption don\'t have two columns' ) {
loader.
}
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.
- 932. By Lohith D Shivamurthy on 2012-01-25
-
[shell][dash] Review comments fixed
- 933. By Lohith D Shivamurthy on 2012-01-26
-
[places][tests] replace sysytem with .execute_
shell_command - 934. By Lohith D Shivamurthy on 2012-01-26
-
[places][tests] Use the global and remove the local @sut
- 935. By Lohith D Shivamurthy on 2012-01-26
-
[places] replace 'system' i teardown too
| Gerry Boland (gerboland) wrote : | # |
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:/
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) wrote : | # |
Please resubmit the MR against lp:unity-2d
- 936. By Lohith D Shivamurthy on 2012-02-16
-
merge
- 937. By Lohith D Shivamurthy on 2012-02-17
-
nerge lp:unity-2d
- 938. By Lohith D Shivamurthy on 2012-02-17
Unmerged revisions
- 938. By Lohith D Shivamurthy on 2012-02-17
- 937. By Lohith D Shivamurthy on 2012-02-17
-
nerge lp:unity-2d
- 936. By Lohith D Shivamurthy on 2012-02-16
-
merge
- 935. By Lohith D Shivamurthy on 2012-01-26
-
[places] replace 'system' i teardown too
- 934. By Lohith D Shivamurthy on 2012-01-26
-
[places][tests] Use the global and remove the local @sut
- 933. By Lohith D Shivamurthy on 2012-01-26
-
[places][tests] replace sysytem with .execute_
shell_command - 932. By Lohith D Shivamurthy on 2012-01-25
-
[shell][dash] Review comments fixed
- 931. By Lohith D Shivamurthy on 2012-01-24
-
[shell] Have a separate QML for new renderer filter-
checkoption- compact - 930. By Lohith D Shivamurthy on 2012-01-24
-
[shell] Add comments to test cases
- 929. By Lohith D Shivamurthy on 2012-01-24
-
[shell] Tests added to verify the new renderer


The tests rely on the fact that Application and Music lens as we know them have 2 and 3 columns, respectively. check-option- compact.
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-
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?