Code review comment for lp:~unity-team/unity8/new-scopes-title-alignment

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

s/titleHorizontalAlignment/headerAlignment/, so that we don't have to rename it later when we apply it to other components.

=====

43 + verticalAlignment: Text.AlignVCenter

The Label has no height defined, so this doesn't do anything.

=====

55 + property var titleHorizontalAlignment: card.titleHorizontalAlignment
56 +
57 Card {
58 id: card
59 template: cardTool.template
60 @@ -110,6 +112,19 @@
61 width: cardTool.cardWidth || implicitWidth
62 height: cardTool.cardHeight || implicitHeight
63
64 + property var titleHorizontalAlignment: {
65 + var subtitle = components["subtitle"];
66 + var price = components["price"];
67 +
68 + var hasSubtitle = subtitle && (typeof subtitle === "string" || subtitle["field"])
69 + var hasPrice = price && (typeof price === "string" || subtitle["field"]);
70 +
71 + var isOnlyTextComponent = !hasSubtitle && !hasPrice;
72 + if (!isOnlyTextComponent) return Text.AlignLeft;
73 +
74 + return (template["card-layout"] === "horizontal") ? Text.AlignHCenter : Text.AlignLeft;
75 + }
76 +

Make it readonly, and just put the whole binding under CardTool, don't move it under Card.

=====

You also need to take summary into account.

review: Needs Fixing

« Back to merge proposal