D8641: FormLayout

[Eike Hein]

hein added a comment.


  In general it looks quite good to me.

INLINE COMMENTS

> FormLayout.qml:66
> +     */
> +    property bool wideMode: width >= lay.wideImplicitWidth
> +

Doesn't Kirigami have wideMode logic somewhere else too?

> FormLayout.qml:114
> +                    //NOTE: this is an heuristic but there are't better ways
> +                    (item.model !== undefined && item.children.length == 0)) {
> +                    continue;

Isn't this going to cause tons of ReferenceErrors without "x in y" style checks?

> mnemonicattached.h:162
> +    //global mapping of mnemonics
> +    //TODO: map by QWindow
> +    static QHash<QKeySequence, MnemonicAttached *> s_sequenceToObject;

This can go with https://phabricator.kde.org/D8827, right?

REPOSITORY
  R169 Kirigami

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

To: mart, #plasma, #kirigami, hein, davidedmundson
Cc: broulik, colomar, ngraham, davidedmundson, plasma-devel, ngiannip, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, hein
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20171127/b429fc94/attachment.html>