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

Reply via email to