Bugzilla – Full Text Bug Listing
| Summary: | 2nd level sorting in Search view | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | z_Archived | Reporter: | Eugene Kuleshov <ekuleshov> | ||||||||||||||||||||||||||||||||
| Component: | Mylyn | Assignee: | Frank Becker <eclipse> | ||||||||||||||||||||||||||||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||||||||||||||||||||||||||||
| Severity: | enhancement | ||||||||||||||||||||||||||||||||||
| Priority: | P2 | CC: | eclipse, mik.kersten, robert.elves, steffen.pingel | ||||||||||||||||||||||||||||||||
| Version: | unspecified | Keywords: | noteworthy, plan | ||||||||||||||||||||||||||||||||
| Target Milestone: | 3.2 | ||||||||||||||||||||||||||||||||||
| Hardware: | PC | ||||||||||||||||||||||||||||||||||
| OS: | All | ||||||||||||||||||||||||||||||||||
| Whiteboard: | |||||||||||||||||||||||||||||||||||
| Bug Depends on: | 231336 | ||||||||||||||||||||||||||||||||||
| Bug Blocks: | |||||||||||||||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||||||||||||||
|
Description
Eugene Kuleshov
Created attachment 89347 [details]
patch for Descending
When I look at this I found a problem.
If you use descending this was not supported for sortorder "Date Created" and "Priority".
If sorted by "Priority" the 2nd level of sorting is by "summary".
Should I start with the implementation of 2nd level of sorting.
Created attachment 89348 [details]
mylyn/context/zip
Created attachment 89358 [details]
Patch with 2nd level sorting
Please review!
Created attachment 89359 [details]
mylyn/context/zip
Frank: this bug is about sorting in the Search view, not Task List. However, it would be great if you could make a new bug for your additional Task List sorting, because there have been some requests for that as well. Regarding this approach, I have a bit of a usability concern with your approach though, because some might have trouble understanding the word "Level", and because doubly-nested menus are hard to use. What if the menu was? * First Sort by -> full list of sorters * Then Sort by -> full list of sorters Btw, if you're interested in providing a patch for this bug that would be great too. What we're after is that no matter what primary sort order is chosen in the Search view, there should be a second sort order that's by ID and description (with the same semantics as in the Task List). (In reply to comment #5) Sorry my mistake :-( I try to do both (create new bug and solve this on) but maybe this will be to late for 2.3. Sounds good Frank. Created attachment 90557 [details]
new Patch
Here the Patch for Search View
Created attachment 90558 [details]
mylyn/context/zip
Frank, you may want to add tests for TaskSearchResultSorter and TaskComparator classes to make sure that backend is working properly. Created attachment 90583 [details]
Patch with JUnit Test
Here the Version with a test.
Created attachment 90584 [details]
mylyn/context/zip
Frank I was seeing some failing tests the other day when reviewing (sorry for not posting soon), and this patch is now a little stale. Any chance you could look into the failures and recut this patch. Otherwise I think it's all good here aside from the need to set the title on the search dialog itself. Created attachment 95415 [details] updated patch (In reply to comment #13) >Otherwise I think it's all good here aside from the need to set the title on the search dialog itself. What do you think that should be done here? Created attachment 95416 [details]
mylyn/context/zip
Need to defer. The task list already has a configuration dialog and two-level sorting. Is there any chance that some of the code could be shared between the task list and search results view? That would be great. Also, as part of that we should make sure that the sorting and grouping setting in the Search view sticks. Created attachment 116103 [details] poc with patch for 231336 this is only to save the current state until bug#231336 is resolved Created attachment 116104 [details]
mylyn/context/zip
I'll move this for 3.2 for now. There are some fixes and refactorings I want to take a look at before we make further changes to the sorting implementation (bug 261998). Created attachment 130300 [details]
patch for 3.2
Created attachment 130301 [details]
mylyn/context/zip
Adding to plan since this is a relevant feature for 3.2. Created attachment 134922 [details]
updated patch
Thanks Frank! Based on your patch I have generalized the persistence of the sorting settings to a memento to get rid of some of the code duplication. I have committed the updated patch. Please take a look at my changes and let me know if that makes sense. (In reply to comment #26) > Thanks Frank! Based on your patch I have generalized the persistence of the > sorting settings to a memento to get rid of some of the code duplication. I > have committed the updated patch. Please take a look at my changes and let me > know if that makes sense. > Looks good! |