[pve-devel] [PATCH manager] fix #6138: ui: storage content: use locale for sorting filenames

[Dominik Csapak]

On 3/4/25 14:05, Fiona Ebner wrote:
> Am 05.02.25 um 14:22 schrieb Dominik Csapak:
>> by using 'localeCompare'.
>>
>> Assume the files 'a', 'B' and 'c'. The current default sorting results
>> in :
>>
>> 'B'
>> 'a'
>> 'c'
>>
>> (C collation)
>>
>> With this patch we use locale dependent sorting, so the browser/client
>> setting controls how it's sorted. For example with english, the list
>> becomes:
>>
>> 'a'
>> 'B'
>> 'c'
>>
>> Which makes it easier to visually search for specific entries. Using the
>> default settings here makes sense (so no explicit 'sensitivity' setting
>> for example.), since that is whats most expected per locale.
>>
>> Interestingly, the initial sorter (via the 'sorters' property of the
>> store), cannot seem to reuse the sorter defined in the columns, so we
>> have to specify it twice, once for the initial sort and once when the
>> user clicks the column header.
>>
>> Previously the default sorter was by 'volid' which was sometimes not
>> even fully visible (i.e. 'local:100/vm-100...' becomes 'vm-100...')
>> which lead to confusing sorting results, as 'base' volumes were
>> interspersed with the normal vm volumes, but the list was sorted by vmid
>> alphabetically (not numerical).
>>
>> Now the result is sorted by what the user can see, which is IMO a better
>> experience.
>>
> 
> While we're at it, should/can we make sorting numerically work too?
> There is a 'numeric' option we could set, not sure if that's better or
> not here and if that is intended to work for mixed strings like
> soemthing9.yaml, something10.yaml or only for purely numeric ones.
> 

good catch, from my short testing, yes this seems to work as intended, e.g.
(copied & adapted from mdn):

console.log("foo2".localeCompare("foo10")); // 1
console.log("foo2".localeCompare("foo10", undefined, { numeric: true })); // -1

i think using numeric sort here is more "natural",
even many users already expect it to be sorted lexical i guess

any other opinions on that?

>> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> 
> Had to use utf8::decode() in the storage backend's get_subdir_files() to
> be able to test with a path with special character 'ä' (to compare
> Swedish and German ordering). Because the returned path from the API is
> garbled otherwise. But that is orthogonal to the fix here, so
> 
> Reviewed-by: Fiona Ebner <f.ebner at proxmox.com>
> Tested-by: Fiona Ebner <f.ebner at proxmox.com>
> 
>> ---
>>   www/manager6/storage/ContentView.js | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/www/manager6/storage/ContentView.js b/www/manager6/storage/ContentView.js
>> index daefaf4f..a40880de 100644
>> --- a/www/manager6/storage/ContentView.js
>> +++ b/www/manager6/storage/ContentView.js
>> @@ -43,10 +43,9 @@ Ext.define('PVE.storage.ContentView', {
>>   		    content: content,
>>   		},
>>   	    },
>> -	    sorters: {
>> -		property: 'volid',
>> -		direction: 'ASC',
>> -	    },
>> +	    sorters: [
>> +		(a, b) => a.data.text.toString().localeCompare(b.data.text.toString()),
>> +	    ],
>>   	});
>>   
>>   	if (!me.sm) {
>> @@ -153,6 +152,7 @@ Ext.define('PVE.storage.ContentView', {
>>   		flex: 2,
>>   		sortable: true,
>>   		renderer: PVE.Utils.render_storage_content,
>> +		sorter: (a, b) => a.data.text.toString().localeCompare(b.data.text.toString()),
>>   		dataIndex: 'text',
>>   	    },
>>   	    'notes': {
>