D28666: Refactor for loops

ngraham added a comment. That was fixed, thanks. REPOSITORY R169 Kirigami REVISION DETAIL https://phabricator.kde.org/D28666 To: cblack, #kirigami, mart Cc: broulik, ngraham, plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, apol, ahiemstra, davidedmundson, mart

D28666: Refactor for loops

ngraham added a comment. This appears to have broken `SwipeListItem`. Now the actions don't show up anymore and there's loads of console spam: file:///home/nate/kde/usr/lib64/qml/org/kde/kirigami.2/templates/SwipeListItem.qml:421: TypeError: Type error file:///home/nate/kde/usr/l

D28666: Refactor for loops

This revision was automatically updated to reflect the committed changes. Closed by commit R169:6a0535050b8c: Refactor for loops (authored by cblack). REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28666?vs=79642&id=80710 REVISION DETAIL https://phabricator

D28666: Refactor for loops

cblack added a comment. In D28666#644079 , @ngraham wrote: > I'm not a Javascript expert, but the old/current versions seem much more readable to me. Is there a performance advantage to your proposed new versions? No, the main point is wr

D28666: Refactor for loops

cblack updated this revision to Diff 79642. cblack marked 3 inline comments as done. cblack added a comment. Address feedback REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28666?vs=79623&id=79642 BRANCH cblack/for-refactor (branched from master) REVIS

D28666: Refactor for loops

cblack added inline comments. INLINE COMMENTS > broulik wrote in Action.qml:156 > 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 u

D28666: Refactor for loops

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) {

D28666: Refactor for loops

ngraham added a comment. I'm not a Javascript expert, but the old/current versions seem much more readable to me. Is there a performance advantage to your proposed new versions? REPOSITORY R169 Kirigami REVISION DETAIL https://phabricator.kde.org/D28666 To: cblack, #kirigami Cc: ngraham

D28666: Refactor for loops

cblack updated this revision to Diff 79623. cblack added a comment. Use Array.prototype.*.call() instead of Array.from to reduce copying REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28666?vs=79621&id=79623 BRANCH cblack/for-refactor (branched from mas

D28666: Refactor for loops

cblack updated this revision to Diff 79621. cblack added a comment. Use Array.from REPOSITORY R169 Kirigami CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28666?vs=79614&id=79621 BRANCH cblack/for-refactor (branched from master) REVISION DETAIL https://phabricator.kde.org/D2

D28666: Refactor for loops

cblack created this revision. cblack added a reviewer: Kirigami. Herald added a project: Kirigami. Herald added a subscriber: plasma-devel. cblack requested review of this revision. REVISION SUMMARY For loops have been refactored to use more idiomatic JavaScript functions (`filter()`, `some()`,