Code review comment for lp:~nick-dedekind/unity/phablet-greeter-indicators

Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

> 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

« Back to merge proposal