Code review comment for lp:~macslow/unity/phablet-notification-renderer
Revision history for this message
| Mirco Müller (macslow) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
0e1f616
(Get the code!)
> 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?
No, this here is meant for the special "icon+summary" layout case Design wants. This is different from a collapsing snap-decision, which is only meant to be used on large screens.
> 141 + visible: type == "Notifications.
> 145 + model: actions
> reference by object - notification.type, notification.
That does not work. How can I make the object known? Do I need to change the way data is passed in the test?
> 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):
I could get rid of "anchors.fill: parent", but I have to keep "width: parent.width". That way the width of the ListView holding the notifications follows any embedding parent-item. Otherwise there's no way to tell the ListView what horizontal space it may occupy.