[pve-devel] [PATCH v2 widget-toolkit 1/3] Buttons: add AltText

Dominik Csapak d.csapak at proxmox.com
Thu Apr 7 12:46:25 CEST 2022


patches LGTM, 2/3 could be squashed into 1/3 so that we have
the new code in the same commit as the Makefile change,
or what do you think @thomas? do you prefer those things
to be split into 2 patches?

on an additional note, we could even add more functionality to the
button, namely a simple 'setTextVariant' (or similar) so that the
caller can easily change between the normal and alt text.
Also a getter (like 'isAltText' or 'isNormalText') would be nice

for now the users of this button always access the 'altText' and 'text'
property directly which doesn't seem that nice if we already have
a special class for it

but this could be done later too, so no hard feelings

On 3/28/22 15:07, Matthias Heiserer wrote:
> The same code is used once in widget toolkit and twice in PVE already,
> so it makes sense to add it as a separate button.
> 
> Signed-off-by: Matthias Heiserer <m.heiserer at proxmox.com>
> ---
> changes from v1:
> move into separate class
> rename vars to something a little bit nicer
> move comment above function
> reorder some statements
> 
> Note: Because it's now guaranteed that the function is called at render time,
> I think we can leave out the extra logic to test which text is currently set.
> 
> 
>   src/button/AltText.js | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)
>   create mode 100644 src/button/AltText.js
> 
> diff --git a/src/button/AltText.js b/src/button/AltText.js
> new file mode 100644
> index 0000000..e74d042
> --- /dev/null
> +++ b/src/button/AltText.js
> @@ -0,0 +1,22 @@
> +Ext.define('Proxmox.button.AltText', {
> +    extend: 'Proxmox.button.Button',
> +    xtype: 'proxmoxAltTextButton',
> +
> +    defaultText: "",
> +    altText: "",
> +
> +    listeners: {
> +	// HACK: calculate the max button width on first render to avoid toolbar glitches
> +	render: function(button) {
> +	    let me = this;
> +
> +	    button.setText(me.altText);
> +	    let altWidth = button.getSize().width;
> +
> +	    button.setText(me.defaultText);
> +	    let defaultWidth = button.getSize().width;
> +
> +	    button.setWidth(defaultWidth > altWidth ? defaultWidth : altWidth);
> +	},
> +    },
> +});






More information about the pve-devel mailing list