Merge lp:~aacid/unity8/new_tabbar into lp:unity8

Proposed by Albert Astals Cid
Status: Merged
Approved by: Andrea Cimitan
Approved revision: 761
Merged at revision: 820
Proposed branch: lp:~aacid/unity8/new_tabbar
Merge into: lp:unity8
Diff against target: 52 lines (+19/-11)
1 file modified
qml/Dash/DashContent.qml (+19/-11)
To merge this branch: bzr merge lp:~aacid/unity8/new_tabbar
Reviewer Review Type Date Requested Status
Andrea Cimitan (community) Approve
Tim Peeters (community) Approve
PS Jenkins bot (community) continuous-integration Needs Fixing
Review via email: mp+210453@code.launchpad.net

Commit message

Adapt to new TabBar

Description of the change

* Are there any related MPs required for this MP to build/function as expected?
https://code.launchpad.net/~tpeeters/ubuntu-ui-toolkit/tabsModelIndex/+merge/210568

* Did you perform an exploratory manual test run of your code change and any related functionality?
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

To post a comment you must log in.
Revision history for this message
Tim Peeters (tpeeters) wrote :

Code looks good, thanks!

We need to check that the autopilot tests still pass on device.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:760
http://jenkins.qa.ubuntu.com/job/unity8-ci/2469/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/3835
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/3421/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/1339/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/990
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/994
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/994/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/990
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/3366
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/3853
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/3853/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/3423
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/3423/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/5784/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/4686

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Albert Astals Cid (aacid) wrote :

> Code looks good, thanks!
>
> We need to check that the autopilot tests still pass on device.

Yes, according to https://wiki.ubuntu.com/Process/Merges/Checklists/Unity8 that is the task of the reviewer, can you do it? Shall we find someone to do it?

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:760
http://jenkins.qa.ubuntu.com/job/unity8-ci/2512/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/3971
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/3556/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/1382
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/1033
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/1037
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/1037/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/1033
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/3467
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4006
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4006/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/3558
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/3558/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/5891/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/4847

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

review: Needs Fixing (continuous-integration)
lp:~aacid/unity8/new_tabbar updated
761. By Albert Astals Cid

Merge

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:761
http://jenkins.qa.ubuntu.com/job/unity8-ci/2616/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/4221
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/3805/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/1486
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/1137
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/1141
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/1141/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/1137
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/3664
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4294
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4294/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/3807
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/3807/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/6101/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/5175

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Tim Peeters (tpeeters) wrote :

I tested this in silo 003 together with https://code.launchpad.net/~aacid/unity8/new_tabbar/+merge/210453

unity8 results: http://paste.ubuntu.com/7204822/ (3 errors, 0 failures)
ubuntuuitoolkit results: http://paste.ubuntu.com/7204836/ (1 error, 0 failures)

The UITK error happened in settle_before, so is not related to UITK.
The unity errors are also I think not related to the changes here, but it is best if a unity team member has a look at them also.

I also did some manual testing of shorts, gallery, weather, unity, dialer and I did not encounter problems.

review: Approve
Revision history for this message
Tim Peeters (tpeeters) wrote :

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

Yes

 * Did CI run pass? If not, please explain why.
errors in before_settle, after_settle and networking, see http://paste.ubuntu.com/7204822/

before_settle and after_settle always failed on my device.
networking is not touched here so I think it is also not related to the MR.

review: Approve
Revision history for this message
Andrea Cimitan (cimi) wrote :

apart that I'd use dashContentList.currentIndex === -1 && tabBar.selectedIndex !== -1

Tested and works

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'qml/Dash/DashContent.qml'
--- qml/Dash/DashContent.qml 2014-03-17 11:44:05 +0000
+++ qml/Dash/DashContent.qml 2014-03-25 09:18:56 +0000
@@ -17,6 +17,7 @@
17import QtQuick 2.017import QtQuick 2.0
18import Ubuntu.Components 0.118import Ubuntu.Components 0.1
19import Unity 0.219import Unity 0.2
20import Utils 0.1
20import "../Components"21import "../Components"
2122
22Item {23Item {
@@ -188,23 +189,30 @@
188 width: parent.width189 width: parent.width
189 style: DashContentTabBarStyle {}190 style: DashContentTabBarStyle {}
190191
191 model: dashContentList.model192 SortFilterProxyModel {
192193 id: tabBarModel
193 onSelectedIndexChanged: {194
194 if (dashContentList.currentIndex == -1 && tabBar.selectedIndex != -1) {195 model: dashContentList.model
195 // TODO This together with the Timer below196
196 // are a workaround for the first tab sometimes not showing the text.197 property int selectedIndex: -1
197 // But Tabs are going away in the future so not sure if makes198 onSelectedIndexChanged: {
198 // sense invetigating what's the problem at this stage199 if (dashContentList.currentIndex == -1 && tabBar.selectedIndex != -1) {
199 selectionModeTimer.restart();200 // TODO This together with the Timer below
201 // are a workaround for the first tab sometimes not showing the text.
202 // But Tabs are going away in the future so not sure if makes
203 // sense invetigating what's the problem at this stage
204 selectionModeTimer.restart();
205 }
206 dashContentList.currentIndex = selectedIndex;
200 }207 }
201 dashContentList.currentIndex = selectedIndex;
202 }208 }
203209
210 model: tabBarModel.count > 0 ? tabBarModel : null
211
204 Connections {212 Connections {
205 target: dashContentList213 target: dashContentList
206 onCurrentIndexChanged: {214 onCurrentIndexChanged: {
207 tabBar.selectedIndex = dashContentList.currentIndex215 tabBarModel.selectedIndex = dashContentList.currentIndex
208 }216 }
209 }217 }
210218

Subscribers

People subscribed via source and target branches