Code review comment for lp:~dandrader/unity8/panelDragHandle

Revision history for this message
Daniel d'Andrada (dandrader) wrote :

> Semi-colons (mostly in tests).
> -----------------------------
>
> 68 - function handlePress() {
> 69 - menuContent.hideAll()
> 70 - menuContent.activateContent()
> 71 - indicators.state = "hint"
> 72 - }
>
> Indicator menu's don't populate anymore. Need to call activateContent before
> showing.

Good catch! You can only spot the difference when trying out on the device. Would be nice to have a test on that but with all those indirections and abstractions coming from indicators-client side, I feel it would be a pain to do so.
Fixed.

>
>
> 41 + showAnimation: StandardAnimation {
> 42 + property: "height"
> 43 + duration: 350
> 44 + to: pinnedMode ? openedHeight - panelHeight : openedHeight
> 45 + easing.type: Easing.OutCubic
> 46 + }
>
> 48 + hideAnimation: StandardAnimation {
> 49 + property: "height"
> 50 + duration: 350
> 51 + to: panelHeight
> 52 + easing.type: Easing.OutCubic
> 53 + }
>
> Perhaps we need a animation standard for showing/hiding? Each showable seems
> to use it's own values.
> Need to ask design about this. Can you add a TODO.

Done.

>
>
> 60 + if (!showAnimation.running && !hideAnimation.running /*&&
> !revealer.hintingAnimation.running*/) {
>
> Can you remove the comment?

Done.

>
>
> 244 - item.reset()
> 245 + //item.reset()
>
> why commented out?

Fixed. Explained on a previous reply.

>
>
> 374 + property real showAnimationProgress: MathLocal.clamp(1 + x /
> width, 0, 1)
> 375 +
>
> not used.

Removed. Wonder where that came from. Maybe the result of some previous syncs with trunk

>
> 348 + property real unitProgress: height / parent.height
>
> Should change between 0.0 and 1.0, but runs between about 0.04 and 0.95
> meaning that background will always be slightly darkened.
>

Jeez, would have never found that. Thanks for spotting this! Fixed.

There's a lot going on in the indicators animations. So unfortunately the resulting QML is still rather puzzling (openedHeight vs. referenceOpenendHeight, handle being outside its parent and growing separately, etc) and refactoring is still tricky (we don't have enough tests to cover all that panel does)

« Back to merge proposal