Review Request 111091: Add "Always Ask" option for filetransfer destination directory

[Daniele E. Domenichelli]

> On June 20, 2013, 12:49 p.m., Daniele E. Domenichelli wrote:
> > Sorry I still have a couple of issues:
> > 
> > 1) Instead of reading and saving the lastDownloadDirectory entry you can use the kfiledialog:///XXXXXXX syntax for file dialog  (see ktp-contact-list/contact-list-widget.cpp:371). You can also use the static methods for KFileDialog, so that you don't need to add a pointer to the class.
> > 
> > 2) I don't like at all that if you enable the "Always Ask" flag you get first a dialog to choose the directory and then, if the file exists, a second dialog to rename the file. Imho, if you enable the flag you should get just one file chooser dialog, with a warning if you are overwriting an existing file, and you should skip the     checkFileExists() call and go straight to checkPartFile();
> 
> Dan Vrátil wrote:
>     I'm using KDirSelectDialog, which is slightly different from KFileDialog (it's not a subclass)
>     
>     ad 1) didn't know about that and it works with KDirSelectDialog, so it's fine
>     
>     ad 2) this can't be solved when using KDirSelectDialog. And using KFileDialog (from user's POV) is slightly less comfortable than KDirSelectDialog when it comes to just selecting a directory. Also this option is about picking a directory, not a filename. Otherwise we would have to show the filename dialog every time.
> 
> Thomas Pfeiffer wrote:
>     Why do you use KDirSelectDialog?
>     Using a normal KFileDialog makes perfect sense in this case, because users may indeed want to save the file under a different name, just like in a browser. From a user's point of view there is pretty much no difference between saving a file in a browser and saving a file received via KTp. It's not about choosing a folder, it's about choosing where and with which file name to save it. 
>     Therefore the suggestion from the usability perspective is to emulate the browser behavior (you may take Rekonq as an example) as closely as possible. It works for browers and it would work for KTp just as well.
>     And if it's currently not technically possible to save a file received via KTp under a different name, than KTp is broken and needs to be fixed.

I agree, it should not just let you choose the folder, but the whole path + file name, using by default the one received from the file-transfer (you can get it with channel->fileName())

I'm not sure if the kfiledialog:///XXXXXXX can handle this though, perhaps if you add the name after (something like kfiledialog:///XXXXXXX/filename.ext) it might save just the directory, you should try ;)


- Daniele E.


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


On June 20, 2013, 9:34 a.m., Dan Vrátil wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111091/
> -----------------------------------------------------------
> 
> (Updated June 20, 2013, 9:34 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> Implements support for "Always Ask" for filetransfer destination directory option.
> 
> The hack with __k__onDownloadDirectoryCancelled() is necessary because KDirSelectDialog::result() always returns 0 and buttonClicked() signal is emitted even when "New Folder" button is clicked and handling that would make the code more complicated.
> 
> 
> This addresses bug 295937.
>     http://bugs.kde.org/show_bug.cgi?id=295937
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt ebd892c 
>   src/filetransfer-handler.cpp 74c701e 
>   src/handle-incoming-file-transfer-channel-job.h d5e53c4 
>   src/handle-incoming-file-transfer-channel-job.cpp 7a15237 
> 
> Diff: http://git.reviewboard.kde.org/r/111091/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Vrátil
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20130621/ea378fc6/attachment.html>