Code review comment for lp:~unity-team/unity8/unity8.previews_textSummary

> 319 + readonly property int __maximumCollapsedLineCount: 7
>
> Please hide it lower in the hierarchy.

Done

>
> =====
>
> 254 + height: parent.height
>
> anchors, please.
>

Used anchors top and bottom

> =====
>
> 231 + property bool more: false
>
> I wonder, shouldn't this be read only?
>

Thought about the case someone wants to start the widget with the text already expanded, so setting this to true

> =====
>
> 253 + Row {
> 254 + height: parent.height
> 255 +
> 256 + Rectangle {
> 257 + width: units.dp(1)
> 258 + height: parent.height
> 259 + opacity: 0.2
> 260 + color: Theme.palette.selected.backgroundText
> 261 + }
> 262 + Rectangle {
> 263 + width: units.dp(1)
> 264 + height: parent.height
> 265 + color: Qt.rgba(1.0, 1.0, 1.0, 0.1)
> 266 + }
> 267 + }
>
> Oh my, why isn't this an image?

Done
>
> =====
>
> 321 + implicitHeight: titleLabel.visible ? titleLabel.height +
> textLabel.height : textLabel.height
>
> childrenRect.height no good?
>

Visibility doesn't affect childrenRect

> =====
>
> 329 + top: parent.top
>
> Not needed, is it?
>

I always specify all dimensions, or three anchors and relative remaining dimension
> =====
>
> 514 + var mappedTextLabel = root.mapFromItem(textLabel, 0, 0)
> 515 + compare(mappedTextLabel.y, titleLabel.height)
>
> They're in the same geometry, do we really need the mapFromItem?
>

We're testing the placement now, and then when it disappears and moves on top. Comment added

> =====
>
> Add cleanup() method to revert widgetData to default after each test, instead
> of setting it explicitly.
>

Ok
> =====
>
> Add init() method with verify(typeof textLabel === "object", "TextLabel object
> could not be found.").

ok