Created an attachment (id=12823) [details]
Enhanced folder selection dialog

Created an attachment (id=12824) [details]
Patch to implement enhanced folder selection feature

Applied and tested against KMail 3.4.0 and 3.4.2

Heya Kumaran, I've looked at the patch and it seems like a good idea.
Unfortunately it doesn't apply against 3.5 (and therefor trunk) anymore. Do you
have time to port it to the changes in the selector dialog that happened since
3.5, maybe? Since 3.5 is feature frozen, this would go into 4.0, so we are in
no real hurry. Distributions might pick it up for their 3.x based packages, so
it's definitely worth porting. Please tell me if you don't have time, then we
can try to find someone on the lists who might be willing to take this on.
Thanks a lot for your contribution, and please excuse our lack of
responsiveness, the team is way to small. :)

Created an attachment (id=14312) [details]
Patch for version 3.5.0

Hi Till,

I've attached a patch which applies cleanly against 3.5.0.
The main functionality seems to work as expected.  I would
appreciate if the team could take a look and report any issues.

One difference from 3.4.2 is that the inactive rows are no
longer grayed out.  Please compare with the previous screenshot
attachment.  I wanted to resolve this issue, but unfortunately
ran out of time before tracking it down.  Perhaps someone on
the team would be willing to fix it?

Best Regards,
Kumaran

On Friday 20 January 2006 06:31, Kumaran Santhanam wrote:

> I've attached a patch which applies cleanly against 3.5.0.
> The main functionality seems to work as expected.  I would
> appreciate if the team could take a look and report any issues.


Ok, I've applied it locally and will be testing it a bit over the next 
few days. I use 'c' and 'm' all day, so I'm sure I'll notice if 
problems come up. One thing I noticed, the old version could remember 
the last used folder in between invocations, which I found useful. 
Should not be hard to add that back in, right? Also, the initial width 
of the first column was very small, leading to an odd-looking initial 
dialog.

I hope that you've had a chance to use the feature over the last month.
Would it be possible to get it committed into the main trunk?  It might
help to avoid another round of porting in case the trunk becomes
unpatchable again.  Please feel free to let me know if I can be of
assistance.

Yes, and I rather like it. I do think it would be nice to retain those features
of our current folder selector like remembering the last used folder, etc. I'll
have a look at the code later today, haven't looked at that at all yet, and
then give you some more feedback, hopefully.

Is there any possibility of getting this feature into a point release like
3.5.2?

I would like to see it go in and will discuss it with the others. We (KDE) are
currently debating lifting the feature freeze a bit anyhow, for 3.5.3. 3.5.2
was just tagged yesterday, but was bugfixes only anyhow. I've fixed the
remembering of the last used folder bug, btw, caused by the initial filter ("")
automatically selecting the root.

Thanks for the advocacy and filter fix.

There is one other minor feature that I just remembered:

When the user types a search string and it displays the matched folders,
the arrow keys only allow selection of matched folders and not
unmatched parents.  When I implemented quick filing in 3.4, I was able
to gray out those non-selectable parent folders.  For some reason, the
same code did not gray them out in the 3.5 patch.

This is just a cosmetic issue, since the search results are fairly
obvious.  However, in the case of deep folder hierarchies, it might
make things more clear.

Do you have any ideas on how to implement this in 3.5?

I've posted a request for inclusion in 3.5.3 to core-devel. We've slightly
lifted the feature freeze, since 4.0 is so far off.

i'm for inclusion :D
seems like it complies with requirements.

SVN commit 536268 by tilladam:

Tiny feature that is part of the folder quick filing patch: Make
ctrl-j open the folder selector (with quick search) to jump to
a folder. I'll commit the rest of the patch in a bit.

Patch by Kumaran Santhanam.

CCMAIL: 113759@bugs.kde.org


 M  +17 -0     kmmainwidget.cpp  
 M  +1 -0      kmmainwidget.h  


--- branches/KDE/3.5/kdepim/kmail/kmmainwidget.cpp #536267:536268
@@ -580,10 +580,15 @@
   new KAction( i18n("Copy Message to Folder"), Key_C, this,
                SLOT(slotCopyMsg()), actionCollection(),
                "copy_message_to_folder" );
