[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: Review Request: Allow KUrlRequest file dialog to be non-modal
From:       Darío Andrés <andresbajotierra () gmail ! com>
Date:       2009-09-28 14:02:01
Message-ID: 20090928140201.10101.57313 () localhost
[Download RAW message or body]


-----------------------------------------------------------
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
> 
> 


[prev in list] [next in list] [prev in thread] [next in thread]