broulik added a comment.
+1 for the `.some()` use -1 on the convoluted `Array.prototype` things, especially when chained. INLINE COMMENTS > Action.qml:156 > readonly property var visibleChildren: { > - var visible = []; > - var child; > - for (var i in children) { > - child = children[i]; > - if (!child.hasOwnProperty("visible") || child.visible) { > - visible.push(child) > - } > - } > - return visible; > + return Array.prototype.filter.call(children, child => > !child.hasOwnProperty("visible") || child.visible) > } Why are you going through the prototype? Or is `children` not a "proper" Array? Also, can we use spread operator `...` here? And yes, please make those a bit more readable by using useful line breaks and using parentheses and braces. > FormLayout.qml:85 > var hint = 0; > - for (var i in knownItems) { > - hint = Math.max(hint, knownItems[i].Layout.preferredWidth > > 0 ? knownItems[i].Layout.preferredWidth : knownItems[i].implicitWidth); > - } > + knownItems.forEach(item => hint = Math.max(hint, > item.Layout.preferredWidth > 0 ? item.Layout.preferredWidth : > item.implicitWidth)); > return hint; Looks like a usecas for `Array.reduce`? > PassiveNotification.qml:62 > > - for (let i = 0; i < outerLayout.children.length - 3; ++i) { > - outerLayout.children[i].close(); > - } > + outerLayout.children.slice(0, -3).forEach(child => child.close()); > I would like a comment here. Note veryone is faimilar with `slice` in that fashion. > PassiveNotification.qml:72 > // Reorder items to have the last on top > - let children = outerLayout.children; > - for (let i in children) { > - children[i].Layout.row = children.length-1-i; > - } > + outerLayout.children.forEach(function(value, index) => { > + value.Layout.row = outerLayout.children.length-1-index; Why no arrow function? REPOSITORY R169 Kirigami REVISION DETAIL https://phabricator.kde.org/D28666 To: cblack, #kirigami Cc: broulik, ngraham, plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, apol, ahiemstra, davidedmundson, mart