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

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

319 + readonly property int __maximumCollapsedLineCount: 7

Please hide it lower in the hierarchy.

=====

254 + height: parent.height

anchors, please.

=====

231 + property bool more: false

I wonder, shouldn't this be read only?

=====

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?

=====

321 + implicitHeight: titleLabel.visible ? titleLabel.height + textLabel.height : textLabel.height

childrenRect.height no good?

=====

329 + top: parent.top

Not needed, is it?

=====

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?

=====

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

=====

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

review: Needs Fixing

« Back to merge proposal