692751 - unifinder column sort order inconsistent behaviour

Status: NEW

(Calendar :: Calendar Frontend, defect)

Description

[Joel Rocha]

Attachment: Patch for correcting this behaviour

User Agent: Opera/9.80 (Windows NT 5.1; U; en) Presto/2.9.168 Version/11.51

Steps to reproduce:

Selected all columns on unifinder (one at a time)


Actual results:

Sort is inconsistent. The first column is sorted ascending the second is sorted descending the third is sorted descending, and so on and so on...


Expected results:

IMHO All the columns should be sorted ascending when selected for the first time. Sorted descending if they are "double" clicked.

Comment 1

[Stefan Sitter [:ssitter]]

Confirming behavior.

Comment 2

[Stefan Sitter [:ssitter]]

Comment on attachment 565484 [details] [diff] [review]
Patch for correcting this behaviour

Thanks for the patch. If you add a patch please mark it correctly and request review from a calendar developer.

Comment 3

[Joel Rocha]

Attachment: Improved routine for performance

the column only gets sorted once. Then it just gets reversed.

Comment 4

[Joel Rocha]

Attachment: Removed TODO comment line

Equal to the previous without the //TODO:

Comment 5

[Joel Rocha]

Attachment: Optimized for readability

Improved for readability, that meant adding a var and an if condition so that the flow of operations can be seen more linearly (and without repeated source code)

Comment 6

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 569621 [details] [diff] [review]
Optimized for readability

Review of attachment 569621 [details] [diff] [review]:
-----------------------------------------------------------------

Joel, does this patch already fix the whole bug? It looks to me as if something is missing? Maybe you could explain a bit more. Thanks!

Comment 7

[Joel Rocha]

Have you tested it? Does not solve the previous behaviour? My last post was made 3 months ago, i do not have the workspace ready to test this (i'm kinda working on something else rigth now, sorry...)

AFAIK, from the diff that can be requested here on the site, the following behaviour was implemented:

//"A priori" when the cycleHeader function is called the column has not been selected yet.
let colAlreadySelected = false;

//Then we try to find out if the column was already selected, if not, the direction of sort is ascending, if the column was already selected we will set colAlreadySelected = true, this will be helpful in the next step. We also have to sort it by the reverse order: if the previous order was descending the current order is ascending and vice versa.

//The next step is to check if the column was already selected, if true, the list is already sorted, we just have to reverse the current list, if not we have to sort it by the assigned column. This way, the sort is only run when the user selects a different column to sort, after that the operation takes much less time, making it more efficient.

Comment 8

[Joel Rocha]

Also note that (and maybe that's what you think it's missing) the only time the sort runs is when the order is ascending (in the first time).

Comment 9

[Joel Rocha]

Did i made myself clear? If not just post any questions, or contact me directly. Thank you!

Comment 10

[Joel Rocha]

Hello Martin, i see you changed status and assignee. The patch seems to be enough to resolve the issue. What else can i do for this to be approved?