Unity 8

Merge lp:~feng-kylin/unity8/fix-lp1413791 into lp:unity8

fix-lp1413791
Merge into trunk

Proposed by handsome_feng on 2015-02-12

Status:

Merged

Approved by:

Albert Astals Cid on 2015-07-21

Approved revision:

1613

Merged at revision:

1892

Proposed branch:

lp:~feng-kylin/unity8/fix-lp1413791

Merge into:

lp:unity8

Diff against target:

94 lines (+44/-1)

4 files modified

qml/Dash/Dash.qml (+1/-1)
qml/Dash/DashContent.qml (+1/-0)
qml/Dash/GenericScopeView.qml (+5/-0)
tests/qmltests/tst_Shell.qml (+37/-0)

To merge this branch:

bzr merge lp:~feng-kylin/unity8/fix-lp1413791

Medium

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Albert Astals Cid (community)		2015-02-12	Approve on 2015-07-21
PS Jenkins bot (community)	continuous-integration		Needs Fixing on 2015-07-20
Michael Zanetti (community)			Needs Fixing on 2015-07-17
Ubuntu design		2015-02-13	Pending
Review via email: mp+249452@code.launchpad.net

Commit message

makes left swip reset the search string.

Description of the change

Added a series of signal and slot to deal with resetSearch when left swip.

* Are there any related MPs required for this MP to build/function as expected? Please list.

* Did you perform an exploratory manual test run of your code change and any related functionality?

yes

* Did you make sure that your branch does not contain spurious tags?

yes

* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?

n/a

* If you changed the UI, has there been a design review?

n/a

Revision history for this message

Albert Astals Cid (aacid) wrote on 2015-02-13:

Thanks for the patch, as you can see on the bug (affects Ubuntu UX as NEW), this is waiting for input by the design team.

Revision history for this message

Albert Astals Cid (aacid) wrote on 2015-02-13:

And by this i mean the bug itself.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-04-17:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/5566/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Albert Astals Cid (aacid) wrote on 2015-04-17:

file:///tmp/buildd/unity8-8.02+15.04.20150409.1bzr1597pkg0vivid729/qml/Dash/Dash.qml:48:9: Cannot assign to non-existent property "onResetAllRequested"

review: Needs Fixing

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-04-27:

FAILED: Continuous integration, rev:1596
http://jenkins.qa.ubuntu.com/job/unity8-ci/5619/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2451/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/782
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/784
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/784
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/2130/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2449
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2449/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19925

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/5619/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Albert Astals Cid (aacid) wrote on 2015-04-28:

Why did you introduce the resetAll if it's always used when dash.setCurrentScope is used? Maybe just do the resetAll code in it?

review: Needs Information

Revision history for this message

handsome_feng (feng-kylin) wrote on 2015-04-29:

> Why did you introduce the resetAll if it's always used when
> dash.setCurrentScope is used? Maybe just do the resetAll code in it?

Because I think long left swipe/BFB may reset something else more than the search string in the future. but I think you are right, the signal is not necessary here, I will modify this.Thank you!

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-04-29:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/5634/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-04-29:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/5635/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Albert Astals Cid (aacid) wrote on 2015-04-29:

- dash.setCurrentScope(0, animate, false)
+ dash.setCurrentScope(0, animate, true)

Why this change?

review: Needs Information

Revision history for this message

handsome_feng (feng-kylin) wrote on 2015-04-30:

> - dash.setCurrentScope(0, animate, false)
> + dash.setCurrentScope(0, animate, true)
>
> Why this change?

To reset the search query and position the list in the beginning when click home button on the launcher.
Notice that in setCurrentScopeAtIndex(index, animate, reset):
            if (reset) {
                dashContentList.currentItem.item.positionAtBeginning()
+ dashContentList.currentItem.item.resetSearch()
            }

Revision history for this message

Albert Astals Cid (aacid) wrote on 2015-04-30:

The change of behaviour in "swipe over dash" is not wanted, the launcher should not be hidden if we are just on the dash and do a long swipe

