Re: [pve-devel] [PATCH manager v2 1/2] ui: resource tree: use 'diskuse' instead of calculating everytime

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Am 12.12.25 um 08:44 schrieb Dominik Csapak:
> the resource store has a field 'diskuse' which it calculates on update.
> Use that instead of calculating the value ourselves everytime.
> 
> For change detection, we only need a resolution of 0.01 (since we want
> to use the percentage as integer) so check that the difference of old and new
> is bigger than 0.9% .
> 
> This works because we only overwrite the values in the treestore if
> anything changed, so multiple small changes to the diskuse will not be
> lost.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> changes from v1:
> * improve commmit message to note why many small changes are not lost
> * use `>= 0.01` instead of `> 0.009`
> 
>  www/manager6/tree/ResourceTree.js | 27 ++++++++++++++++++---------
>  1 file changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/www/manager6/tree/ResourceTree.js b/www/manager6/tree/ResourceTree.js
> index bb016f8c..b0e094f1 100644
> --- a/www/manager6/tree/ResourceTree.js
> +++ b/www/manager6/tree/ResourceTree.js

> @@ -409,14 +407,25 @@ Ext.define('PVE.tree.ResourceTree', {
>                          }
>                      }
>  
> -                    // tree item has been updated
> -                    for (const field of changedFields) {
> -                        if (item.data[field] !== olditem.data[field]) {
> +                    let diskuse = item.data.diskuse;

While the API is hard to change, we could still use a bit more telling
variable names, like diskUsage and oldDiskUsage. Just a not, and if I would not
had noticed below tiny potential bug, I might have just fixed this up locally.

> +                    let oldDiskuse = olditem.data.diskuse;
> +
> +                    if (diskuse !== undefined || oldDiskuse !== undefined) {
> +                        if (Math.abs(diskuse - oldDiskuse) >= 0.01) {

this is never true vor the case where one is defined and the other
value is undefined though, as the result then is NaN. But, when old is
undefined and new isn't we probably always want to trigger a change event?

Might be enough to add a extra branch like:

} else if (typeof diskUsage !== typeof oldDiskUsage) {
    changed = true;
}

>                              changed = true;
> -                            break;
>                          }
>                      }
> -                    // FIXME: also test filterfn()?
> +
> +                    if (!changed) {
> +                        // tree item has been updated
> +                        for (const field of changedFields) {
> +                            if (item.data[field] !== olditem.data[field]) {
> +                                changed = true;
> +                                break;
> +                            }
> +                        }
> +                        // FIXME: also test filterfn()?
> +                    }
>                  }
>  
>                  if (changed) {



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel