D11316: Components for Cards

David Edmundson noreply at phabricator.kde.org
Thu Mar 15 00:38:49 UTC 2018


davidedmundson added inline comments.

INLINE COMMENTS

> CardsLayoutGallery.qml:83
> +            Kirigami.AbstractCard {
> +                Layout.fillWidth: true
> +                showClickFeedback: true

Why is it the responsiblility of the user of a CardLayout to set Layout.fillWidth: true on every item yet CardLayout goes to great lengths to auto-inject Layout.fillHeight on every item? Surely one of the other?

(also it seems to visually completely explode if you don't have this set, which sounds like the sign of a bug elsewhere)

> CardsLayout.qml:35
> + * such as a mobile phone screen.
> + * A CardsLayout should always be contained within a ColumnLayout.
> + * @inherits GridLayout

I can see you want it in a layout because you're setting the relevant attached properties, but why explicitly Column?

> CardsLayout.qml:55
> +
> +    Layout.preferredWidth: maximumColumnWidth * 2
> +    Layout.maximumWidth: maximumColumnWidth * 2

shouldn't this include the spacing? Otherwise it's always ever so slightly smaller than my maxSize

> BannerImage.qml:106
> +            color: source != "" ? "white" : Kirigami.Theme.textColor
> +            elide: Text.ElideRight
> +        }

this will never elide properly, the layout it's filling is not constrained.

     left: root.titleAlignment & Qt.AlignLeft ? parent.left : undefined
  right: root.titleAlignment & Qt.AlignRight ? parent.right : undefined

will only have one.

> AbstractApplicationHeader.qml:88
> -    transform: Translate {
> -        id: translateTransform
> -        y: {

What are these changes in the header about?

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D11316

To: mart, #kirigami, davidedmundson
Cc: apol, ngraham, davidedmundson, progwolff, plasma-devel, mart, hein
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20180315/e5971f84/attachment.html>


More information about the Plasma-devel mailing list