review: Needs Fixing

Revision history for this message

Albert Astals Cid (aacid) wrote on 2015-05-07:

Do we need those clear() in tst_Shell.qml?

I removed them and the test still passes fine.

review: Needs Information

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-05-11:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/5701/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Albert Astals Cid (aacid) wrote on 2015-05-12:

Text conflict in qml/Dash/GenericScopeView.qml
1 conflicts encountered.

Please re-merge your branch with lp:untiy8

review: Needs Fixing

Revision history for this message

handsome_feng (feng-kylin) wrote on 2015-05-15:

> Text conflict in qml/Dash/GenericScopeView.qml
> 1 conflicts encountered.
>
> Please re-merge your branch with lp:untiy8

done

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-05-15:

FAILED: Continuous integration, rev:1604
http://jenkins.qa.ubuntu.com/job/unity8-ci/5726/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-wily-touch/5
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-amd64-ci/4
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-i386-ci/4
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-wily-mako/4
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/5
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/5/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/20536

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/5726/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Albert Astals Cid (aacid) wrote on 2015-05-19:

Breaks in this case

* Be on non first scope
* Do a long left swipe
* See how you changed to the first scope

That should not happen

review: Needs Fixing

Revision history for this message

Albert Astals Cid (aacid) wrote on 2015-05-19:

Also please add a test for the thing you're fixing

Revision history for this message

Albert Astals Cid (aacid) wrote on 2015-05-20:

retriggered CI

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-05-20:

FAILED: Continuous integration, rev:1605
http://jenkins.qa.ubuntu.com/job/unity8-ci/5747/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-wily-touch/26
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-amd64-ci/25
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-wily-i386-ci/25
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-wily-mako/18
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/26
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-wily-armhf/26/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/20659

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/5747/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Albert Astals Cid (aacid) wrote on 2015-05-21:

I think maybe i'm not explaining myself correctly, this needs to be fixed:
* Make sure you have at least two scopes
* Be on the second scope
* Do a long left swipe that shows the launhcher
* Release the finger
* See how the scope has changed to the first scope

It should stay in the second scope like it does if when not using this branch.

review: Needs Fixing

Revision history for this message

Albert Astals Cid (aacid) wrote on 2015-05-22:

Retriggered CI

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-05-22:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/5759/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-05-26:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/5769/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Albert Astals Cid (aacid) wrote on 2015-05-27:

There's another regression. Steps to reproduce:
* Have various scopes in dash
* Go to the last one
* Start app from launcher
* Do a long swipe from the left
* See that until you release the shown scope is the last scope instead of the first one

This is most likely due to the removal of the code of onDashSwipeChanged

review: Needs Fixing

Revision history for this message

Albert Astals Cid (aacid) wrote on 2015-06-22:

Please move the launcherShowDashHomeSpy.clear(); to either the beginning of the function or to init(), having it there in the middle of the function is a bit weird since it may imply it needs clearing after doing part of the things above it

review: Needs Fixing

Revision history for this message

Albert Astals Cid (aacid) wrote on 2015-07-02:

* Did you perform an exploratory manual test run of the code change and any related functionality?
Yes

* Did CI run pass?
As much as they pass lately.

* Did you make sure that the branch does not contain spurious tags?
Yes

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-07-15:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/6049/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Michael Zanetti (mzanetti) wrote on 2015-07-17:

Sorry. This branch failed QA verification.

It breaks going to the "home" scope when you're on another scope by pressing the BFB.

review: Needs Fixing

lp:~feng-kylin/unity8/fix-lp1413791 updated on 2015-07-20

1611. By handsome_feng on 2015-07-20: merge trunk
1612. By handsome_feng on 2015-07-20: BFB and left swipe both should reset the dash
1613. By handsome_feng on 2015-07-20: modified the test

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-07-20:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/6089/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Albert Astals Cid (aacid) wrote on 2015-07-21:

Re-approve the issue Michael mentioned has been fixed and can't find it having any other regression.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Mohammad Rakibul Hasan

Unity Team

