hein marked 7 inline comments as done. hein added a comment.
Will update once I have a Qt again. INLINE COMMENTS > broulik wrote in ContextMenu.qml:537 > Unrelated Will do. > broulik wrote in GroupDialog.qml:63 > Remove Will do. > broulik wrote in GroupDialog.qml:113-114 > Why this change? I was doing heavy refactoring and changing the nesting and trying different things a few times, happened along the way. I'll clean it out of the diff. > broulik wrote in GroupDialog.qml:119 > FTR: Qt 5.9 will add a "positioningComplete" signal :) Nifty, about time. That also means we can remove some fugly code in main.qml for the layouting and delegate geo publishs. > broulik wrote in GroupDialog.qml:156 > Can you perhaps have the task do that automatically since fwict you always do > forceActiveFocus and then ensureItemVisible: > > onActiveFocusChanged: { > if (activeFocus) { > ensureItemVisible(this); > } > } I see the appeal, but I'd rather not do it implicitly and calling GroupDialog API ... I'm rather unhappy with how much coupling there is between Task and GroupDialog already (writing a delegate that relies on API outside of it is always pretty ugly). I'll think about whether I can find some other nice solution. > broulik wrote in GroupDialog.qml:253 > ? Doing it declaratively creates a binding loop in this case. > broulik wrote in GroupDialog.qml:264 > Why not assign it directly above? > > property TextMetrics textMetrics: TextMetrics {} I think this wasn't possible in older Qt Quicks? Nice that it's possible now, will do. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D4229 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: hein, #plasma Cc: broulik, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas