Merge lp://staging/~nick-dedekind/unity/phablet-greeter-indicators into lp://staging/unity/phablet

Is this WIP for a particular reason?

From Saviq:

Only thing I'm not sure of here is that it's possible to tap on the indicators to open them, looks to me like a potential "phone in pocket" issue.

Maybe we should only leave the drag gesture when over greeter?

> Is this WIP for a particular reason?

No, was just waiting on design review on your branch from Katie.
But since we've moved here then I've added the design review here as well.

> From Saviq:
>
> Only thing I'm not sure of here is that it's possible to tap on the indicators
> to open them, looks to me like a potential "phone in pocket" issue.
>
> Maybe we should only leave the drag gesture when over greeter?

Problem with disabling tap is that you won't be able to access the device overview page anymore.

FAILED: Continuous integration, rev:537
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/689/
Executed test runs:
    UNSTABLE: http://s-jenkins:8080/job/unity-phablet-qmluitests/501
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/691
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/691/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-i386-ci/689
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/565
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/565/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/569
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1218

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/689/rebuild

FAILED: Continuous integration, rev:537
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/719/
Executed test runs:
    UNSTABLE: http://s-jenkins:8080/job/unity-phablet-qmluitests/543
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/721
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/721/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-i386-ci/719
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/595
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/595/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/599
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1262

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/719/rebuild

qmluitests don't pass

The below tests don't pass:
    qmltestrunner.Panel::test_search_click_when_disabled
    qmltestrunner.Panel::test_search_click_when_enabled
    qmltestrunner.SearchIndicator::test_hideUp
    qmltestrunner.SearchIndicator::test_show

FAILED: Continuous integration, rev:537
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/747/
Executed test runs:
    UNSTABLE: http://s-jenkins:8080/job/unity-phablet-qmluitests/582
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/749
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/749/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-i386-ci/747
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/623
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/623/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/627
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1295

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/747/rebuild

FAILED: Continuous integration, rev:537
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/852/
Executed test runs:
    UNSTABLE: http://s-jenkins:8080/job/unity-phablet-qmluitests/772
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/728/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/732/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/852/rebuild

FAILED: Continuous integration, rev:537
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/880/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/unity-phablet-qmluitests/809/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/756/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/760/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/880/rebuild

PASSED: Continuous integration, rev:539
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/898/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-phablet-qmluitests/832
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/774
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/774/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/778

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/898/rebuild

I don't think we should have tap to open on indicators in the greeter. It feels like a gesture that is too easy and would be prone to lots of accidental taps.

Tapping in the top part of the greeter screen should trigger a hint that the indicator menus open. (hint = reveal a small section of the indicator menu, and then fairly quickly hide it.)

This is similar to what we plan for the launcher, see Michael Zanetti's ~mzanetti/unity/phablet-tease-launcher

23 + // because of dynamic assignment of states, we need to assign state after completion.
24 + Component.onCompleted: state = "initial"

Is this really true? Why would « state: "initial" » not work?

=====

43 - property alias searchEnabled: search.enabled
44 + property bool searchVisible: true

Why this rename?

=====