handsome_feng

 === modified file 'qml/Dash/Dash.qml'
 --- qml/Dash/Dash.qml	2015-05-18 23:04:12 +0000
 +++ qml/Dash/Dash.qml	2015-07-20 08:36:50 +0000
@@ -41,7 +41,7 @@
          onSetCurrentScopeRequested: {
              if (!isSwipe || !window.active || bottomEdgeController.progress != 0 || scopeItem.scope || dashContent.subPageShown) {
                  if (bottomEdgeController.progress != 0 && window.active) animate = false;
--                dashContent.setCurrentScopeAtIndex(index, animate, isSwipe)
++                dashContent.setCurrentScopeAtIndex(index, animate, true)
                  backToDashContent()
+             }
+         }
 === modified file 'qml/Dash/DashContent.qml'
 --- qml/Dash/DashContent.qml	2015-03-19 15:25:45 +0000
 +++ qml/Dash/DashContent.qml	2015-07-20 08:36:50 +0000
@@ -84,6 +84,7 @@
              if (reset) {
                  dashContentList.currentItem.item.positionAtBeginning()
++                dashContentList.currentItem.item.resetSearch()
+             }
+         }
 === modified file 'qml/Dash/GenericScopeView.qml'
 --- qml/Dash/GenericScopeView.qml	2015-07-03 14:24:08 +0000
 +++ qml/Dash/GenericScopeView.qml	2015-07-20 08:36:50 +0000
@@ -67,6 +67,11 @@
          subPageLoader.closeSubPage()
+     }
++    function resetSearch() {
++        if(pageHeaderLoader.item && showPageHeader)
++            pageHeaderLoader.item.resetSearch()
++    }
++
      function itemClicked(index, result, item, itemModel, resultsModel, limitedCategoryItemCount, categoryId) {
          if (itemModel.uri.indexOf("scope://") === 0 || scope.id === "clickscope" || (scope.id === "videoaggregator" && categoryId === "myvideos-getstarted")) {
              // TODO Technically it is possible that calling activate() will make the scope emit
 === modified file 'tests/qmltests/tst_Shell.qml'
 --- tests/qmltests/tst_Shell.qml	2015-06-23 17:40:03 +0000
 +++ tests/qmltests/tst_Shell.qml	2015-07-20 08:36:50 +0000
@@ -472,6 +472,41 @@
              confirmLoggedIn(data.loggedIn)
+         }
++        function test_longLeftEdgeSwipeTakesToAppsAndResetSearchString() {
++            loadShell("phone");
++            swipeAwayGreeter();
++            dragLauncherIntoView();
++            dashCommunicatorSpy.clear();
++
++            tapOnAppIconInLauncher();
++            waitUntilApplicationWindowIsFullyVisible();
++
++            verify(ApplicationManager.focusedApplicationId !== "unity8-dash")
++
++            //Long left swipe
++            swipeFromLeftEdge(units.gu(30));
++
++            tryCompare(ApplicationManager, "focusedApplicationId", "unity8-dash");
++
++            compare(dashCommunicatorSpy.count, 1);
++        }
++
++        function test_ClickUbuntuIconInLauncherTakesToAppsAndResetSearchString() {
++            loadShell("phone");
++            swipeAwayGreeter();
++            dragLauncherIntoView();
++            dashCommunicatorSpy.clear();
++
++            var launcher = findChild(shell, "launcher");
++            var dashIcon = findChild(launcher, "dashItem");
++            verify(dashIcon != undefined);
++            mouseClick(dashIcon);
++
++            tryCompare(ApplicationManager, "focusedApplicationId", "unity8-dash");
++
++            compare(dashCommunicatorSpy.count, 1);
++        }
++
          function test_suspend() {
              loadShell("phone");
              swipeAwayGreeter();
@@ -1082,6 +1117,8 @@
+         }
          function test_tapUbuntuIconInLauncherOverAppSpread() {
++            launcherShowDashHomeSpy.clear();
++
              loadShell("phone");
              swipeAwayGreeter();