Re: [pve-devel] [PATCH v2 widget-toolkit] fix #1454: InfoWidget for Memory

[Stefan Sterz <s.sterz@proxmox.com>]

On 28.03.23 14:49, Matthias Heiserer wrote:
> Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
> ---
> 
> I'm a bit unsure about the color. If clashes a bit with the red when the
> RAM is near full in light mode. Open for better suggestions, but
> should work for now.
> 
> changes from v1:
> ignore arcsize when not set
> separate progress/memory widget:
> override rendertpl and dont inject second bar
> 
>  src/Makefile                  |  1 +
>  src/css/ext6-pmx.css          |  6 ++++
>  src/panel/NodeMemoryWidget.js | 54 +++++++++++++++++++++++++++++++++++
>  src/panel/StatusView.js       |  2 +-
>  4 files changed, 62 insertions(+), 1 deletion(-)
>  create mode 100644 src/panel/NodeMemoryWidget.js
> 
> diff --git a/src/Makefile b/src/Makefile
> index 30e8fd5..a10f019 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -55,6 +55,7 @@ JSSRC=					\
>  	panel/EOLNotice.js		\
>  	panel/InputPanel.js		\
>  	panel/InfoWidget.js		\
> +	panel/NodeMemoryWidget.js	\
>  	panel/LogView.js		\
>  	panel/NodeInfoRepoStatus.js	\
>  	panel/JournalView.js		\
> diff --git a/src/css/ext6-pmx.css b/src/css/ext6-pmx.css
> index 2ffd2a8..8d2c3ef 100644
> --- a/src/css/ext6-pmx.css
> +++ b/src/css/ext6-pmx.css
> @@ -348,3 +348,9 @@ div.right-aligned {
>      color: #555;
>  }
>  /* action column fix end */
> +
> +.zfs-arc {
> +	background-color: #c976b7;
> +	color: #c976b7;

thanks for this, it will surely help user's better understand the ram
usage chart. i'd have two suggestions:

1) can we start using hsl for color going forward? the dark theme
already does that, and it helps us stay more consistent (e.g., if you
want a dark/lighter version of a color, that is much easier in hsl, in
rgb it's very easy to accidentally get a different hue etc.)

2) i think it would also be good to start using css variables in
general. if we ever do need to change these colors in a theme, it's much
easier to change a variable, then to have to override each css style.

in regard to the general color choice: yeah this purple is not ideal.
maybe you could use something that is closer to the blue in hue.
currently, the two colors used have these two closest hsl equivalents,
blue: hsl(206deg, 65%, 85%) red: hsl(360deg, 100%, 77%). so, potentially
hsl(280deg, 82.5%, 60%) would work. but im not to sure either.

purple in other contexts is used as a more intense version of red (e.g.,
heat maps) so perhaps we should think of a different hue altogether.

however, putting this all together would yield:

:root {
	--pwt-zfs-arc: hsl(280deg, 82.5%, 60%);
}

.zfs-arc {
	background-color: var(--pwt-zfs-arc);
	color: var(--pwt-zfs-arc);
}

then in the dark theme we could simply do (this color is imo more
consistent with the dark theme chart/gauge colors):

:root {
	 --pwt-zfs-arc: hsl(280deg, 100%, 40.5%);
}



> +	height: 100%;
> +}
> diff --git a/src/panel/NodeMemoryWidget.js b/src/panel/NodeMemoryWidget.js
> new file mode 100644
> index 0000000..e7619fd
> --- /dev/null
> +++ b/src/panel/NodeMemoryWidget.js
> @@ -0,0 +1,54 @@
> +Ext.define('Proxmox.panel.ArcProgress', {
> +    extend: 'Ext.ProgressBar',
> +    alias: 'widget.pmxArcProgress',
> +
> +   childEls: [
> +	'arcbar',
> +   ],
> +
> +    // modified from https://docs.sencha.com/extjs/7.0.0/classic/src/ProgressBar.js.html
> +    renderTpl: [
> +	`<div id="{id}-arcbar" data-ref="arcbar"
> +	    style='margin-right: 100%; width: auto;'
> +	    class='zfs-arc'>
> +	</div>`,
> +	'<div id="{id}-bar" data-ref="bar" class="{baseCls}-bar {baseCls}-bar-{ui}" role="presentation" style="width:{percentage}%">',
> +	    '<tpl if="internalText">',
> +		'<div class="{baseCls}-text" role="presentation">',
> +		    '<div role="presentation">{text}</div>',
> +		'</div>',
> +	    '</tpl>',
> +	'</div>',
> +    ],
> +
> +    updateArc: function(width) {
> +	this.arcbar.setStyle('margin-right', `${width}%`);
> +    },
> +
> +    initComponent: function() {
> +	this.callParent();
> +    },
> +});
> +
> +
> +Ext.define('Proxmox.widget.NodeMemory', {
> +    extend: 'Proxmox.widget.Info',
> +    alias: 'widget.pmxNodeMemoryWidget',
> +
> +    updateValue: function(text, usage, mem) {
> +	let me = this;
> +
> +	if (mem) {
> +	    usage = (mem.used - (mem.arcsize || 0)) / mem.total;
> +	    me.getComponent("progress").updateArc((mem.free / mem.total) * 100);
> +	    me.callParent([text, usage]);
> +	}
> +    },
> +
> +    initComponent: function() {
> +	let me = this;
> +	me.items.filter(i => i.xtype === 'progressbar')
> +	    .forEach(i => { i.xtype = 'pmxArcProgress'; });
> +	me.callParent();
> +    },
> +});
> diff --git a/src/panel/StatusView.js b/src/panel/StatusView.js
> index e2e81e2..7258f36 100644
> --- a/src/panel/StatusView.js
> +++ b/src/panel/StatusView.js
> @@ -72,7 +72,7 @@ Ext.define('Proxmox.panel.StatusView', {
>  	    if (Ext.isFunction(field.calculate)) {
>  		calculate = field.calculate;
>  	    }
> -	    field.updateValue(renderer.call(field, used, max), calculate(used, max));
> +	    field.updateValue(renderer.call(field, used, max), calculate(used, max), used);
>  	}
>      },
>