Code review comment for lp:~macslow/unity/phablet-snap-decision-action-expansion

Revision history for this message
MichaƂ Sawicz (saviq) wrote :

71 + property real buttonWidth: (width - units.gu(1)) / 2

103 + leftMargin: units.gu(1)

118 + spacing: units.gu(1)

Please use contentColumn.spacing here

79 + height: units.gu(4)

107 + height: units.gu(4)

Please use anchors instead.

90 + NumberAnimation {
91 + duration: UbuntuAnimation.SnapDuration
92 + easing: UbuntuAnimation.StandardEasing
93 + }

193 - duration: Timings.snapBeat
194 - easing.type: Timings.easing
195 + duration: UbuntuAnimation.SnapDuration
196 + easing: UbuntuAnimation.StandardEasing

204 - duration: Timings.snapBeat
205 - easing.type: Timings.easing
206 + duration: UbuntuAnimation.SnapDuration
207 + easing: UbuntuAnimation.StandardEasing

215 - duration: Timings.fastBeat
216 - easing.type: Timings.easing
217 + duration: UbuntuAnimation.FastDuration
218 + easing: UbuntuAnimation.StandardEasing

225 - duration: Timings.fastBeat
226 - easing.type: Timings.easing
227 + duration: UbuntuAnimation.FastDuration
228 + easing: UbuntuAnimation.StandardEasing

You should use UbuntuNumberAnimation with the default easing here.

115 + id: buttonColumn
116 +

That's unused.

334 + }
335 + else {

Please use a single line.

329 +
330 + // click to collapse
331 + mouseClick(buttonCancel, buttonCancel.width / 2, buttonCancel.height / 2)
332 + waitForRendering(notification)
333 + actionSpy.clear()

This seems unused?

339 + actionSpy.clear()

Should probably happen in cleanup()?

review: Needs Fixing

« Back to merge proposal