Review Request: Support drag and drop file transfer from text-ui

[David Edmundson]

> On July 10, 2012, 10:44 p.m., David Edmundson wrote:
> > lib/chat-widget.cpp, line 370
> > <http://git.reviewboard.kde.org/r/105511/diff/1/?file=72045#file72045line370>
> >
> >     filename isn't unique enough, use the URL as you have it.
> >     
> >     
> >     OR
> >     
> >     simply QList<Tp::ChannelPtr> ?
> >
> 
> Dan Vratil wrote:
>     It's easier to retrieve the pointer from QMap using the file as index. Since only temporary files are stored in the map, the path will always be /tmp/kde-$USER/ so the filename is actually unique enough in this case. The variable should be named tempFilesTransfers though.

you're right about the name (because I was confused over how this is only for temp files)

the list would still work, as all you do is remove it. so we could d->transfers.remove(channelPtr).. but I'm happy it's not broken now.. so I don't really care.


> On July 10, 2012, 10:44 p.m., David Edmundson wrote:
> > lib/chat-widget.cpp, line 431
> > <http://git.reviewboard.kde.org/r/105511/diff/1/?file=72045#file72045line431>
> >
> >     ...is that true? What if it's a mimetype we can't decode?
> 
> Dan Vratil wrote:
>     You can either drop text/plain or text/html, which we display directly. You can drop raw image data, which we write to tmp file and send or you can drop _any_ file and we send it. 
>     
>     In general, it's impossible (and extremely unlikely anyone would do it anyway) to drop raw binary data (it would most probably be sent as text/plain anyway).
>     
>     ...but I can be wrong of course.

I meant what if I drag over ... lets say a contact from the contact list. It's neither text, image or file.. it's custom mimetype that is app specific, we don't want to say we can accept this drop.

We show that we can accept it, and then in the drop event we do nothing.


> On July 10, 2012, 10:44 p.m., David Edmundson wrote:
> > lib/chat-widget.cpp, line 141
> > <http://git.reviewboard.kde.org/r/105511/diff/1/?file=72045#file72045line141>
> >
> >     I'm not 100% sure why you need this.
> >     
> >     Surely the drop will propogate to the parent (this) anyway, and we can catch it in our own event() method which we already have?
> >     
> >     I may be wrong, ignore me if I am.
> >
> 
> Dan Vratil wrote:
>     Both QTextEdit and QWebView have their own drop event implementations, so I would have to reimplement the event handlers in both subclasses, but that would lead to code duplication (or at least complication). EventFilters receive events before the actual target object, so we handle it there and don't let it propagate to the target object.

but if you tell both of them to setAcceptDrops(false) then they won't use their own event handling (for drops)

then set ChatWidget to setAcceptDrops(true) and they will be passed here.
We can leave your dropEvent in place as-is, and just add a:

void ChatWidget::dragEnterEvent(QDragEnterEvent *event)
{
    event->accept();
}

(tested and it seems to be working)


> On July 10, 2012, 10:44 p.m., David Edmundson wrote:
> > lib/chat-widget.cpp, line 397
> > <http://git.reviewboard.kde.org/r/105511/diff/1/?file=72045#file72045line397>
> >
> >     I think (though could be wrong) this won't work very well.
> >     
> >     A good web browser will fill in both the text and HTML part of the mimedata and thus never trigger this.
> >     
> >     If it did.. the message you send will be full of HTML code, which would look pretty bad to the user the other side.
> >     
> >     (it wouldn't render like HTML they'd see all the "<h1>" type tags.)
> >     
> >     *shrug* after merging we can experiment like crazy and find out what doesn't work.
> 
> Dan Vratil wrote:
>     QTextEdit::insertHtml() \o/

I can't find a way to trigger this (dragging from kmail, chromium and rekonq) because they all set text with it.

I guess at that point I don't really care what goes here.. but is there even a need for it? 


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105511/#review15645
-----------------------------------------------------------


On July 10, 2012, 9:32 p.m., Dan Vratil wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105511/
> -----------------------------------------------------------
> 
> (Updated July 10, 2012, 9:32 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> - dropping a local URL starts a file transfer
> - dropping a remote URL appends it to the SendMessageBox
> - dropping text or HTML data appends it to the SendMessageBox
> - dropping image data creates a temporary file (PNG), starts a file transfer and when it's finished the file is removed again
> 
> 
> This addresses bug 288561.
>     http://bugs.kde.org/show_bug.cgi?id=288561
> 
> 
> Diffs
> -----
> 
>   app/main.cpp b8b2523 
>   lib/chat-text-edit.h 3e2dec0 
>   lib/chat-text-edit.cpp 2985aee 
>   lib/chat-widget.h 686b632 
>   lib/chat-widget.cpp e3439d8 
> 
> Diff: http://git.reviewboard.kde.org/r/105511/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Vratil
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20120711/0a321386/attachment-0001.html>