Review Request: Allow KUrlRequest file dialog to be non-modal

[Darío Andrés]

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


I have been thinking that the signals (dis)connection on modality change could not be needed... I should check if even in modal behavior (and using exec()), the signal "finished" is emitted from the dialog. If that works I could remove the updateMyFileDialogModality() function as this wont be needed. (and the code could be adapted)

About the naming of the functions I agree (I'm bad with names)

Also, David Faure pointed out that he added the comments about KDirSelectDialog in the apidox recently; so we could remove that and use the KFileDialog directory mode to implement the directory selection in a non-modal behavior. (this may need more discussion)

Thanks for the review

- Darío


On 2009-09-27 21:43:16, Darío Andrés wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1716/
> -----------------------------------------------------------
> 
> (Updated 2009-09-27 21:43:16)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> The current KUrlRequester behavior is to use modal dialogs (for both selecting files and directories)
> This is sometimes not desirable as it causes blocks like the one described in bug 162616
> 
> This patch introduces two public functions: setModalFileDialog and modalFileDialog
> 
> The default behavior is to use modal=true (legacy)
> 
> I have adapted the code the set the modality of the file dialog and to use (or not) show() + signals and slots instead of exec()
> 
> - Known problem (fix later):
> 
> When KUrlRequester is set to select Directories, the static function "KFileDialog::getExistingDirectory" is used; so I tried different approaches to replace it:
> - Setting the KFileDialog to the directory mode will have a different GUI ("KFileDialog::getExistingDirectory" uses KDirSelectDialog; when KFileDialog with a directory mode uses a simple file dialog but hiding the file listing....). Using this method is going to change the "behavior" as another kind of dialog is going to be used. Also, the apidox of the class states that in Directory mode the KDirSelectDialog is used; and I think we shouldn't change this for compatibility reasons.
> - Using KDirSelectDialog + show() + signals and slots (I wasn't really sure about this; the first time I tried it, the linker complained; and I didn't wanted to add another link library and mess with KIO/KFile dependencies)
> 
> gkiakia commented that KFileDialog with a Directory mode having a different GUI than KDirSelectDialog could be a bug/inconsistency; but we don't know the code that much in order to try to fix it.
> 
> As I didn't found a better implementation I left the modal dialog for directories. (It should probably be commented in the apidox...)
> 
> 
> This addresses bug 162616.
>     https://bugs.kde.org/show_bug.cgi?id=162616
> 
> 
> Diffs
> -----
> 
>   svn://anonsvn.kde.org/home/kde/trunk/KDE/kdelibs/kio/kfile/kurlrequester.h 1028323 
>   svn://anonsvn.kde.org/home/kde/trunk/KDE/kdelibs/kio/kfile/kurlrequester.cpp 1028323 
> 
> Diff: http://reviewboard.kde.org/r/1716/diff
> 
> 
> Testing
> -------
> 
> Now the Picture Frame file selector do not block Plasma anymore :)
> Further testing would be needed
> 
> 
> Thanks,
> 
> Darío
> 
>