Hi all,
here is the next revision of threaded message list patch
Changes comparing to roundcubemail-threading-20090706.patch from http://www.atomice.com/blog/?page_id=34:
- Ported to 0.3-SVN-r2976
- Implemented auto expand of threads with unread messages only.
- Added GUI configuration option for autoexpand control (on, off, unread).
- Autoexpansion works in both static and JS message lists.
- Automatically change between 'unread children' and message icons in a
list (in parent message rows) when child message is marked read/unread. 6. Count messages, not threads in a message view pane. 7. Fixed navigation in a message view pane when threading is enabled. 8. Fixed setting folders with non-ASCII named to threaded mode. 9. Fixed incorrectly displayed rows (children of collapsed row) after multi-level expand/collapse. Simplified expand logic. 10. Fixed expand indicator is not changing when expanding/collapsing with keyboard. 11. Fixed plus key on a numeric keypad is not working (at least on linux Fedora 11 + Firefox 3.5.2). 12. Added support for whole thread expand/collapse with clicking/pressing hotkeys while holding the Control key. 13. Save expand state in env everywhere (after keypresses too). 14. Added russian translation. 15. Fixed indentation and braces style in JS to comply with RC coding style.
Changes to my previous public revision:
- (Multiple) selection works fine with threaded view too now (based on
patch published by Nathan Kinkade). Selection list is altered immediately on a select action (opposed to Nathan's patch). Nested messages are added to selection list when collapsed parent is selected, and removed from it on a thread expand. I ask everybody to test this feature with both mouse and keyboard shortcuts. 17. Remove a bit of unused code. 18. Add only one div of a variable width to a messagelist row instead of set of divs. See comments inline. 19. Make dragging work with threaded view (locate subject text differently). 20. Re-read messagelist portion after move/delete operation if threading is enabled. See comments inline. (This does not fix a bug with non-atomic move/delete operations I noted before). I'm not sure I did it 100% right because of that bug which makes testing nearly impossible in my setup.
I think that code (except thread caching which I don't use and didn't touch it at all, it is up to Chris) is a true beta quality now (based on my testing) and is ready to be tested intensively in different setups. Comments are welcome as usual.
Best, Vladislav
--- 8< --- detachments --- 8< --- The following attachments have been detached and are available for viewing. http://detached.gigo.com/rc/1h/WkvtBXFZ/roundcubemail-thread.patch http://detached.gigo.com/rc/1h/WkvtBXFZ/unread_children.png Only click these links if you trust the sender, as well as this message. --- 8< --- detachments --- 8< ---
List info: http://lists.roundcube.net/dev/
Vladislav Bogdanov wrote:
I think that code (except thread caching which I don't use and didn't touch it at all, it is up to Chris) is a true beta quality now (based on my testing) and is ready to be tested intensively in different setups. Comments are welcome as usual.
Great, I plan to start including this into trunk in October.
On Tue, 22 Sep 2009 12:11:02 +0300, Vladislav Bogdanov bubble@hoster-ok.com wrote:
Hi all,
here is the next revision of threaded message list patch
Changes comparing to roundcubemail-threading-20090706.patch from http://www.atomice.com/blog/?page_id=34:
<-- snip lots of changes -->
Looks good. Do you mind if I add your patch to my web page (with credit)?
I think that code (except thread caching which I don't use and didn't touch it at all, it is up to Chris) is a true beta quality now (based on my testing) and is ready to be tested intensively in different setups. Comments are welcome as usual.
I found thread caching (and also message caching) just slows things down if your IMAP server is running on the same machine as Roundcube, so I don't use it either. It may be better if you use SQLite instead of MySQL.
Chris
List info: http://lists.roundcube.net/dev/
Chris January wrote:
Looks good. Do you mind if I add your patch to my web page (with credit)?
Approved, I think it's a best place for it. I really hate code forking. And this leads to a wider testing.
Please don't forget to publish (originally yours) icon separately too ;) And please do not forget to credit Nathan Kinkade, that patch was very useful.
I found thread caching (and also message caching) just slows things down if your IMAP server is running on the same machine as Roundcube, so I don't use it either. It may be better if you use SQLite instead of MySQL.
It should be tested anyway if caching code exists in a trunk. Personally I do not use caching too, cyrus is quick enough even on a neighbor host.
Vladislav _______________________________________________ List info: http://lists.roundcube.net/dev/
Patch I sent originally mistakenly contains a bit of uncommented debug code I placed there while catching that 'non-atomic deletion' bug.
Resending with this code commented.
Thanks to Eric Appelt for noticing that,
Vladislav
--- 8< --- detachments --- 8< --- The following attachments have been detached and are available for viewing. http://detached.gigo.com/rc/NT/1MR2llPQ/roundcubemail-thread.patch http://detached.gigo.com/rc/NT/1MR2llPQ/unread_children.png Only click these links if you trust the sender, as well as this message. --- 8< --- detachments --- 8< ---
List info: http://lists.roundcube.net/dev/
On Tue, 22 Sep 2009 11:29:22 +0200, "A.L.E.C" alec@alec.pl wrote:
Vladislav Bogdanov wrote:
I think that code (except thread caching which I don't use and didn't touch it at all, it is up to Chris) is a true beta quality now (based
on
my testing) and is ready to be tested intensively in different setups. Comments are welcome as usual.
Great, I plan to start including this into trunk in October.
Is that still planned? I'm really excited about threaded view coming to Roundcube. _______________________________________________ List info: http://lists.roundcube.net/dev/
Jean-Baptiste Hétier wrote:
Great, I plan to start including this into trunk in October.
Is that still planned? I'm really excited about threaded view coming to Roundcube.
Yes, but I was too busy. I'll start probably in next week. Do you guys have any newer version of this patch?
A.L.E.C wrote:
Yes, but I was too busy. I'll start probably in next week. Do you guys have any newer version of this patch?
Not yet. I'm quite busy with other projects now, sorry. Everything except rcube_imap.php should should apply clear still (at least I hope). I tried to apply last patch week or two ago, and everything applied fine except that file. Anyway I think it would be better if you reorganize that code yourself, does anybody else know that algorithm with headers better in its current state? :)
Only few notes from me:
- We should probably combine threading with index_sort in a way that
index_sort is implicitly enabled when threading is on. This would simplify code a bit and make some hunks unnecessary (I mean a so-called 'default' sorting mode).
- We should give user a possibility to switch between threaded and
non-threaded view from a message_list itself. I already wrote that an ideal solution for me would be a tiny dropdown in the very left message_list header position with options for threading, index_sort and default sort (I mean sorting by columns) modes. One problem I see with this is a 'date' sorting which is probably a kinda special case, it should be enabled for all of three modes (FIXME), giving user an option to 'reverse-sort' the list. I hope somebody can propose a better solution for that 'reverse' listing. Hmmm.... I think all other sorting options should be disabled when either threading or index_sort are enabled, shouldn't they?
I see no problems with all the added JS code, and I'd say it is stable.
Only one quite big problem remains - that async deletion bug I wrote before. It would be really cool if you fix it in one shot with threading so we have finished threading solution.
Best, Vladislav _______________________________________________ List info: http://lists.roundcube.net/dev/
Vladislav Bogdanov wrote:
- We should probably combine threading with index_sort in a way that
index_sort is implicitly enabled when threading is on. This would simplify code a bit and make some hunks unnecessary (I mean a so-called 'default' sorting mode).
- We should give user a possibility to switch between threaded and
non-threaded view from a message_list itself. I already wrote that an ideal solution for me would be a tiny dropdown in the very left message_list header position with options for threading, index_sort and default sort (I mean sorting by columns) modes. One problem I see with this is a 'date' sorting which is probably a kinda special case, it should be enabled for all of three modes (FIXME), giving user an option to 'reverse-sort' the list. I hope somebody can propose a better solution for that 'reverse' listing. Hmmm.... I think all other sorting options should be disabled when either threading or index_sort are enabled, shouldn't they?
Yeah, something like that. I like to see a popup with coulmns, sorting and grouping selection:
Columns (checkboxes): [x] Subject [x] Date [x] From/To [x] Size [ ] Attachment [ ] Flag ... other columns here Sorting (radio buttons): (o) None (current index_sort) ( ) Date ( ) Arrival date (not implemented yet) ( ) Subject ( ) From/To ( ) Size Order (radio buttons): (o) Ascending ( ) Descending Grouping (radio buttons): (o) None ( ) Thread ( ) Group (or sth in future)
This could be a simple form or two-level menu. Any volunteers to implement this?
Hmmm.... I think all other sorting options should be disabled when either threading or index_sort are enabled, shouldn't they?
I think not. We can sort threads (root messages) by any field.
A.L.E.C wrote:
Yeah, something like that. I like to see a popup with coulmns, sorting and grouping selection:
Columns (checkboxes): [x] Subject [x] Date [x] From/To [x] Size [ ] Attachment [ ] Flag ... other columns here Sorting (radio buttons): (o) None (current index_sort) ( ) Date ( ) Arrival date (not implemented yet) ( ) Subject ( ) From/To ( ) Size Order (radio buttons): (o) Ascending ( ) Descending Grouping (radio buttons): (o) None ( ) Thread ( ) Group (or sth in future)
This could be inspired by a contextmenu plugin implementation. One more thought: There should be a 'Reset to default' option. And, this sorting and grouping state should be a per-folder one... So, may be one more option - 'Reset all to default' may be useful.
In general I like your idea even more then mine ;) It even adds missing control over what columns are displayed and adds overall consistency to a whole UI.
This could be a simple form or two-level menu. Any volunteers to implement this?
I prefer one-level. It is quicker to work with. And size of menu is almost constant, so I see no need for a second level. That second level is cool for a contextmenu, which has a (variable sized) list of folders to move message to, but not for a sorting control I think. Only 'Reset' commands could be moved to a second level, as they are not of a frequent use.
Something like this: Reset view -> This folder All folders
And one more note about possible implementation: clicks on columns checkboxes shouldn't hide menu, while click on sorting and grouping radios should.
Hmmm.... I think all other sorting options should be disabled when either threading or index_sort are enabled, shouldn't they?
I think not. We can sort threads (root messages) by any field.
Yep, you are right about threading. But what about index_sort? Should click on any header disable it and switch to a 'sorted' mode? Ah, yes, it should.
Best, Vladislav _______________________________________________ List info: http://lists.roundcube.net/dev/
Vladislav Bogdanov wrote:
This could be inspired by a contextmenu plugin implementation. One more thought: There should be a 'Reset to default' option. And, this sorting and grouping state should be a per-folder one... So, may be one more option - 'Reset all to default' may be useful. In general I like your idea even more then mine ;)
We don't need two-levels until the menu height will be not too high. Another step to make messages list complete will be columns order changing using drag'n'drop (preferred) or using up/down buttons in the menu.
And one more note about possible implementation: clicks on columns checkboxes shouldn't hide menu, while click on sorting and grouping radios should.
For sorting/grouping also shouldn't. I think we should add a "Save" link/button at bottom, also "Reset" and "Reset all" (the last one with user warning).
Hmmm.... I think all other sorting options should be disabled when either threading or index_sort are enabled, shouldn't they?
I think not. We can sort threads (root messages) by any field.
Yep, you are right about threading. But what about index_sort? Should click on any header disable it and switch to a 'sorted' mode? Ah, yes, it should.
Exectly. That's how it should working from the beginning, but we've got no such sorting menu.
A.L.E.C wrote:
We don't need two-levels until the menu height will be not too high. Another step to make messages list complete will be columns order changing using drag'n'drop (preferred) or using up/down buttons in the menu.
I'd prefer drag'n'drop in a message_list itself. This would be more intuitive. But checkboxes in menu should follow order in which columns appear in a message_list anyway. And I'd move this functionality into a separate commit after main features are mature. And, maybe I'd prefer to have this menu in a separate (of threading) commit also. Just not to mess things.
And one more note about possible implementation: clicks on columns checkboxes shouldn't hide menu, while click on sorting and grouping radios should.
For sorting/grouping also shouldn't. I think we should add a "Save" link/button at bottom, also "Reset" and "Reset all" (the last one with user warning).
+1
But I'd write 'Reset view' or 'Reset options' instead of 'Reset'. 'Reset' word alone may mean anything for an user... ;)
Vladislav
List info: http://lists.roundcube.net/dev/
Finally I've started to work on this. Just want to share an idea of performace improvement. Now, we're fetching all messages in all threads for current page. I think we should do this only when "Autoexpand/Expand All" is enabled. Otherwise we should fetch headers for roots only and fetch children headers on demand. Also the last operation could be improved with SEARCH=INTHREAD use.
On Fri, 06 Nov 2009 14:22:36 +0100, "A.L.E.C" alec@alec.pl wrote:
Finally I've started to work on this. Just want to share an idea of performace improvement. Now, we're fetching all messages in all threads for current page. I think we should do this only when "Autoexpand/Expand
All" is enabled. Otherwise we should fetch headers for roots only and fetch children headers on demand. Also the last operation could be improved with SEARCH=INTHREAD use.
It's a good idea. I think that displaying the number of messages in a thread is an important feature as well though. Could you do that without fetching all messages? _______________________________________________ List info: http://lists.roundcube.net/dev/
Jean-Baptiste Hétier wrote:
It's a good idea. I think that displaying the number of messages in a thread is an important feature as well though.
It's not important for me.
Could you do that without fetching all messages?
Of course, THREAD returns message ids for roots and its children, so number of messages in a thread is not a problem.
On Fri, 06 Nov 2009 14:53:18 +0100, "A.L.E.C" alec@alec.pl wrote:
Jean-Baptiste Hétier wrote:
Could you do that without fetching all messages?
Of course, THREAD returns message ids for roots and its children, so number of messages in a thread is not a problem.
Cool!
Great to read that you're working on that by the way! _______________________________________________ List info: http://lists.roundcube.net/dev/
Jean-Baptiste Hétier wrote:
Cool!
Also, what you think abut such feature.... I like to implement the whole thread expanding/collapsing with one click. I think we can do this like in Thunderbird. I don't like to click every child to expand the whole thread. The expand/collapse icon should be displayed only for a root message. Currently it's displayed for root and every child with children.
Hi all,
sorry for delay
A.L.E.C wrote:
Also, what you think abut such feature.... I like to implement the whole thread expanding/collapsing with one click. I think we can do this like
Ctrl-Left_Click on an expando icon already does this. This should be mentioned somewhere in a help.
in Thunderbird. I don't like to click every child to expand the whole thread. The expand/collapse icon should be displayed only for a root message. Currently it's displayed for root and every child with children.
I like current behavior much more than thunderbirds one.
-1
Vladislav _______________________________________________ List info: http://lists.roundcube.net/dev/
A.L.E.C wrote:
Finally I've started to work on this. Just want to share an idea of performace improvement. Now, we're fetching all messages in all threads for current page. I think we should do this only when "Autoexpand/Expand All" is enabled. Otherwise we should fetch headers for roots only and fetch children headers on demand. Also the last operation could be
+1
improved with SEARCH=INTHREAD use.
List info: http://lists.roundcube.net/dev/
On Fri, 06 Nov 2009 15:27:12 +0100, "A.L.E.C" alec@alec.pl wrote:
Jean-Baptiste Hétier wrote:
Cool!
Also, what you think abut such feature.... I like to implement the whole
thread expanding/collapsing with one click. I think we can do this like in Thunderbird. I don't like to click every child to expand the whole thread. The expand/collapse icon should be displayed only for a root message. Currently it's displayed for root and every child with
children.
I prefer the current behaviour (but then I would...) - if you have a large thread with a hundred messages in it (some mailing list threads can get this large) you don't necessarily want to expand the whole thing.
Chris
List info: http://lists.roundcube.net/dev/
-
A.L.E.C -
Chris January -
Jean-Baptiste Hétier -
Vladislav Bogdanov