94 - function show() {
95 - search.state = "visible"
96 + function show_state() {
97 + return "visible"
98 }
99
100 - function hideUp() {
101 - search.state = "hiddenUp"
102 + function hideUp_state() {
103 + return "hiddenUp"
104 }

169 - searchIndicator.hideUp()
170 + searchIndicator.state = searchIndicator.hideUp_state()

177 - searchIndicator.show()
178 + searchIndicator.state = searchIndicator.show_state()

This is weird, why the _state() methods at all? Either direct names or readonly props would suffice, no?

=====

116 + available: true

This is the default.

> 23 + // because of dynamic assignment of states, we need to assign state
> after completion.
> 24 + Component.onCompleted: state = "initial"
>
> Is this really true? Why would « state: "initial" » not work?
>

Yes, it's a bit of a quirk with QML. It seems the dynamic assignments are computed after the static ones.

ie.
  states: pinnedModeStates
is assigned after
  state: "initial"
no matter where in the code it's placed.

so, when states property is assigned after state, it clears the state "initial".

> =====
>
> 43 - property alias searchEnabled: search.enabled
> 44 + property bool searchVisible: true
>
> Why this rename?
>

Because it's visiblily vs enabling. Previously search was always visible, only it was disabled. Now it's not visible at all.

> =====
>
> 94 - function show() {
> 95 - search.state = "visible"
> 96 + function show_state() {
> 97 + return "visible"
> 98 }
> 99
> 100 - function hideUp() {
> 101 - search.state = "hiddenUp"
> 102 + function hideUp_state() {
> 103 + return "hiddenUp"
> 104 }
>
> 169 - searchIndicator.hideUp()
> 170 + searchIndicator.state = searchIndicator.hideUp_state()
>
> 177 - searchIndicator.show()
> 178 + searchIndicator.state = searchIndicator.show_state()
>
> This is weird, why the _state() methods at all? Either direct names or
> readonly props would suffice, no?
>

Done.
Guess I'm not a fan of setting state by string from external object. :(

> =====
>
> 116 + available: true
>
> This is the default.

Done

PASSED: Continuous integration, rev:540
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/975/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-phablet-qmluitests/944
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/851
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/851/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/855
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1554

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/975/rebuild

W dniu 20.05.2013 16:01, Nick Dedekind pisze:
>> 23 + // because of dynamic assignment of states, we need to assign state
>> after completion.
>> 24 + Component.onCompleted: state = "initial"
>>
>> Is this really true? Why would « state: "initial" » not work?
>>
>
> Yes, it's a bit of a quirk with QML. It seems the dynamic assignments are computed after the static ones.
>
> ie.
> states: pinnedModeStates
> is assigned after
> state: "initial"
> no matter where in the code it's placed.
>
> so, when states property is assigned after state, it clears the state "initial".

I could even imagine they're evaluated in alphabetical order ;). After
all you're not supposed to rely on that order.

>> 94 - function show() {
>> 95 - search.state = "visible"
>> 96 + function show_state() {
>> 97 + return "visible"
>> 98 }
>> 99
>> 100 - function hideUp() {
>> 101 - search.state = "hiddenUp"
>> 102 + function hideUp_state() {
>> 103 + return "hiddenUp"
>> 104 }
>>
>> 169 - searchIndicator.hideUp()
>> 170 + searchIndicator.state = searchIndicator.hideUp_state()
>>
>> 177 - searchIndicator.show()
>> 178 + searchIndicator.state = searchIndicator.show_state()
>>
>> This is weird, why the _state() methods at all? Either direct names or
>> readonly props would suffice, no?
>>
>
> Done.
> Guess I'm not a fan of setting state by string from external object. :(

Well, then the previous approach (with hideUp(), show()) was one that
didn't do it, no? .state = .show_state() is IMO the same as .state =
"visible" if show_state() just returns "visible". If you want to avoid
that, a show() function that sets the state to "visible" internally was
the right way, no? Why do we need to change the hideUp() and show()
methods at all?

--
Michał Sawicz <email address hidden>
Canonical Services Ltd.

> W dniu 20.05.2013 16:01, Nick Dedekind pisze:
> >> 23 + // because of dynamic assignment of states, we need to assign
> state
> >> after completion.
> >> 24 + Component.onCompleted: state = "initial"
> >>
> >> Is this really true? Why would « state: "initial" » not work?
> >>
> >
> > Yes, it's a bit of a quirk with QML. It seems the dynamic assignments are
> computed after the static ones.
> >
> > ie.
> > states: pinnedModeStates
> > is assigned after
> > state: "initial"
> > no matter where in the code it's placed.
> >
> > so, when states property is assigned after state, it clears the state
> "initial".
>
> I could even imagine they're evaluated in alphabetical order ;). After
> all you're not supposed to rely on that order.
>
>
> >> 94 - function show() {
> >> 95 - search.state = "visible"
> >> 96 + function show_state() {
> >> 97 + return "visible"
> >> 98 }
> >> 99
> >> 100 - function hideUp() {
> >> 101 - search.state = "hiddenUp"
> >> 102 + function hideUp_state() {
> >> 103 + return "hiddenUp"
> >> 104 }
> >>
> >> 169 - searchIndicator.hideUp()
> >> 170 + searchIndicator.state = searchIndicator.hideUp_state()
> >>
> >> 177 - searchIndicator.show()
> >> 178 + searchIndicator.state = searchIndicator.show_state()
> >>
> >> This is weird, why the _state() methods at all? Either direct names or
> >> readonly props would suffice, no?
> >>
> >
> > Done.
> > Guess I'm not a fan of setting state by string from external object. :(
>
> Well, then the previous approach (with hideUp(), show()) was one that
> didn't do it, no? .state = .show_state() is IMO the same as .state =
> "visible" if show_state() just returns "visible". If you want to avoid
> that, a show() function that sets the state to "visible" internally was
> the right way, no? Why do we need to change the hideUp() and show()
> methods at all?
>

Because the search bar state is dependant on the searchVisible flag and the width of the panel, search & indicatorbar, not the state of the Inidcators.
Thought it better to have the state determination in a single place than to have multiple entry points.
I could have had connections to the change of various items which determine the state of the search call into a function to re-interprets that state. But it's a bit messy.

> --
> Michał Sawicz <email address hidden>
> Canonical Services Ltd.

FYI, I believe the change for the search bar states will fix https://bugs.launchpad.net/touch-preview-images/+bug/1181249

Need to confirm though.

I'm not buying the s/enabled/visible/ change, but that's fine ;)

We need to later hide the search label when dash isn't focused (and even later - interface with the app to see if it supports search to determine whether to hide or not).

Also it feels like it's the search label that will hide when there's too many indicators, and it should be the other way 'round - it's the overflow indicators that should hide.

Aaand also the SEARCH label will need to get replaced (slide in from the bottom) with corresponding MUSIC, APPS, etc., or the actual search query (in double quotes).

Aaaand it should also be possible to edge-swipe the search entry down (should on the phone, not only tap on it...

Let me add those as WIs, so we don't lose them :)