+  new KAction( i18n("Jump to Folder"), Key_J, this,
+               SLOT(slotJumpToFolder()), actionCollection(),
+               "jump_to_folder" );
   mAccel->connectItem(mAccel->insertItem(Key_M),
 		     this, SLOT(slotMoveMsg()) );
   mAccel->connectItem(mAccel->insertItem(Key_C),
 		     this, SLOT(slotCopyMsg()) );
+  mAccel->connectItem(mAccel->insertItem(Key_J),
+		     this, SLOT(slotJumpToFolder()) );
 
   // create list of folders
   mFolderTree = new KMFolderTree(this, folderParent, "folderTree");
@@ -1483,6 +1488,18 @@
 }
 
 //-----------------------------------------------------------------------------
+void KMMainWidget::slotJumpToFolder()
+{
+  KMail::KMFolderSelDlg dlg( this, i18n("Jump to Folder"), true );
+  KMFolder* dest;
+
+  if (!dlg.exec()) return;
+  if (!(dest = dlg.folder())) return;
+
+  slotSelectFolder( dest );
+}
+
+//-----------------------------------------------------------------------------
 void KMMainWidget::slotMoveMsg()
 {
   KMail::KMFolderSelDlg dlg( this, i18n("Move Message to Folder"), true );
--- branches/KDE/3.5/kdepim/kmail/kmmainwidget.h #536267:536268
@@ -282,6 +282,7 @@
   void slotSaveMsg();
   void slotOpenMsg();
   void slotSaveAttachments();
+  void slotJumpToFolder();
   void slotMoveMsg();
   //void slotMoveMsgToFolder( KMFolder *dest);
   void slotCopyMsgToFolder( KMFolder *dest);

SVN commit 536272 by tilladam:

Patch by Kumaran Santhanam implementing folder quick filing as described in
http://bugs.kde.org/show_bug.cgi?id=113759. Ok'd on core-devel, reviewed,
tested, etc, as per the "lifted feature freeze" policy.

Thanks for the patch, mate.

BUG: 113759


 M  +192 -0    kmfolderseldlg.cpp  
 M  +7 -0      kmfolderseldlg.h  

I'm sorry, but I'd like to see this patch removed again. Adding a second column
to a dialog which is supposed to be a simple folder selection dialog is IMO not
an option. Since the second column basically shows the same information as the
first column there's obviously one column too much. Add to this that the second
column takes basically all space which makes it necessary to make the folder
selection dialog unreasonably wide (and which, on first invocation after
updating, made me go "wtf happened to the dialog. where's the folder tree?").
We got many reports from users who thought the folder tree had vanished when we
introduced the Unread/Read columns (which also took up basically all space).
Moreover, the column layout is not saved/restored, so that the next time I move
a message the new column again takes up all space.

Since a second column is not an option I suggest one of the following options:
- Make the quick filing work with the original layout, i.e. without adding the
second column. Or, if really necessary, add the second column, but make it a
hidden column.
- Remove the first column. This would make it possible to get rid of all
folders which don't match the currently entered pattern even the parents of
matching folders. (But such a change can't be done in a bug fix release.)

Since KDE 3.5.3 is supposed to be tagged in 9 days and QListView unfortunately
doesn't provide an easy way to hide the second column and adding the current
pattern to the first column header would violate the message freeze, I have to
revert the patch for now. I don't take this decision easily, but I have no
other choice.

SVN commit 540881 by kloecker:

Revert the last change for now.
CCBUG: 113759

 M  +0 -192    kmfolderseldlg.cpp  
 M  +0 -7      kmfolderseldlg.h  

There is a very specific reason for the second column.  It was not
added frivolously to show redundant information.  Rather, it shows
the entire path to that folder in one line.  This is very important
while performing searches on deep hierarchies.

Take the following set of folders:

Parent              Parent
  Apples            Parent/Apples
    Oranges         Parent/Apples/Oranges
    Pears           Parent/Apples/Pears

Let's assume that the sub-folders of Parent are so numerous that the
entire hierarchy is not visible on a single page.  When the user types
'apple', it can be immediately seen why 'Pears' is visible.  Furthermore,
if the user types 'apples/oranges', those matched sub-folders will be
shown.  Without the second column, it is easy to get lost within the
hierarchy.

I would like to submit a comment describing the history of this patch
and some concerns that I have over the process of its evaluation.

This patch was initially developed last summer, almost one year ago.
Initially, the dialog did not have a second column, but I found that it
became necessary as folder hierarchies became more complex.  As such,
the column was added before the patch was submitted to the KDE bug
database.

I deployed the patch throughout our organization and it has been tested
very well by our users.  I waited until everything was stable before
submitting the patch into the KDE bug database.  As such, by the time
it was submitted, it was already indispensable to those of us who have
been using it every day.

I understand the resource constraints of the KDE team and was more than
happy to port the patch to the latest version in the hopes that it would
be included in the upcoming point release.  The port took some time, but
I was willing to help move the project along.  Many thanks to Till Adam
for getting it reviewed and committed into the source tree.

I am concerned that comment #15 was made at the last minute with only
a cursory evaluation, resulting in a reversion of the committed patch.
This patch has been in the KDE bug database since Oct 2005.  If the issues
mentioned in the comment were so critical, would it not have been better
to address them at an earlier date?

Here is a compromise which I hope would be acceptable for 3.5.3:

- In normal operation, the dialog has only one column with the second
  column being hidden from view.

- If the user types anything, the second column appears to show the
  full folder path and search terms.

- If the typed search terms are deleted using backspace, the second
  column once again disappears from view.

If this sounds good, please let me know as soon as possible so that I
have time to implement it before the tagging deadline.  I will not
be available next week, so it would be best to resolve everything by
this Friday if possible.

As I said I'm sorry, but apart from the second column problem there was also
the other problem I mentioned, namely that the very first time the dialog with
two columns is invoked the second column will take all space. Of course, this
is only an issue the first time the dialog is invoked because most users would
resize the dialog to make the first column visible again. But nevertheless this
small problem shows that the patch was not well enough tested. The first
impression is important and therefore all new features (in particular if they
involve resizable dialogs) have to be tested with new users and with upgrading
users.

I fully understand why the second column was added, but it's the wrong solution
for the problem. Two columns representing the same information is just wrong.
If you reread my first comment you'll see that I proposed as one option to get
rid of the first column and only show the second column. Of course, we'd lose
the collapseblity of subfolder hierachies, but I wonder whether this is
actually needed in a folder selection dialog whose only purpose is to allow the
user to select a folder as easily and quickly as possible.

Yes, it would have been better to address the issues at an earlier date.
Unfortunately, the last 6-12 months I had almost no time to do any KDE-related
work. Hopefully this will change in the future. If I would have continued to
have no time for another two weeks then your patch would have been in KDE
3.5.3. But, two days ago I finally had time again after month to compile the
recent svn-branch-version of KMail. The first time I invoked the "Move Message"
dialog I was shocked (as I already wrote above). If the dialog would have been
opened with a reasonable default width I would probably have accepted the
change. But that was not the case.

I propose the following compromise for 3.5.3:
- Get rid of the first column.
- Use the title of the first column, i.e. "Folder" as title of the second
column (instead of "Path").
- Make sure the dialog is started with a reasonable default width, i.e. all
paths of depth three should be completely visible, but the dialog must not be
wider (by default) than 3/4 of the screen width.
- I'm a bit concerned about deep folder hierachies with long folder names
because this would require the dialog to be very wide. Maybe path which don't
fit into the current dialog width should be center-squeezed (?). See
KStringHandler::cPixelSqueeze()
(http://developer.kde.org/documentation/library/3.5-api/kdelibs-apidocs/kdecore/html/classKStringHandler.html#e19).

If you provide a patch implementing the first three points then I'll test it
and consider re-adding your patch. Unfortunately, the time frame for inclusion
in KDE 3.5.3 is rather short. I'd prefer to aim for inclusion in KDE 3.5.4
instead (with path squeezing).

BTW, looking at the patch I found the following word puzzle:
i18n("Path") + "  ( " + filter + " )"
Read
http://developer.kde.org/documentation/library/kdeqt/kde3arch/kde-i18n-howto.html#SECTION00036000000000000000
if you want to learn how to do it correctly. (But for 3.5.3 you should leave it
as it is except for replacing "Path" with "Folder".)

Thanks for the feedback.

I spent a significant amount time thinking about your suggestion
from both a user interface and coding perspective.  I also had a
chance to talk to other users about this to get a better sense of
the overall user experience.

I respectfully disagree with the proposal to eliminate the first
column.  The user feedback that I received indicates that the first
column provides a handy visual hierarchy reference.  The second
column is necessary to elucidate the matching algorithm.  They do
not show exactly the same information, but rather different
perspectives.  All of the users have been using the feature for
almost a year and thus have considerable experience with it.

I also wanted to mention that it seems a bit unfair to point at the
resize issue and make a claim that the code is not well-tested.  If
I had encountered this issue, I would have fixed it before
submitting the patch.  As it stands, we have found no problems with
the code after nearly a year of use.  The resize issue that you
encountered may be specific to a particular new or upgraded
configuration.  With that said, I'm more than happy to track down
and correct the issue that you experienced.

Removing the first column is a drastic change that could be met with
resistance from KDE users.  Furthermore, it requires quite a bit of
code surgery, due to the KFolderTree widget.  It would seem that the
safest approach is to resolve the resize issue and implement the
patch as-is, leaving the final decision to the KDE users.

As I had mentioned before, the patch has been in the database since
Oct 2005.  During that time period, since it had not been submitted,
I had to spend a significant amount of time upgrading it to the new
version of KMail.  The longer it stays out of the codebase, the more
maintenance it requires for new versions.

Given that user feedback has been good and the dialog is only
slightly altered, I ask that you consider accepting a version with
the resize issue resolved.  Then, it will be much easier to glean
ideas from the entire user base, resulting in more improvements
moving forward.

Created an attachment (id=16210) [details]
Patch for version 3.5.2

Attached is a patch implementing the folder selection dialog with
the requested column width preservation functionality.	This patch
is against version 3.5.2.  There is a slightly different patch for
the version in SVN.

Created an attachment (id=16211) [details]
Patch for version svn-20060521

This is the same patch in attachment 16210 [details] made against the
current SVN tree as of 2006-05-21.

SVN commit 544826 by kloecker:

Re-add the folder quick filing patch now that the dialog size problems have been resolved. Patch by Kumaran Santhanam.
CCBUG: 113759

 M  +5 -0      kmail.upd  
 M  +214 -2    kmfolderseldlg.cpp  
 M  +11 -1     kmfolderseldlg.h  
 M  +2 -1      kmstartup.cpp  

Unfortunately, I didn't see your new patch until now because the bug report
wasn't cc'ed to kmail-devel. I'm sorry that your patch won't be in KDE 3.5.3,
but it will be in KDE 3.5.4. Additionally to your patch I've made sure that the
old stored value of "Size" is removed for upgraders, so that they also get the
new wider default width.

Till just encountered a crash in the folder selection dialog after typing
something in, selecting and scrolling with the mouse wheel. We currently can
reproduce it reliably. Did your users also encounter a crash in the dialog?

If one adds and removes letters (i.e. first I enter "spa", then I press
Backspace, then I type an "a" again) then more than one folder can be selected.
Could you please look into this problem?

Comment 25:
We've never encountered a crash, and I have had no success in reproducing
the problem.  I'm using Fedora Core 4 with the packaged RPMs.  Can you
please give me some more steps to reproduce?

Comment 26:
This has not been reported before and I am unable to reproduce the problem.
It always seems to select one folder on my machine.  Perhaps the behavior
depends on a particular set of folders coupled with the search pattern.
Can you give me some more details about your setup?

Comment 25: Only Till could reproduce the crash.

Comment 26: The following works for me: Folder hierarchy contains only the standard folders plus Local Folders/test/special and Local Folders/test/x-spam. Now select a message and press 'm'. Click with the mouse on the first line (Local Folders), type "spa" (and note that x-spam is selected, but Local Folders is still the current item), now press Backspace. Result: special and x-spam are both selected. If you press Backspace again, then inbox, special and x-spam are selected.
The problem does not occur if one selects the first line with the keyboard (e.g. by pressing the Home key). So there seems to be a difference between selection by mouse and selection by keyboard.