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
