Code review comment for lp:~macslow/unity/phablet-notification-renderer

=========

48 + color: Qt.rgba(0, 0, 0, 0.85)
with that, shouldn't you be able to just drop the Item?

=========

54 + top: parent.top
unnecessary

=========

56 + height: childrenRect.height + contentColumn.anchors.margins * 2
since spacing / margins are units.gu(1) everywhere, let's just use contentColumn.spacing everywhere?

=========

81 + width: (body == "") ? units.gu(2) : units.gu(6)
82 + height: (body == "") ? units.gu(2) : units.gu(6)
we need to be able to collapse items that _do_ have body text, no? actually, it's the secondaryIcon that should stay on screen in collapsed state, even if there is a primary icon, no? IMO it's a perfect place to use States and Transitions [1]

expanded => collapsed transition as follows:
SequentialAnimation {
  ParallelAnimation {
    icon shrinks
    icon fades out
    labelColumn.height shrinks to summaryLabel.height
    labelColumn fades out
  }
  PropertyAction {
    icon becomes invisible
    bodyLabel becomes invisible
  }
}

collapsed => expanded transition as follows:
SequentialAnimation {
  PropertyAction {
    icon becomes visible
    bodyLabel becomes visible
  }
  ParallelAnimation {
    icon grows
    icon fades in
    labelColumn.height grows to labelColumn.implicitHeight
    labelColumn fades in
  }
}

you also need to bind labelColumn.clip to Transition.running [2] of the two above.

=========

107 + height: childrenRect.height
this shouldn't be needed in Positioners - they get sized by their children implicitly

=========

113 + width: parent.width - x
this should only be like that in labelColumn, the Labels themselves should anchor to parent.left/right

=========

124 + visible: body != ""
as mentioned above, use States and Transitions for that

=========

141 + visible: type == "Notifications.Type.SnapDecision"
145 + model: actions
reference by object - notification.type, notification.actions

=========

74 + width: parent.width
142 + width: childrenRect.width
you should anchor the Rows to parent.left/right instead

=========

because of the quality issues, let's drop sourceSize for now (sorry)

=========

46 + width: parent.width
191 + anchors.fill: parent
these need to go away (component dimensions should generally be handled in their parents, unless they depend on content dimensions):

=========

225 +
rogue newline

=========

237 +<<<<<<< TREE
238 +=======
239 +add_qml_test(DashPreview)
240 +add_qml_test(NotificationRenderer)
241 +>>>>>>> MERGE-SOURCE
conflict (DashPreview shouldn't be touched)

=========

add some controls next to the view so that you can manually drive the test (see other tests in qmluitests for examples)

=========

[1] http://qt-project.org/doc/qt-5.0/qtquick/qtquick-statesanimations-states.html
[2] http://qt-project.org/doc/qt-5.0/qtquick/qml-qtquick2-transition.html#running-prop