D11663: Add InlineMessage type and Gallery app example page

[Eike Hein]

hein marked 5 inline comments as done.
hein added inline comments.

INLINE COMMENTS

> broulik wrote in InlineMessagesGallery.qml:45
> Wrong class name

Thanks, will fix.

> broulik wrote in InlineMessagesGallery.qml:115
> There's no `start-here` in Breeze, just `start-here-kde` which won't fall back to `start-here` (which is distro-branded for you apparently)

I need something generic that's not tied to Breeze, hmm ... suggestions?

> broulik wrote in InlineMessage.qml:364
> `findIndex` is new in Qt 5.9
> 
> [1] https://doc.qt.io/qt-5/qtqml-javascript-functionlist.html

This was copy-pasted from the Cards code (that's why Marco's copyright is in the header). Marco?

> broulik wrote in InlineMessage.qml:387
> Note that `visible` propagates recursively so when the component containing a default-visible message widget isn't shown this will still animate. But then I don't know if that's a thing and how to fix that..

I'm not sure it's a problem it will animate when you unhide with a parent.

> broulik wrote in InlineMessage.qml:100
> Should this be a `var` property so you could also pass a `QIcon` or "QtQuick Controls Icon"?

I wouldn't have any objections, but Kirigami.Icon.source is 'string' too, so it wouldn't work currently.

> broulik wrote in InlineMessage.qml:113
> `KMessageWidget` names it `closeButtonVisible`

Yes, but as mentioned in the review description I named it showCloseButton for consistency with another Kirigami component.

> broulik wrote in InlineMessage.qml:127
> `contentItem.hasOwnProperty("animating")`

Will do.

> broulik wrote in enums.h:39
> With Qt 5.8 this could become
> 
>   namespace InlineMessageType
>   {
>       Q_NAMESPACE
>       enum Type {
>   ...
> 
> with `qmlRegisterUncreatableMetaObject`
> (purely informational comment)

Could be a nice follow-up, but it's good practice to emulate surrounding style in a patch.

> broulik wrote in enums.h:42
> `Q_ENUM`
> 
> (also informational, the code around it does the same, could be cleaned up eventually)

See above.

REPOSITORY
  R169 Kirigami

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

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