luci-base: fix sort order for size columns

[raenye]

Currently elements of package size, packet size, etc. columns are interpreted as text strings, so we have "800 B" > "7.54 KiB" > "5.11 MiB". To fix this, a new regexp is added to UITable.deriveSortKey.

It is still not a perfect fix for System->Software->Installed, since pagination is done based on package names, so within each page the packages are sorted correctly, but the smallest (resp., largest) package is not necessarily on the first (resp., last) page.

Closes: #5994 (see also #6120)

[raenye]

@jow- Would you please take a look?

[jow-]

I think a better solution is adding a data-value attribute to the affected cells which contains the raw, unformatted byte value.

[raenye]

Maybe this should be the only value, and let css magic do the text formatting for rendering?

[jow-]

CSS magic can't do that as far as I know.

[systemcrash]

I think a better solution is adding a data-value attribute to the affected cells which contains the raw, unformatted byte value.

I came to suggest this. Exclude any units from the numeric value, and append those at a later stage, so sorting is done on raw numbers.

[systemcrash]

This looks related to #4087

[knarrff]

This looks related to #4087

It is - plus as #4087 correctly points out, 'k' 'M' and 'G' are for 10^3, 10^6 and 10^9. However, the idea of the report is still an important one: columns should sort by real number, and even across pages.

[raenye]

@systemcrash my JS skills are not sufficient to make the recommended changes. Should I close the PR?

[systemcrash]

Have a crack at it. This is a great learning opportunity.

[jow-]

Are there currently any tables left requiring this change? While I agree that it might be nice to smartly detect formatted size values and sort them accordingly, the proper fix should be adjusting affected tables to contain a data-value attribute. This can be achieved by changing the existing value expression to a two-element array where the first element should be the raw numeric value and the second value the formatted display text.

See for example commit 4f4ad9b which fixed some tables shown in the nlbwmon application. The underlying capability to the UITable class has been added in 90a2b1e

[systemcrash]

Does the sorting logic then depend in the browser?

[jow-]

Does the sorting logic then depend in the browser?

I don't understand that question. Why should it depend on the browser?

[systemcrash]

Because the data is at the client, and the client is the users web browser. So column sorting is performed by the browser. Or?

[jow-]

Uhm, yes but the current sorting is done by the browser as well?

[knarrff]

Because the data is at the client, and the client is the users web browser. So column sorting is performed by the browser. Or?

It should not be done at the browser, because at most the data of the current page is available to the browser (edit: I was wrong here) and sorting should span pages.

[jow-]

It should not be done at the browser, because at most the data of the current page is available to the browser and sorting should span pages.

There is no server side rendering in LuCI anymore, all data is rendered locally in the browser by building the table DOM from a multi dimensional array (or, in some cases by reading JS generated HTML table DOM and translating it into an internally table representation which is then eventually re-rendered) so sorting happens in the browser as well, naturally.

There's also no paging for the most part. The only paging I can think of is the software package list in the opkg menu, but that is rendered browser-side as well, it actually parses the raw package metadata list locally using JavaScript and dynamically builds the tables and paging logic

[systemcrash]

So pagination of data should also happen at the browser.

[jow-]

So pagination of data should also happen at the browser.

It is, there is no single server side rendered/paged table view in LuCI within this repository I am aware of.

The only exception should be the opkg package list display, but neither this PR nor the existing quick column sort logic will fix sorting across it's paged views. Solving that would require reworking the entire opkg view (or building paging logic right into the UITable class here: https://github.com/openwrt/luci/blob/master/modules/luci-base/htdocs/luci-static/resources/ui.js#L3179)

[knarrff]

The only exception should be the opkg package list display

Looking at that page (/cgi-bin/luci/admin/system/opkg ?), also this seems to get all data at once (an almost 5 MB data chunk), which is then locally paginated. So, it seems what @jow- suggests should be possible, including sorting across pages (which currently does not work).

[systemcrash]

@knarrff was opkg page your only concern here?

[knarrff]

I did not have a particular page in mind, but assumed that pagination happens because it would reduce bandwidth usage. That pagination is not done server-side should make sorting a whole lot easier.

[systemcrash]

@systemcrash my JS skills are not sufficient to make the recommended changes. Should I close the PR?

Would you like to try and add the necessary fields using this PR that @jow- highlighted earlier here?

[raenye]

I'll try this, thanks.

[systemcrash]

@raenye Do you have an update?

[systemcrash]

See @jow-'s latest e7f2e6b

[raenye]

See @jow-'s latest e7f2e6b

Thanks, but 63e5d38 already did that part for opkg.js.
Nevertheless, it doesn't solve the pagination issue, just the per-page sorting.

In any case, this PR is no longer relevant, so I'm closing this. If I find the time to fix pagination, I'll create a new PR.