Review Request 126804: DragArea: Implement grabbing delegate item

[Sebastian Kügler]

> On Jan. 21, 2016, 11:10 a.m., Sebastian Kügler wrote:
> > src/qmlcontrols/draganddrop/DeclarativeDragArea.cpp, line 390
> > <https://git.reviewboard.kde.org/r/126804/diff/1/?file=434496#file434496line390>
> >
> >     Q_FOREACH and const&
> 
> David Rosca wrote:
>     But the value is not used here, so the plain `for` could be faster? (no accessing the data in container).
>     
>     Originally, it was Q_FOREACH, but it looked really weird to me. But sure, if you think it would be better, I'll change it back.

Ah, right. Good point.


> On Jan. 21, 2016, 11:10 a.m., Sebastian Kügler wrote:
> > src/qmlcontrols/draganddrop/DeclarativeDragArea.cpp, line 311
> > <https://git.reviewboard.kde.org/r/126804/diff/1/?file=434496#file434496line311>
> >
> >     So this means that we either pass an empty image, or an outdated one? I'm not entirely following the code, but I think the image should be redrawn when the drag starts, as the data may change and we definitely want to show the currently dragged data.
> 
> David Rosca wrote:
>     No, at this point, the drag should be started. But if we have delegate (QQuickItem), we must first render the image before starting the drag. If the delegate is null, we start the drag immediately and as image use either delegateImage or guess it from mimeData (this code path is not changed).
> 
> Sebastian Kügler wrote:
>     Ow right, delegateImage gets set by the the user? If so, the delegateImage should be preferred to our rendered grab, no?
> 
> David Rosca wrote:
>     Yes, and also `delegate` is set by user. I think we should prefer `delegate` as it provides the exact image as the item being grabbed. But then again, it doesn't really matter (though it should be noted in docs) - user can choose to just don't set `delegate`.

Makes sense, I'll leave it to you which one would be preferred.


- Sebastian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126804/#review91400
-----------------------------------------------------------


On Jan. 18, 2016, 9 p.m., David Rosca wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126804/
> -----------------------------------------------------------
> 
> (Updated Jan. 18, 2016, 9 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kdeclarative
> 
> 
> Description
> -------
> 
> Implement grabbing image of delegate with QQuickItem::grabToImage.
> 
> 
> Diffs
> -----
> 
>   src/qmlcontrols/draganddrop/DeclarativeDragArea.h 32092ab 
>   src/qmlcontrols/draganddrop/DeclarativeDragArea.cpp ee78ff9 
> 
> Diff: https://git.reviewboard.kde.org/r/126804/diff/
> 
> 
> Testing
> -------
> 
> QQuickItem::grabToImage is async, not sure how safe is to delay the start of drag. It seems to work without any issues though.
> 
> 
> Thanks,
> 
> David Rosca
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20160121/7a176a21/attachment.html>