Review Request 121678: Dolphin: port from KonqOperations::doDrop to the new KIO::DropJob

[David Faure]

> On Dec. 29, 2014, 8:46 p.m., Emmanuel Pescosta wrote:
> > dolphin/src/panels/folders/folderspanel.cpp, line 243
> > <https://git.reviewboard.kde.org/r/121678/diff/1/?file=335850#file335850line243>
> >
> >     Please use a lambda function which emits errorMessage on job errors. We should keep the error handling in my opinion. (same for all other error handling comments)
> 
> David Faure wrote:
>     Ah, I see. Misunderstanding. There were two error cases. Immediate errors (QString error was set) were handled here (but this case no longer happens so I removed it).
>     The error in the async operation was handled by KonqOperations itself (by popping up a message box, which is against dolphin's best practices). Now that it's a proper kio job, we can either enable autoerrorhandling (and get a messagebox again) or do the error handling the dolphin way. I didn't "remove" the latter, but I can add it indeed, with a bit of guidance :)
>     
>     Can you tell me if we could factorize these calls to dropUrls to have less places where to do error handling? I already removed the call in DolphinTabWidget by delegating to DolphinViewContainer, which is the one with proper error handling.
>     What about DolphinView::slotPasteJobResult? It's missing error handling --- but it takes care of selecting the newly created files, which we might want in all other cases too?
>     Is there any way to merge the dnd handling in DolphinView and DolphinViewContainer? I don't really know the dolphin design, so I'm not sure what the difference is.
>     
>     For folderspanel and placespanel they should emit errorMessage, OK. Should they select the newly created files too? I guess not (different view), but better ask to be sure ;)
> 
> Emmanuel Pescosta wrote:
>     > DolphinView::slotPasteJobResult?
>     
>     DolphinView::slotItemCreated already takes care of selecting newly created items.
>     
>     > which we might want in all other cases too?
>     
>     Yes good idea!
>     
>     > Is there any way to merge the dnd handling in DolphinView and DolphinViewContainer?
>     
>     We can add DolphinView::dropUrls and call it in DolphinViewContainer::dropUrls and in DolphinView::slotItemDropEvent. What do you think?

Excellent idea, that's exactly the kind of suggestion I was looking for. Done now.


- David


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


On Jan. 2, 2015, 8:36 a.m., David Faure wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121678/
> -----------------------------------------------------------
> 
> (Updated Jan. 2, 2015, 8:36 a.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> -------
> 
> REVIEW: 121678
> 
> 
> Diffs
> -----
> 
>   dolphin/src/dolphintabwidget.cpp a0c9b9d8198269bcf9c560106332cdf0dd9b45a7 
>   dolphin/src/dolphinviewcontainer.h bd4141db5a2d56e6a605247eaba4da3291664645 
>   dolphin/src/dolphinviewcontainer.cpp 3954a12e6638c2abc7ab57c669e1dc1680b073a6 
>   dolphin/src/panels/folders/folderspanel.cpp f56f5a1391831666555ac31c359ac02ba794998a 
>   dolphin/src/panels/places/placespanel.cpp b04191f1c73fa42b8e9d1f18f698c2e6a4a4ee02 
>   dolphin/src/views/dolphinview.h c054c311a360971d29ea796070dec9f3924ab1d9 
>   dolphin/src/views/dolphinview.cpp cb25c6555ab2a4f3af67a762262f7868aaab687a 
>   dolphin/src/views/draganddrophelper.h c4ae974b50ab5e0af5eb9dc749d383a8bdadb2fa 
>   dolphin/src/views/draganddrophelper.cpp a09faa345179fbb016b67289c550650a56639614 
> 
> Diff: https://git.reviewboard.kde.org/r/121678/diff/
> 
> 
> Testing
> -------
> 
> Dropping between the views of a splitted dolphin mainwindow. Without modifiers, and with Ctrl, Shift, Ctrl+Shift.
> 
> Dropping onto a breadcrumb of the url navigator.
> 
> Dropping onto a tab.
> 
> 
> Thanks,
> 
> David Faure
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20150102/5a6d3447/attachment.htm>